Re: [Qemu-devel] [PATCH v2 1/3] Rework --name to use QemuOpts

[Laszlo Ersek <lersek@redhat.com>]

On 01/30/14 11:20, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  vl.c | 52 +++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 7f4fe0d..5f993e4 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -531,6 +531,27 @@ static QemuOptsList qemu_msg_opts = {
>      },
>  };
>  
> +static QemuOptsList qemu_name_opts = {
> +    .name = "name",
> +    .implied_opt_name = "guest",
> +    .merge_lists = true,
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_name_opts.head),
> +    .desc = {
> +        {
> +            .name = "guest",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Sets the name of the guest.\n"
> +                    "This name will be displayed in the SDL window caption.\n"
> +                    "The name will also be used for the VNC server",
> +        }, {
> +            .name = "process",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Sets the name of the QEMU process, as shown in top etc",
> +        },
> +        { /* End of list */ }
> +    },
> +};
> +
>  /**
>   * Get machine options
>   *
> @@ -981,6 +1002,18 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
>      return 0;
>  }
>  
> +static void parse_name(QemuOpts *opts)
> +{
> +    const char *proc_name;
> +
> +    qemu_name = qemu_opt_get(opts, "guest");
> +
> +    proc_name = qemu_opt_get(opts, "process");
> +    if (proc_name) {
> +        os_set_proc_name(proc_name);
> +    }
> +}
> +
>  bool usb_enabled(bool default_usb)
>  {
>      return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb);
> @@ -2895,6 +2928,7 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_tpmdev_opts);
>      qemu_add_opts(&qemu_realtime_opts);
>      qemu_add_opts(&qemu_msg_opts);
> +    qemu_add_opts(&qemu_name_opts);
>  
>      runstate_init();
>  
> @@ -3630,19 +3664,11 @@ int main(int argc, char **argv, char **envp)
>                                  "is no longer supported.\n");
>                  break;
>              case QEMU_OPTION_name:
> -                qemu_name = g_strdup(optarg);
> -		 {
> -		     char *p = strchr(qemu_name, ',');
> -		     if (p != NULL) {
> -		        *p++ = 0;
> -			if (strncmp(p, "process=", 8)) {
> -			    fprintf(stderr, "Unknown subargument %s to -name\n", p);
> -			    exit(1);
> -			}
> -			p += 8;
> -			os_set_proc_name(p);
> -		     }
> -		 }
> +                opts = qemu_opts_parse(qemu_find_opts("name"), optarg, 1);
> +                if (!opts) {
> +                    exit(1);
> +                }
> +                parse_name(opts);
>                  break;
>              case QEMU_OPTION_prom_env:
>                  if (nb_prom_envs >= MAX_PROM_ENVS) {
> 

I have one question, but it doesn't block my R-b:

Did you test (and if so, how) the new .help text for "guest"? Because it
seems to be the only such text that has newline characters embedded. I
looked around the tree a bit, and it seems that the only way to get
these option texts is the "query-command-line-options" QMP command
(apparently not available via HMP). If that's the case, then the
embedded newlines could / should be dropped. But I don't really care
about those.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo