[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 01/10] qemu-option: add help fallback to print t

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH 01/10] qemu-option: add help fallback to print the list of options
Date: Thu, 6 Sep 2018 10:26:07 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]