[7/7] s390: make sclp ascii console the default

This patch makes the sclp ascii default for S390.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390-virtio.c |    1 -
 vl.c             |   44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletions(-)

On Tue, Jul 24, 2012 at 7:37 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> This patch makes the sclp ascii default for S390.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390-virtio.c |    1 -
>  vl.c             |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+), 1 deletions(-)
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index 28e320d..8b48f66 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -341,7 +341,6 @@ static QEMUMachine s390_machine = {
>      .no_serial = 1,
>      .no_parallel = 1,
>      .no_sdcard = 1,
> -    .use_virtcon = 1,
>      .max_cpus = 255,
>      .is_default = 1,
>  };
> diff --git a/vl.c b/vl.c
> index 46248b9..7197724 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -168,6 +168,7 @@ int main(int argc, char **argv)
>  #define DEFAULT_RAM_SIZE 128
>
>  #define MAX_VIRTIO_CONSOLES 1
> +#define MAX_SCLP_CONSOLES   1
>
>  static const char *data_dir;
>  const char *bios_name = NULL;
> @@ -195,6 +196,7 @@ int no_quit = 0;
>  CharDriverState *serial_hds[MAX_SERIAL_PORTS];
>  CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
>  CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
> +CharDriverState *sclpcon_hds[MAX_SCLP_CONSOLES];
>  int win2k_install_hack = 0;
>  int usb_enabled = 0;
>  int singlestep = 0;
> @@ -268,6 +270,7 @@ static int default_floppy = 1;
>  static int default_cdrom = 1;
>  static int default_sdcard = 1;
>  static int default_vga = 1;
> +static int default_sclpcon = 1;
>
>  static struct {
>      const char *driver;
> @@ -289,6 +292,7 @@ static struct {
>      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
>      { .driver = "vmware-svga",          .flag = &default_vga       },
>      { .driver = "qxl-vga",              .flag = &default_vga       },
> +    { .driver = "sclpconsole",          .flag = &default_sclpcon   },
>  };
>
>  static void res_free(void)
> @@ -1935,6 +1939,7 @@ struct device_config {
>          DEV_VIRTCON,   /* -virtioconsole */
>          DEV_DEBUGCON,  /* -debugcon */
>          DEV_GDB,       /* -gdb, -s */
> +        DEV_SCLPCON,   /* sclp console */
>      } type;
>      const char *cmdline;
>      Location loc;
> @@ -2014,6 +2019,36 @@ static int parallel_parse(const char *devname)
>      return 0;
>  }
>
> +static int sclpcon_parse(const char *devname)
> +{
> +    QemuOptsList *device = qemu_find_opts("device");
> +    static int index = 0;
> +    char label[32];
> +    QemuOpts *dev_opts;
> +
> +    if (strcmp(devname, "none") == 0)

Braces.

> +        return 0;
> +    if (index == MAX_SCLP_CONSOLES) {
> +        fprintf(stderr, "qemu: too many sclp consoles\n");

The user may wonder what is the max, you could tell that.

> +        exit(1);
> +    }
> +
> +    dev_opts = qemu_opts_create(device, NULL, 0, NULL);
> +    qemu_opt_set(dev_opts, "driver", "sclpconsole");
> +
> +    snprintf(label, sizeof(label), "sclpcon%d", index);
> +    sclpcon_hds[index] = qemu_chr_new(label, devname, NULL);
> +    if (!sclpcon_hds[index]) {
> +        fprintf(stderr, "qemu: could not open sclp console '%s': %s\n",
> +                devname, strerror(errno));
> +        return -1;
> +    }
> +    qemu_opt_set(dev_opts, "chardev", label);
> +
> +    index++;
> +    return 0;
> +}
> +
>  static int virtcon_parse(const char *devname)
>  {
>      QemuOptsList *device = qemu_find_opts("device");
> @@ -3122,6 +3157,7 @@ int main(int argc, char **argv, char **envp)
>                  default_cdrom = 0;
>                  default_sdcard = 0;
>                  default_vga = 0;
> +                default_sclpcon = 0;
>                  break;
>              case QEMU_OPTION_xen_domid:
>                  if (!(xen_available())) {
> @@ -3303,11 +3339,15 @@ int main(int argc, char **argv, char **envp)
>              add_device_config(DEV_PARALLEL, "null");
>          if (default_serial && default_monitor) {
>              add_device_config(DEV_SERIAL, "mon:stdio");
> +        } else if (default_sclpcon && default_monitor) {
> +            add_device_config(DEV_SCLPCON, "mon:stdio");
>          } else if (default_virtcon && default_monitor) {
>              add_device_config(DEV_VIRTCON, "mon:stdio");
>          } else {
>              if (default_serial)
>                  add_device_config(DEV_SERIAL, "stdio");
> +            if (default_sclpcon)

Braces, also below two times.

> +                add_device_config(DEV_SCLPCON, "stdio");
>              if (default_virtcon)
>                  add_device_config(DEV_VIRTCON, "stdio");
>              if (default_monitor)
> @@ -3320,6 +3360,8 @@ int main(int argc, char **argv, char **envp)
>              add_device_config(DEV_PARALLEL, "vc:80Cx24C");
>          if (default_monitor)
>              monitor_parse("vc:80Cx24C", "readline");
> +        if (default_sclpcon)
> +            add_device_config(DEV_SCLPCON, "vc:80Cx24C");
>          if (default_virtcon)
>              add_device_config(DEV_VIRTCON, "vc:80Cx24C");
>      }
> @@ -3490,6 +3532,8 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
>          exit(1);
> +    if (foreach_device_config(DEV_SCLPCON, sclpcon_parse) < 0)
> +        exit(1);
>      if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0)
>          exit(1);
>      if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
> --
> 1.7.0.1
>

On 24/07/12 21:35, Blue Swirl wrote:
[...]
> Braces
[...]
> Braces, also below two times.

a generic question. We did it that way because the other code in vl.c is like that.

So we should make new code follow the CodingStyle even if the surrounding code looks
different, but we dont touch that surrounding code. right?

Am 25.07.2012 08:55, schrieb Christian Borntraeger:
> On 24/07/12 21:35, Blue Swirl wrote:
> [...]
>> Braces
> [...]
>> Braces, also below two times.
> 
> a generic question. We did it that way because the other code in vl.c is like that.
> 
> So we should make new code follow the CodingStyle even if the surrounding code looks
> different, but we dont touch that surrounding code. right?

There has been resistance towards doing touch-all Coding Style cleanups
since it is invasive and messes with git-blame.

Our policy is to have new code comply with the current Coding Style
(with some exceptions in audio code) and to apply fixes (e.g., add
braces) on the lines touched or in the same hunk.
scripts/checkpatch.pl complains otherwise.

Depending on amount and invasiveness of such cleanups it has sometimes
been requested (e.g., for code movements that show up as deletes and
adds) that larger cleanups be split out into a preceding patch, to keep
things reviewable and to avoid checkpatch.pl errors.

HTE,
Andreas