Re: [Qemu-devel] [PATCH 01/10] qemu-option: add help fallback to print the list of options

[Eric Blake]

On 09/06/2018 10:12 AM, Marc-André Lureau wrote:
> QDev options accept '?' or 'help' in the list of parameters, which is
> really handy to list the available options.
>
> Unfortunately, this isn't built in QemuOpts. qemu_opts_parse_noisily()
> seems to be the common path for command line options, so place a
> fallback to check for '?' and print help listing available options.
>
> This is very handy, for example with qemu "-spice ?".

Is that literal spelling intended (a single argument with an embedded 
space)? Because that would result in:
qemu: -spice help: invalid option

Or did you mean "qemu -spice '?'" (with the outer quotes delimiting what 
you type, and the inner quote properly escaping the ? to avoid 
unintended globbing if you have a one-byte file name in the current 
directory)?

Also, I'd rather see you favor the 'help' spelling in your text; the '?' 
spelling should continue to work (by virtue of has_help_option), but as 
that form requires shell quoting while 'help' does not, we should 
minimize its use in our documentation to avoid people forgetting to 
quote it and then hitting unintended shell globbing effects.  That would 
make your example "qemu -spice help".

> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>   util/qemu-option.c | 33 ++++++++++++++++++++++-----------
>   1 file changed, 22 insertions(+), 11 deletions(-)
>
> @@ -886,10 +891,16 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, 
> const char *params,
>   {
>       Error *err = NULL;
>       QemuOpts *opts;
> +    bool invalidp = false;
>   
> -    opts = opts_parse(list, params, permit_abbrev, false, &err);
> +    opts = opts_parse(list, params, permit_abbrev, false, &invalidp, &err);
>       if (err) {
> -        error_report_err(err);
> +        if (invalidp && has_help_option(params)) {
> +            qemu_opts_print_help(list);
> +            error_free(err);
> +        } else {
> +            error_report_err(err);
> +        }

This makes sense.  With a better commit message,
Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org