Re: [Qemu-devel] [PATCH 01/17] chardev: Properly initialize ChardevCommon components

[Paolo Bonzini]

On 20/02/2016 01:19, Eric Blake wrote:
> Commit d0d7708b forgot to parse logging for spice chardevs and
> virtual consoles. This requires making qemu_chr_parse_common()
> non-static. While at it, use a temporary variable to make the
> code shorter, as well as reduce the churn when a later patch
> alters the layout of simple unions.
> 
> Signed-off-by: Eric Blake <address@hidden>
> CC: Daniel P. Berrange <address@hidden>
> ---
>  include/sysemu/char.h | 10 ++++++++++
>  qemu-char.c           |  2 +-
>  spice-qemu-char.c     | 12 ++++++++----
>  ui/console.c          | 20 +++++++++++---------
>  4 files changed, 30 insertions(+), 14 deletions(-)
> 
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index e035d1c..e46884f 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -115,6 +115,16 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>                                      Error **errp);
> 
>  /**
> + * @qemu_chr_parse_common:
> + *
> + * Parse the common options available to all character backends.
> + *
> + * @opts the options that still need parsing
> + * @backend a new backend
> + */
> +void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend);
> +
> +/**
>   * @qemu_chr_new:
>   *
>   * Create a new character backend from a URI.
> diff --git a/qemu-char.c b/qemu-char.c
> index ad11b75..fc8ffda 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3490,7 +3490,7 @@ fail:
>      return NULL;
>  }
> 
> -static void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend)
> +void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend)
>  {
>      const char *logfile = qemu_opt_get(opts, "logfile");
> 
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 7c1f438..21885c5 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -366,26 +366,30 @@ static void qemu_chr_parse_spice_vmc(QemuOpts *opts, 
> ChardevBackend *backend,
>                                       Error **errp)
>  {
>      const char *name = qemu_opt_get(opts, "name");
> +    ChardevSpiceChannel *spicevmc;
> 
>      if (name == NULL) {
>          error_setg(errp, "chardev: spice channel: no name given");
>          return;
>      }
> -    backend->u.spicevmc = g_new0(ChardevSpiceChannel, 1);
> -    backend->u.spicevmc->type = g_strdup(name);
> +    spicevmc = backend->u.spicevmc = g_new0(ChardevSpiceChannel, 1);
> +    qemu_chr_parse_common(opts, qapi_ChardevSpiceChannel_base(spicevmc));
> +    spicevmc->type = g_strdup(name);
>  }
> 
>  static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend 
> *backend,
>                                        Error **errp)
>  {
>      const char *name = qemu_opt_get(opts, "name");
> +    ChardevSpicePort *spiceport;
> 
>      if (name == NULL) {
>          error_setg(errp, "chardev: spice port: no name given");
>          return;
>      }
> -    backend->u.spiceport = g_new0(ChardevSpicePort, 1);
> -    backend->u.spiceport->fqdn = g_strdup(name);
> +    spiceport = backend->u.spiceport = g_new0(ChardevSpicePort, 1);
> +    qemu_chr_parse_common(opts, qapi_ChardevSpicePort_base(spiceport));
> +    spiceport->fqdn = g_strdup(name);
>  }
> 
>  static void register_types(void)
> diff --git a/ui/console.c b/ui/console.c
> index b739ae9..7db0fd2 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -2060,31 +2060,33 @@ static void qemu_chr_parse_vc(QemuOpts *opts, 
> ChardevBackend *backend,
>                                Error **errp)
>  {
>      int val;
> +    ChardevVC *vc;
> 
> -    backend->u.vc = g_new0(ChardevVC, 1);
> +    vc = backend->u.vc = g_new0(ChardevVC, 1);
> +    qemu_chr_parse_common(opts, qapi_ChardevVC_base(vc));
> 
>      val = qemu_opt_get_number(opts, "width", 0);
>      if (val != 0) {
> -        backend->u.vc->has_width = true;
> -        backend->u.vc->width = val;
> +        vc->has_width = true;
> +        vc->width = val;
>      }
> 
>      val = qemu_opt_get_number(opts, "height", 0);
>      if (val != 0) {
> -        backend->u.vc->has_height = true;
> -        backend->u.vc->height = val;
> +        vc->has_height = true;
> +        vc->height = val;
>      }
> 
>      val = qemu_opt_get_number(opts, "cols", 0);
>      if (val != 0) {
> -        backend->u.vc->has_cols = true;
> -        backend->u.vc->cols = val;
> +        vc->has_cols = true;
> +        vc->cols = val;
>      }
> 
>      val = qemu_opt_get_number(opts, "rows", 0);
>      if (val != 0) {
> -        backend->u.vc->has_rows = true;
> -        backend->u.vc->rows = val;
> +        vc->has_rows = true;
> +        vc->rows = val;
>      }
>  }
> 

I've picked up this patch and will send a pull request ASAP.

Paolo