[Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Peter Maydell]

For command line options which permit '?' meaning 'please list the
permitted values', add support for 'help' as a synonym, by abstracting
the check out into a helper function.

This change means that in some cases where we were being lazy in
our string parsing, "?junk" will now be rejected as an invalid option
rather than being (undocumentedly) treated the same way as "?".

Update the documentation to use 'help' rather than '?', since '?'
is a shell metacharacter and thus prone to fail confusingly if there
is a single character filename in the current working directory and
the '?' has not been escaped. It's therefore better to steer users
towards 'help', though '?' is retained for backwards compatibility.

We do not, however, update the output of the system emulator's -help
(or any documentation autogenerated from the qemu-options.hx which
is the source of the -help text) because libvirt parses our -help
output and will break. At a later date when QEMU provides a better
interface so libvirt can avoid having to do this, we can update the
-help text too.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Changes v1->v2:
 * dropped the changes to -help text to avoid breaking libvirt
Changes v2->v3:
 * call out in the commit message that we've tightened up the
   parsing so '?junk' is now rejected rather than treated like '?'
 * add a TODO comment to qemu-options.hx about updating the help text
   when we are able to
 * drop a reindent of a comment we only did to placate checkpatch
 * add 'help' support to "-device devicename,help" (required the
   introduction of a new qemu_opt_has_help_opt() function)
 * note that '?' is deprecated in is_help_option documentation.

 arch_init.c       |    4 ++--
 blockdev.c        |   10 +++++-----
 bsd-user/main.c   |    4 ++--
 hw/mips_jazz.c    |    2 +-
 hw/qdev-monitor.c |    4 ++--
 hw/watchdog.c     |    2 +-
 linux-user/main.c |    4 ++--
 net.c             |    3 ++-
 qemu-common.h     |   18 ++++++++++++++++++
 qemu-doc.texi     |    2 +-
 qemu-ga.c         |    2 +-
 qemu-img.c        |    4 ++--
 qemu-option.c     |   12 ++++++++++++
 qemu-option.h     |   12 ++++++++++++
 qemu-options.hx   |    4 ++++
 qemu-timer.c      |    2 +-
 vl.c              |    4 ++--
 17 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 26f30ef..60823ba 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -680,7 +680,7 @@ void select_soundhw(const char *optarg)
 {
     struct soundhw *c;
 
-    if (*optarg == '?') {
+    if (is_help_option(optarg)) {
     show_valid_cards:
 
         printf("Valid sound card names (comma separated):\n");
@@ -688,7 +688,7 @@ void select_soundhw(const char *optarg)
             printf ("%-11s %s\n", c->name, c->descr);
         }
         printf("\n-soundhw all will enable all of the above\n");
-        exit(*optarg != '?');
+        exit(!is_help_option(optarg));
     }
     else {
         size_t l;
diff --git a/blockdev.c b/blockdev.c
index 3d75015..8669142 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -398,11 +398,11 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 #endif
 
     if ((buf = qemu_opt_get(opts, "format")) != NULL) {
-       if (strcmp(buf, "?") == 0) {
-           error_printf("Supported formats:");
-           bdrv_iterate_format(bdrv_format_print, NULL);
-           error_printf("\n");
-           return NULL;
+        if (is_help_option(buf)) {
+            error_printf("Supported formats:");
+            bdrv_iterate_format(bdrv_format_print, NULL);
+            error_printf("\n");
+            return NULL;
         }
         drv = bdrv_find_whitelisted_format(buf);
         if (!drv) {
diff --git a/bsd-user/main.c b/bsd-user/main.c
index cd33d65..095ae8e 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -681,7 +681,7 @@ static void usage(void)
            "-g port           wait gdb connection to port\n"
            "-L path           set the elf interpreter prefix (default=%s)\n"
            "-s size           set the stack size in bytes (default=%ld)\n"
-           "-cpu model        select CPU (-cpu ? for list)\n"
+           "-cpu model        select CPU (-cpu help for list)\n"
            "-drop-ld-preload  drop LD_PRELOAD for target process\n"
            "-E var=value      sets/modifies targets environment variable(s)\n"
            "-U var            unsets targets environment variable(s)\n"
@@ -825,7 +825,7 @@ int main(int argc, char **argv)
             qemu_uname_release = argv[optind++];
         } else if (!strcmp(r, "cpu")) {
             cpu_model = argv[optind++];
-            if (strcmp(cpu_model, "?") == 0) {
+            if (is_help_option(cpu_model)) {
 /* XXX: implement xxx_cpu_list for targets that still miss it */
 #if defined(cpu_list)
                     cpu_list(stdout, &fprintf);
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index bf1b799..db927f1 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -239,7 +239,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
             dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
                          rc4030_opaque, rc4030_dma_memory_rw);
             break;
-        } else if (strcmp(nd->model, "?") == 0) {
+        } else if (is_help_option(nd->model)) {
             fprintf(stderr, "qemu: Supported NICs: dp83932\n");
             exit(1);
         } else {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 7915b45..b22a37a 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -138,13 +138,13 @@ int qdev_device_help(QemuOpts *opts)
     ObjectClass *klass;
 
     driver = qemu_opt_get(opts, "driver");
-    if (driver && !strcmp(driver, "?")) {
+    if (driver && is_help_option(driver)) {
         bool show_no_user = false;
         object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
         return 1;
     }
 
-    if (!driver || !qemu_opt_get(opts, "?")) {
+    if (!driver || !qemu_opt_has_help_opt(opts)) {
         return 0;
     }
 
diff --git a/hw/watchdog.c b/hw/watchdog.c
index a42124d..b52aced 100644
--- a/hw/watchdog.c
+++ b/hw/watchdog.c
@@ -55,7 +55,7 @@ int select_watchdog(const char *p)
     QemuOpts *opts;
 
     /* -watchdog ? lists available devices and exits cleanly. */
-    if (strcmp(p, "?") == 0) {
+    if (is_help_option(p)) {
         QLIST_FOREACH(model, &watchdog_list, entry) {
             fprintf(stderr, "\t%s\t%s\n",
                      model->wdt_name, model->wdt_description);
diff --git a/linux-user/main.c b/linux-user/main.c
index a0ab8e8..25eaa11 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3140,7 +3140,7 @@ static void handle_arg_uname(const char *arg)
 static void handle_arg_cpu(const char *arg)
 {
     cpu_model = strdup(arg);
-    if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
+    if (cpu_model == NULL || is_help_option(cpu_model)) {
         /* XXX: implement xxx_cpu_list for targets that still miss it */
 #if defined(cpu_list_id)
         cpu_list_id(stdout, &fprintf, "");
@@ -3231,7 +3231,7 @@ struct qemu_argument arg_table[] = {
     {"s",          "QEMU_STACK_SIZE",  true,  handle_arg_stack_size,
      "size",       "set the stack size to 'size' bytes"},
     {"cpu",        "QEMU_CPU",         true,  handle_arg_cpu,
-     "model",      "select CPU (-cpu ? for list)"},
+     "model",      "select CPU (-cpu help for list)"},
     {"E",          "QEMU_SET_ENV",     true,  handle_arg_set_env,
      "var=value",  "sets targets environment variable (see below)"},
     {"U",          "QEMU_UNSET_ENV",   true,  handle_arg_unset_env,
diff --git a/net.c b/net.c
index dbca77b..32ca50e 100644
--- a/net.c
+++ b/net.c
@@ -691,8 +691,9 @@ int qemu_show_nic_models(const char *arg, const char *const *models)
 {
     int i;
 
-    if (!arg || strcmp(arg, "?"))
+    if (!arg || !is_help_option(arg)) {
         return 0;
+    }
 
     fprintf(stderr, "qemu: Supported NIC models: ");
     for (i = 0 ; models[i]; i++)
diff --git a/qemu-common.h b/qemu-common.h
index d26ff39..dd91912 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -136,6 +136,24 @@ int qemu_main(int argc, char **argv, char **envp);
 void qemu_get_timedate(struct tm *tm, int offset);
 int qemu_timedate_diff(struct tm *tm);
 
+/**
+ * is_help_option:
+ * @s: string to test
+ *
+ * Check whether @s is one of the standard strings which indicate
+ * that the user is asking for a list of the valid values for a
+ * command option like -cpu or -M. The current accepted strings
+ * are 'help' and '?'. '?' is deprecated (it is a shell wildcard
+ * which makes it annoying to use in a reliable way) but provided
+ * for backwards compatibility.
+ *
+ * Returns: true if @s is a request for a list.
+ */
+static inline bool is_help_option(const char *s)
+{
+    return !strcmp(s, "?") || !strcmp(s, "help");
+}
+
 /* cutils.c */
 void pstrcpy(char *buf, int buf_size, const char *str);
 void strpadcpy(char *buf, int buf_size, const char *str, char pad);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 84dad19..a41448a 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2390,7 +2390,7 @@ Set the x86 elf interpreter prefix (default=/usr/local/qemu-i386)
 @item -s size
 Set the x86 stack size in bytes (default=524288)
 @item -cpu model
-Select CPU model (-cpu ? for list and additional feature selection)
+Select CPU model (-cpu help for list and additional feature selection)
 @item -ignore-environment
 Start with an empty environment. Without this option,
 the initial environment is a copy of the caller's environment.
diff --git a/qemu-ga.c b/qemu-ga.c
index 8199da7..f1a39ec 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -736,7 +736,7 @@ int main(int argc, char **argv)
             break;
         case 'b': {
             char **list_head, **list;
-            if (*optarg == '?') {
+            if (is_help_option(optarg)) {
                 list_head = list = qmp_get_command_list();
                 while (*list != NULL) {
                     printf("%s\n", *list);
diff --git a/qemu-img.c b/qemu-img.c
index 80cfb9b..b866f80 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -350,7 +350,7 @@ static int img_create(int argc, char **argv)
         img_size = (uint64_t)sval;
     }
 
-    if (options && !strcmp(options, "?")) {
+    if (options && is_help_option(options)) {
         ret = print_block_option_help(filename, fmt);
         goto out;
     }
@@ -744,7 +744,7 @@ static int img_convert(int argc, char **argv)
     /* Initialize before goto out */
     qemu_progress_init(progress, 2.0);
 
-    if (options && !strcmp(options, "?")) {
+    if (options && is_help_option(options)) {
         ret = print_block_option_help(out_filename, out_fmt);
         goto out;
     }
diff --git a/qemu-option.c b/qemu-option.c
index 8334190..27891e7 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -529,6 +529,18 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
     return opt ? opt->str : NULL;
 }
 
+bool qemu_opt_has_help_opt(QemuOpts *opts)
+{
+    QemuOpt *opt;
+
+    QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) {
+        if (is_help_option(opt->name)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
 {
     QemuOpt *opt = qemu_opt_find(opts, name);
diff --git a/qemu-option.h b/qemu-option.h
index 951dec3..ca72986 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -107,6 +107,18 @@ struct QemuOptsList {
 };
 
 const char *qemu_opt_get(QemuOpts *opts, const char *name);
+/**
+ * qemu_opt_has_help_opt:
+ * @opts: options to search for a help request
+ *
+ * Check whether the options specified by @opts include one of the
+ * standard strings which indicate that the user is asking for a
+ * list of the valid values for a command line option (as defined
+ * by is_help_option()).
+ *
+ * Returns: true if @opts includes 'help' or equivalent.
+ */
+bool qemu_opt_has_help_opt(QemuOpts *opts);
 bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
diff --git a/qemu-options.hx b/qemu-options.hx
index dc68e15..9277414 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -6,6 +6,10 @@ HXCOMM construct option structures, enums and help message for specified
 HXCOMM architectures.
 HXCOMM HXCOMM can be used for comments, discarded from both texi and C
 
+HXCOMM TODO : when we are able to change -help output without breaking
+HXCOMM libvirt we should update the help options which refer to -cpu ?,
+HXCOMM -driver ?, etc to use the preferred -cpu help etc instead.
+
 DEFHEADING(Standard options:)
 STEXI
 @table @option
diff --git a/qemu-timer.c b/qemu-timer.c
index de98977..062fdf2 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -183,7 +183,7 @@ void configure_alarms(char const *opt)
     char *name;
     struct qemu_alarm_timer tmp;
 
-    if (!strcmp(opt, "?")) {
+    if (is_help_option(opt)) {
         show_available_alarms();
         exit(0);
     }
diff --git a/vl.c b/vl.c
index 9fea320..1fd1114 100644
--- a/vl.c
+++ b/vl.c
@@ -2086,7 +2086,7 @@ static QEMUMachine *machine_parse(const char *name)
         printf("%-20s %s%s\n", m->name, m->desc,
                m->is_default ? " (default)" : "");
     }
-    exit(!name || *name != '?');
+    exit(!name || !is_help_option(name));
 }
 
 static int tcg_init(void)
@@ -3216,7 +3216,7 @@ int main(int argc, char **argv, char **envp)
      */
     cpudef_init();
 
-    if (cpu_model && *cpu_model == '?') {
+    if (cpu_model && is_help_option(cpu_model)) {
         list_cpus(stdout, &fprintf, cpu_model);
         exit(0);
     }
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> For command line options which permit '?' meaning 'please list the
> permitted values', add support for 'help' as a synonym, by abstracting
> the check out into a helper function.
>
> This change means that in some cases where we were being lazy in
> our string parsing, "?junk" will now be rejected as an invalid option
> rather than being (undocumentedly) treated the same way as "?".
>
> Update the documentation to use 'help' rather than '?', since '?'
> is a shell metacharacter and thus prone to fail confusingly if there
> is a single character filename in the current working directory and
> the '?' has not been escaped. It's therefore better to steer users
> towards 'help', though '?' is retained for backwards compatibility.
>
> We do not, however, update the output of the system emulator's -help
> (or any documentation autogenerated from the qemu-options.hx which
> is the source of the -help text) because libvirt parses our -help
> output and will break. At a later date when QEMU provides a better
> interface so libvirt can avoid having to do this, we can update the
> -help text too.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

qdev properties named help won't work because they get interpreted as
"-device FOO,help".  No such properties exist now.  Tolerable.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> For command line options which permit '?' meaning 'please list the
> permitted values', add support for 'help' as a synonym, by abstracting
> the check out into a helper function.
>
> This change means that in some cases where we were being lazy in
> our string parsing, "?junk" will now be rejected as an invalid option
> rather than being (undocumentedly) treated the same way as "?".
>
> Update the documentation to use 'help' rather than '?', since '?'
> is a shell metacharacter and thus prone to fail confusingly if there
> is a single character filename in the current working directory and
> the '?' has not been escaped. It's therefore better to steer users
> towards 'help', though '?' is retained for backwards compatibility.
>
> We do not, however, update the output of the system emulator's -help
> (or any documentation autogenerated from the qemu-options.hx which
> is the source of the -help text) because libvirt parses our -help
> output and will break. At a later date when QEMU provides a better
> interface so libvirt can avoid having to do this, we can update the
> -help text too.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Applied. Thanks.

Regards,

Anthony Liguori

> ---
> Changes v1->v2:
>  * dropped the changes to -help text to avoid breaking libvirt
> Changes v2->v3:
>  * call out in the commit message that we've tightened up the
>    parsing so '?junk' is now rejected rather than treated like '?'
>  * add a TODO comment to qemu-options.hx about updating the help text
>    when we are able to
>  * drop a reindent of a comment we only did to placate checkpatch
>  * add 'help' support to "-device devicename,help" (required the
>    introduction of a new qemu_opt_has_help_opt() function)
>  * note that '?' is deprecated in is_help_option documentation.
>
>  arch_init.c       |    4 ++--
>  blockdev.c        |   10 +++++-----
>  bsd-user/main.c   |    4 ++--
>  hw/mips_jazz.c    |    2 +-
>  hw/qdev-monitor.c |    4 ++--
>  hw/watchdog.c     |    2 +-
>  linux-user/main.c |    4 ++--
>  net.c             |    3 ++-
>  qemu-common.h     |   18 ++++++++++++++++++
>  qemu-doc.texi     |    2 +-
>  qemu-ga.c         |    2 +-
>  qemu-img.c        |    4 ++--
>  qemu-option.c     |   12 ++++++++++++
>  qemu-option.h     |   12 ++++++++++++
>  qemu-options.hx   |    4 ++++
>  qemu-timer.c      |    2 +-
>  vl.c              |    4 ++--
>  17 files changed, 70 insertions(+), 23 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 26f30ef..60823ba 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -680,7 +680,7 @@ void select_soundhw(const char *optarg)
>  {
>      struct soundhw *c;
>  
> -    if (*optarg == '?') {
> +    if (is_help_option(optarg)) {
>      show_valid_cards:
>  
>          printf("Valid sound card names (comma separated):\n");
> @@ -688,7 +688,7 @@ void select_soundhw(const char *optarg)
>              printf ("%-11s %s\n", c->name, c->descr);
>          }
>          printf("\n-soundhw all will enable all of the above\n");
> -        exit(*optarg != '?');
> +        exit(!is_help_option(optarg));
>      }
>      else {
>          size_t l;
> diff --git a/blockdev.c b/blockdev.c
> index 3d75015..8669142 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -398,11 +398,11 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>  #endif
>  
>      if ((buf = qemu_opt_get(opts, "format")) != NULL) {
> -       if (strcmp(buf, "?") == 0) {
> -           error_printf("Supported formats:");
> -           bdrv_iterate_format(bdrv_format_print, NULL);
> -           error_printf("\n");
> -           return NULL;
> +        if (is_help_option(buf)) {
> +            error_printf("Supported formats:");
> +            bdrv_iterate_format(bdrv_format_print, NULL);
> +            error_printf("\n");
> +            return NULL;
>          }
>          drv = bdrv_find_whitelisted_format(buf);
>          if (!drv) {
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index cd33d65..095ae8e 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -681,7 +681,7 @@ static void usage(void)
>             "-g port           wait gdb connection to port\n"
>             "-L path           set the elf interpreter prefix (default=%s)\n"
>             "-s size           set the stack size in bytes (default=%ld)\n"
> -           "-cpu model        select CPU (-cpu ? for list)\n"
> +           "-cpu model        select CPU (-cpu help for list)\n"
>             "-drop-ld-preload  drop LD_PRELOAD for target process\n"
>             "-E var=value      sets/modifies targets environment variable(s)\n"
>             "-U var            unsets targets environment variable(s)\n"
> @@ -825,7 +825,7 @@ int main(int argc, char **argv)
>              qemu_uname_release = argv[optind++];
>          } else if (!strcmp(r, "cpu")) {
>              cpu_model = argv[optind++];
> -            if (strcmp(cpu_model, "?") == 0) {
> +            if (is_help_option(cpu_model)) {
>  /* XXX: implement xxx_cpu_list for targets that still miss it */
>  #if defined(cpu_list)
>                      cpu_list(stdout, &fprintf);
> diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
> index bf1b799..db927f1 100644
> --- a/hw/mips_jazz.c
> +++ b/hw/mips_jazz.c
> @@ -239,7 +239,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
>              dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
>                           rc4030_opaque, rc4030_dma_memory_rw);
>              break;
> -        } else if (strcmp(nd->model, "?") == 0) {
> +        } else if (is_help_option(nd->model)) {
>              fprintf(stderr, "qemu: Supported NICs: dp83932\n");
>              exit(1);
>          } else {
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 7915b45..b22a37a 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -138,13 +138,13 @@ int qdev_device_help(QemuOpts *opts)
>      ObjectClass *klass;
>  
>      driver = qemu_opt_get(opts, "driver");
> -    if (driver && !strcmp(driver, "?")) {
> +    if (driver && is_help_option(driver)) {
>          bool show_no_user = false;
>          object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
>          return 1;
>      }
>  
> -    if (!driver || !qemu_opt_get(opts, "?")) {
> +    if (!driver || !qemu_opt_has_help_opt(opts)) {
>          return 0;
>      }
>  
> diff --git a/hw/watchdog.c b/hw/watchdog.c
> index a42124d..b52aced 100644
> --- a/hw/watchdog.c
> +++ b/hw/watchdog.c
> @@ -55,7 +55,7 @@ int select_watchdog(const char *p)
>      QemuOpts *opts;
>  
>      /* -watchdog ? lists available devices and exits cleanly. */
> -    if (strcmp(p, "?") == 0) {
> +    if (is_help_option(p)) {
>          QLIST_FOREACH(model, &watchdog_list, entry) {
>              fprintf(stderr, "\t%s\t%s\n",
>                       model->wdt_name, model->wdt_description);
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a0ab8e8..25eaa11 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3140,7 +3140,7 @@ static void handle_arg_uname(const char *arg)
>  static void handle_arg_cpu(const char *arg)
>  {
>      cpu_model = strdup(arg);
> -    if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
> +    if (cpu_model == NULL || is_help_option(cpu_model)) {
>          /* XXX: implement xxx_cpu_list for targets that still miss it */
>  #if defined(cpu_list_id)
>          cpu_list_id(stdout, &fprintf, "");
> @@ -3231,7 +3231,7 @@ struct qemu_argument arg_table[] = {
>      {"s",          "QEMU_STACK_SIZE",  true,  handle_arg_stack_size,
>       "size",       "set the stack size to 'size' bytes"},
>      {"cpu",        "QEMU_CPU",         true,  handle_arg_cpu,
> -     "model",      "select CPU (-cpu ? for list)"},
> +     "model",      "select CPU (-cpu help for list)"},
>      {"E",          "QEMU_SET_ENV",     true,  handle_arg_set_env,
>       "var=value",  "sets targets environment variable (see below)"},
>      {"U",          "QEMU_UNSET_ENV",   true,  handle_arg_unset_env,
> diff --git a/net.c b/net.c
> index dbca77b..32ca50e 100644
> --- a/net.c
> +++ b/net.c
> @@ -691,8 +691,9 @@ int qemu_show_nic_models(const char *arg, const char *const *models)
>  {
>      int i;
>  
> -    if (!arg || strcmp(arg, "?"))
> +    if (!arg || !is_help_option(arg)) {
>          return 0;
> +    }
>  
>      fprintf(stderr, "qemu: Supported NIC models: ");
>      for (i = 0 ; models[i]; i++)
> diff --git a/qemu-common.h b/qemu-common.h
> index d26ff39..dd91912 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -136,6 +136,24 @@ int qemu_main(int argc, char **argv, char **envp);
>  void qemu_get_timedate(struct tm *tm, int offset);
>  int qemu_timedate_diff(struct tm *tm);
>  
> +/**
> + * is_help_option:
> + * @s: string to test
> + *
> + * Check whether @s is one of the standard strings which indicate
> + * that the user is asking for a list of the valid values for a
> + * command option like -cpu or -M. The current accepted strings
> + * are 'help' and '?'. '?' is deprecated (it is a shell wildcard
> + * which makes it annoying to use in a reliable way) but provided
> + * for backwards compatibility.
> + *
> + * Returns: true if @s is a request for a list.
> + */
> +static inline bool is_help_option(const char *s)
> +{
> +    return !strcmp(s, "?") || !strcmp(s, "help");
> +}
> +
>  /* cutils.c */
>  void pstrcpy(char *buf, int buf_size, const char *str);
>  void strpadcpy(char *buf, int buf_size, const char *str, char pad);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 84dad19..a41448a 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2390,7 +2390,7 @@ Set the x86 elf interpreter prefix (default=/usr/local/qemu-i386)
>  @item -s size
>  Set the x86 stack size in bytes (default=524288)
>  @item -cpu model
> -Select CPU model (-cpu ? for list and additional feature selection)
> +Select CPU model (-cpu help for list and additional feature selection)
>  @item -ignore-environment
>  Start with an empty environment. Without this option,
>  the initial environment is a copy of the caller's environment.
> diff --git a/qemu-ga.c b/qemu-ga.c
> index 8199da7..f1a39ec 100644
> --- a/qemu-ga.c
> +++ b/qemu-ga.c
> @@ -736,7 +736,7 @@ int main(int argc, char **argv)
>              break;
>          case 'b': {
>              char **list_head, **list;
> -            if (*optarg == '?') {
> +            if (is_help_option(optarg)) {
>                  list_head = list = qmp_get_command_list();
>                  while (*list != NULL) {
>                      printf("%s\n", *list);
> diff --git a/qemu-img.c b/qemu-img.c
> index 80cfb9b..b866f80 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -350,7 +350,7 @@ static int img_create(int argc, char **argv)
>          img_size = (uint64_t)sval;
>      }
>  
> -    if (options && !strcmp(options, "?")) {
> +    if (options && is_help_option(options)) {
>          ret = print_block_option_help(filename, fmt);
>          goto out;
>      }
> @@ -744,7 +744,7 @@ static int img_convert(int argc, char **argv)
>      /* Initialize before goto out */
>      qemu_progress_init(progress, 2.0);
>  
> -    if (options && !strcmp(options, "?")) {
> +    if (options && is_help_option(options)) {
>          ret = print_block_option_help(out_filename, out_fmt);
>          goto out;
>      }
> diff --git a/qemu-option.c b/qemu-option.c
> index 8334190..27891e7 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -529,6 +529,18 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
>      return opt ? opt->str : NULL;
>  }
>  
> +bool qemu_opt_has_help_opt(QemuOpts *opts)
> +{
> +    QemuOpt *opt;
> +
> +    QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) {
> +        if (is_help_option(opt->name)) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
>  bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
>  {
>      QemuOpt *opt = qemu_opt_find(opts, name);
> diff --git a/qemu-option.h b/qemu-option.h
> index 951dec3..ca72986 100644
> --- a/qemu-option.h
> +++ b/qemu-option.h
> @@ -107,6 +107,18 @@ struct QemuOptsList {
>  };
>  
>  const char *qemu_opt_get(QemuOpts *opts, const char *name);
> +/**
> + * qemu_opt_has_help_opt:
> + * @opts: options to search for a help request
> + *
> + * Check whether the options specified by @opts include one of the
> + * standard strings which indicate that the user is asking for a
> + * list of the valid values for a command line option (as defined
> + * by is_help_option()).
> + *
> + * Returns: true if @opts includes 'help' or equivalent.
> + */
> +bool qemu_opt_has_help_opt(QemuOpts *opts);
>  bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
>  uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
>  uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index dc68e15..9277414 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -6,6 +6,10 @@ HXCOMM construct option structures, enums and help message for specified
>  HXCOMM architectures.
>  HXCOMM HXCOMM can be used for comments, discarded from both texi and C
>  
> +HXCOMM TODO : when we are able to change -help output without breaking
> +HXCOMM libvirt we should update the help options which refer to -cpu ?,
> +HXCOMM -driver ?, etc to use the preferred -cpu help etc instead.
> +
>  DEFHEADING(Standard options:)
>  STEXI
>  @table @option
> diff --git a/qemu-timer.c b/qemu-timer.c
> index de98977..062fdf2 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -183,7 +183,7 @@ void configure_alarms(char const *opt)
>      char *name;
>      struct qemu_alarm_timer tmp;
>  
> -    if (!strcmp(opt, "?")) {
> +    if (is_help_option(opt)) {
>          show_available_alarms();
>          exit(0);
>      }
> diff --git a/vl.c b/vl.c
> index 9fea320..1fd1114 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2086,7 +2086,7 @@ static QEMUMachine *machine_parse(const char *name)
>          printf("%-20s %s%s\n", m->name, m->desc,
>                 m->is_default ? " (default)" : "");
>      }
> -    exit(!name || *name != '?');
> +    exit(!name || !is_help_option(name));
>  }
>  
>  static int tcg_init(void)
> @@ -3216,7 +3216,7 @@ int main(int argc, char **argv, char **envp)
>       */
>      cpudef_init();
>  
> -    if (cpu_model && *cpu_model == '?') {
> +    if (cpu_model && is_help_option(cpu_model)) {
>          list_cpus(stdout, &fprintf, cpu_model);
>          exit(0);
>      }
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Eduardo Habkost]

On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
> > For command line options which permit '?' meaning 'please list the
> > permitted values', add support for 'help' as a synonym, by abstracting
> > the check out into a helper function.
> >
> > This change means that in some cases where we were being lazy in
> > our string parsing, "?junk" will now be rejected as an invalid option
> > rather than being (undocumentedly) treated the same way as "?".
> >
> > Update the documentation to use 'help' rather than '?', since '?'
> > is a shell metacharacter and thus prone to fail confusingly if there
> > is a single character filename in the current working directory and
> > the '?' has not been escaped. It's therefore better to steer users
> > towards 'help', though '?' is retained for backwards compatibility.
> >
> > We do not, however, update the output of the system emulator's -help
> > (or any documentation autogenerated from the qemu-options.hx which
> > is the source of the -help text) because libvirt parses our -help
> > output and will break. At a later date when QEMU provides a better
> > interface so libvirt can avoid having to do this, we can update the
> > -help text too.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
> Applied. Thanks.

I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
"-cpu ?model":

[qemu/(c8057f9...)|BISECTING]$ ./install/bin/qemu-system-x86_64 -cpu ?dump
Unable to find x86 CPU definition
[qemu/(c8057f9...)|BISECTING]$


> 
> Regards,
> 
> Anthony Liguori
> 
> > ---
> > Changes v1->v2:
> >  * dropped the changes to -help text to avoid breaking libvirt
> > Changes v2->v3:
> >  * call out in the commit message that we've tightened up the
> >    parsing so '?junk' is now rejected rather than treated like '?'
> >  * add a TODO comment to qemu-options.hx about updating the help text
> >    when we are able to
> >  * drop a reindent of a comment we only did to placate checkpatch
> >  * add 'help' support to "-device devicename,help" (required the
> >    introduction of a new qemu_opt_has_help_opt() function)
> >  * note that '?' is deprecated in is_help_option documentation.
> >
> >  arch_init.c       |    4 ++--
> >  blockdev.c        |   10 +++++-----
> >  bsd-user/main.c   |    4 ++--
> >  hw/mips_jazz.c    |    2 +-
> >  hw/qdev-monitor.c |    4 ++--
> >  hw/watchdog.c     |    2 +-
> >  linux-user/main.c |    4 ++--
> >  net.c             |    3 ++-
> >  qemu-common.h     |   18 ++++++++++++++++++
> >  qemu-doc.texi     |    2 +-
> >  qemu-ga.c         |    2 +-
> >  qemu-img.c        |    4 ++--
> >  qemu-option.c     |   12 ++++++++++++
> >  qemu-option.h     |   12 ++++++++++++
> >  qemu-options.hx   |    4 ++++
> >  qemu-timer.c      |    2 +-
> >  vl.c              |    4 ++--
> >  17 files changed, 70 insertions(+), 23 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 26f30ef..60823ba 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -680,7 +680,7 @@ void select_soundhw(const char *optarg)
> >  {
> >      struct soundhw *c;
> >  
> > -    if (*optarg == '?') {
> > +    if (is_help_option(optarg)) {
> >      show_valid_cards:
> >  
> >          printf("Valid sound card names (comma separated):\n");
> > @@ -688,7 +688,7 @@ void select_soundhw(const char *optarg)
> >              printf ("%-11s %s\n", c->name, c->descr);
> >          }
> >          printf("\n-soundhw all will enable all of the above\n");
> > -        exit(*optarg != '?');
> > +        exit(!is_help_option(optarg));
> >      }
> >      else {
> >          size_t l;
> > diff --git a/blockdev.c b/blockdev.c
> > index 3d75015..8669142 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -398,11 +398,11 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
> >  #endif
> >  
> >      if ((buf = qemu_opt_get(opts, "format")) != NULL) {
> > -       if (strcmp(buf, "?") == 0) {
> > -           error_printf("Supported formats:");
> > -           bdrv_iterate_format(bdrv_format_print, NULL);
> > -           error_printf("\n");
> > -           return NULL;
> > +        if (is_help_option(buf)) {
> > +            error_printf("Supported formats:");
> > +            bdrv_iterate_format(bdrv_format_print, NULL);
> > +            error_printf("\n");
> > +            return NULL;
> >          }
> >          drv = bdrv_find_whitelisted_format(buf);
> >          if (!drv) {
> > diff --git a/bsd-user/main.c b/bsd-user/main.c
> > index cd33d65..095ae8e 100644
> > --- a/bsd-user/main.c
> > +++ b/bsd-user/main.c
> > @@ -681,7 +681,7 @@ static void usage(void)
> >             "-g port           wait gdb connection to port\n"
> >             "-L path           set the elf interpreter prefix (default=%s)\n"
> >             "-s size           set the stack size in bytes (default=%ld)\n"
> > -           "-cpu model        select CPU (-cpu ? for list)\n"
> > +           "-cpu model        select CPU (-cpu help for list)\n"
> >             "-drop-ld-preload  drop LD_PRELOAD for target process\n"
> >             "-E var=value      sets/modifies targets environment variable(s)\n"
> >             "-U var            unsets targets environment variable(s)\n"
> > @@ -825,7 +825,7 @@ int main(int argc, char **argv)
> >              qemu_uname_release = argv[optind++];
> >          } else if (!strcmp(r, "cpu")) {
> >              cpu_model = argv[optind++];
> > -            if (strcmp(cpu_model, "?") == 0) {
> > +            if (is_help_option(cpu_model)) {
> >  /* XXX: implement xxx_cpu_list for targets that still miss it */
> >  #if defined(cpu_list)
> >                      cpu_list(stdout, &fprintf);
> > diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
> > index bf1b799..db927f1 100644
> > --- a/hw/mips_jazz.c
> > +++ b/hw/mips_jazz.c
> > @@ -239,7 +239,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
> >              dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
> >                           rc4030_opaque, rc4030_dma_memory_rw);
> >              break;
> > -        } else if (strcmp(nd->model, "?") == 0) {
> > +        } else if (is_help_option(nd->model)) {
> >              fprintf(stderr, "qemu: Supported NICs: dp83932\n");
> >              exit(1);
> >          } else {
> > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> > index 7915b45..b22a37a 100644
> > --- a/hw/qdev-monitor.c
> > +++ b/hw/qdev-monitor.c
> > @@ -138,13 +138,13 @@ int qdev_device_help(QemuOpts *opts)
> >      ObjectClass *klass;
> >  
> >      driver = qemu_opt_get(opts, "driver");
> > -    if (driver && !strcmp(driver, "?")) {
> > +    if (driver && is_help_option(driver)) {
> >          bool show_no_user = false;
> >          object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
> >          return 1;
> >      }
> >  
> > -    if (!driver || !qemu_opt_get(opts, "?")) {
> > +    if (!driver || !qemu_opt_has_help_opt(opts)) {
> >          return 0;
> >      }
> >  
> > diff --git a/hw/watchdog.c b/hw/watchdog.c
> > index a42124d..b52aced 100644
> > --- a/hw/watchdog.c
> > +++ b/hw/watchdog.c
> > @@ -55,7 +55,7 @@ int select_watchdog(const char *p)
> >      QemuOpts *opts;
> >  
> >      /* -watchdog ? lists available devices and exits cleanly. */
> > -    if (strcmp(p, "?") == 0) {
> > +    if (is_help_option(p)) {
> >          QLIST_FOREACH(model, &watchdog_list, entry) {
> >              fprintf(stderr, "\t%s\t%s\n",
> >                       model->wdt_name, model->wdt_description);
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index a0ab8e8..25eaa11 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -3140,7 +3140,7 @@ static void handle_arg_uname(const char *arg)
> >  static void handle_arg_cpu(const char *arg)
> >  {
> >      cpu_model = strdup(arg);
> > -    if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
> > +    if (cpu_model == NULL || is_help_option(cpu_model)) {
> >          /* XXX: implement xxx_cpu_list for targets that still miss it */
> >  #if defined(cpu_list_id)
> >          cpu_list_id(stdout, &fprintf, "");
> > @@ -3231,7 +3231,7 @@ struct qemu_argument arg_table[] = {
> >      {"s",          "QEMU_STACK_SIZE",  true,  handle_arg_stack_size,
> >       "size",       "set the stack size to 'size' bytes"},
> >      {"cpu",        "QEMU_CPU",         true,  handle_arg_cpu,
> > -     "model",      "select CPU (-cpu ? for list)"},
> > +     "model",      "select CPU (-cpu help for list)"},
> >      {"E",          "QEMU_SET_ENV",     true,  handle_arg_set_env,
> >       "var=value",  "sets targets environment variable (see below)"},
> >      {"U",          "QEMU_UNSET_ENV",   true,  handle_arg_unset_env,
> > diff --git a/net.c b/net.c
> > index dbca77b..32ca50e 100644
> > --- a/net.c
> > +++ b/net.c
> > @@ -691,8 +691,9 @@ int qemu_show_nic_models(const char *arg, const char *const *models)
> >  {
> >      int i;
> >  
> > -    if (!arg || strcmp(arg, "?"))
> > +    if (!arg || !is_help_option(arg)) {
> >          return 0;
> > +    }
> >  
> >      fprintf(stderr, "qemu: Supported NIC models: ");
> >      for (i = 0 ; models[i]; i++)
> > diff --git a/qemu-common.h b/qemu-common.h
> > index d26ff39..dd91912 100644
> > --- a/qemu-common.h
> > +++ b/qemu-common.h
> > @@ -136,6 +136,24 @@ int qemu_main(int argc, char **argv, char **envp);
> >  void qemu_get_timedate(struct tm *tm, int offset);
> >  int qemu_timedate_diff(struct tm *tm);
> >  
> > +/**
> > + * is_help_option:
> > + * @s: string to test
> > + *
> > + * Check whether @s is one of the standard strings which indicate
> > + * that the user is asking for a list of the valid values for a
> > + * command option like -cpu or -M. The current accepted strings
> > + * are 'help' and '?'. '?' is deprecated (it is a shell wildcard
> > + * which makes it annoying to use in a reliable way) but provided
> > + * for backwards compatibility.
> > + *
> > + * Returns: true if @s is a request for a list.
> > + */
> > +static inline bool is_help_option(const char *s)
> > +{
> > +    return !strcmp(s, "?") || !strcmp(s, "help");
> > +}
> > +
> >  /* cutils.c */
> >  void pstrcpy(char *buf, int buf_size, const char *str);
> >  void strpadcpy(char *buf, int buf_size, const char *str, char pad);
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index 84dad19..a41448a 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -2390,7 +2390,7 @@ Set the x86 elf interpreter prefix (default=/usr/local/qemu-i386)
> >  @item -s size
> >  Set the x86 stack size in bytes (default=524288)
> >  @item -cpu model
> > -Select CPU model (-cpu ? for list and additional feature selection)
> > +Select CPU model (-cpu help for list and additional feature selection)
> >  @item -ignore-environment
> >  Start with an empty environment. Without this option,
> >  the initial environment is a copy of the caller's environment.
> > diff --git a/qemu-ga.c b/qemu-ga.c
> > index 8199da7..f1a39ec 100644
> > --- a/qemu-ga.c
> > +++ b/qemu-ga.c
> > @@ -736,7 +736,7 @@ int main(int argc, char **argv)
> >              break;
> >          case 'b': {
> >              char **list_head, **list;
> > -            if (*optarg == '?') {
> > +            if (is_help_option(optarg)) {
> >                  list_head = list = qmp_get_command_list();
> >                  while (*list != NULL) {
> >                      printf("%s\n", *list);
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 80cfb9b..b866f80 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -350,7 +350,7 @@ static int img_create(int argc, char **argv)
> >          img_size = (uint64_t)sval;
> >      }
> >  
> > -    if (options && !strcmp(options, "?")) {
> > +    if (options && is_help_option(options)) {
> >          ret = print_block_option_help(filename, fmt);
> >          goto out;
> >      }
> > @@ -744,7 +744,7 @@ static int img_convert(int argc, char **argv)
> >      /* Initialize before goto out */
> >      qemu_progress_init(progress, 2.0);
> >  
> > -    if (options && !strcmp(options, "?")) {
> > +    if (options && is_help_option(options)) {
> >          ret = print_block_option_help(out_filename, out_fmt);
> >          goto out;
> >      }
> > diff --git a/qemu-option.c b/qemu-option.c
> > index 8334190..27891e7 100644
> > --- a/qemu-option.c
> > +++ b/qemu-option.c
> > @@ -529,6 +529,18 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
> >      return opt ? opt->str : NULL;
> >  }
> >  
> > +bool qemu_opt_has_help_opt(QemuOpts *opts)
> > +{
> > +    QemuOpt *opt;
> > +
> > +    QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) {
> > +        if (is_help_option(opt->name)) {
> > +            return true;
> > +        }
> > +    }
> > +    return false;
> > +}
> > +
> >  bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
> >  {
> >      QemuOpt *opt = qemu_opt_find(opts, name);
> > diff --git a/qemu-option.h b/qemu-option.h
> > index 951dec3..ca72986 100644
> > --- a/qemu-option.h
> > +++ b/qemu-option.h
> > @@ -107,6 +107,18 @@ struct QemuOptsList {
> >  };
> >  
> >  const char *qemu_opt_get(QemuOpts *opts, const char *name);
> > +/**
> > + * qemu_opt_has_help_opt:
> > + * @opts: options to search for a help request
> > + *
> > + * Check whether the options specified by @opts include one of the
> > + * standard strings which indicate that the user is asking for a
> > + * list of the valid values for a command line option (as defined
> > + * by is_help_option()).
> > + *
> > + * Returns: true if @opts includes 'help' or equivalent.
> > + */
> > +bool qemu_opt_has_help_opt(QemuOpts *opts);
> >  bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
> >  uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval);
> >  uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index dc68e15..9277414 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -6,6 +6,10 @@ HXCOMM construct option structures, enums and help message for specified
> >  HXCOMM architectures.
> >  HXCOMM HXCOMM can be used for comments, discarded from both texi and C
> >  
> > +HXCOMM TODO : when we are able to change -help output without breaking
> > +HXCOMM libvirt we should update the help options which refer to -cpu ?,
> > +HXCOMM -driver ?, etc to use the preferred -cpu help etc instead.
> > +
> >  DEFHEADING(Standard options:)
> >  STEXI
> >  @table @option
> > diff --git a/qemu-timer.c b/qemu-timer.c
> > index de98977..062fdf2 100644
> > --- a/qemu-timer.c
> > +++ b/qemu-timer.c
> > @@ -183,7 +183,7 @@ void configure_alarms(char const *opt)
> >      char *name;
> >      struct qemu_alarm_timer tmp;
> >  
> > -    if (!strcmp(opt, "?")) {
> > +    if (is_help_option(opt)) {
> >          show_available_alarms();
> >          exit(0);
> >      }
> > diff --git a/vl.c b/vl.c
> > index 9fea320..1fd1114 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2086,7 +2086,7 @@ static QEMUMachine *machine_parse(const char *name)
> >          printf("%-20s %s%s\n", m->name, m->desc,
> >                 m->is_default ? " (default)" : "");
> >      }
> > -    exit(!name || *name != '?');
> > +    exit(!name || !is_help_option(name));
> >  }
> >  
> >  static int tcg_init(void)
> > @@ -3216,7 +3216,7 @@ int main(int argc, char **argv, char **envp)
> >       */
> >      cpudef_init();
> >  
> > -    if (cpu_model && *cpu_model == '?') {
> > +    if (cpu_model && is_help_option(cpu_model)) {
> >          list_cpus(stdout, &fprintf, cpu_model);
> >          exit(0);
> >      }
> > -- 
> > 1.7.9.5
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Peter Maydell]

On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > For command line options which permit '?' meaning 'please list the
>> > permitted values', add support for 'help' as a synonym, by abstracting
>> > the check out into a helper function.

>> Applied. Thanks.
>
> I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
> "-cpu ?model":

These options appear to be completely undocumented. They're also pretty
ugly syntax and seem to be x86 specific. However we can unbreak them
if we must with a patch like this:

--- a/vl.c
+++ b/vl.c
@@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp)
      */
     cpudef_init();

-    if (cpu_model && is_help_option(cpu_model)) {
+    /* We have to check for "starts with '?' as well as is_help_option
+     * to support targets which implement various weird help options
+     * via '?thingy' syntax.
+     */
+    if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) {
         list_cpus(stdout, &fprintf, cpu_model);
         exit(0);
     }

(will send as a proper patch with commit message and signoff tomorrow).

Any suggestions for what the sane syntax for these options would be?
(ie the analogous change to having '?' go to 'help').

-- PMM

Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

On 08/09/2012 03:02 PM, Peter Maydell wrote:
> On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>> For command line options which permit '?' meaning 'please list the
>>>> permitted values', add support for 'help' as a synonym, by abstracting
>>>> the check out into a helper function.
> 
>>> Applied. Thanks.
>>
>> I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
>> "-cpu ?model":
> 
> These options appear to be completely undocumented. They're also pretty
> ugly syntax and seem to be x86 specific.

Thankfully, libvirt is not using them.  If there is a reason for libvirt
to start using them, then libvirt would rather go through QMP than any
new syntax when talking to newer qemu (that is, I'm okay if we
completely lose the spelling as long as we don't lose the feature).

> 
> Any suggestions for what the sane syntax for these options would be?
> (ie the analogous change to having '?' go to 'help').

-cpu help=dump
-cpu help=cpuid

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Eduardo Habkost]

On Thu, Aug 09, 2012 at 10:02:22PM +0100, Peter Maydell wrote:
> On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >> > For command line options which permit '?' meaning 'please list the
> >> > permitted values', add support for 'help' as a synonym, by abstracting
> >> > the check out into a helper function.
> 
> >> Applied. Thanks.
> >
> > I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
> > "-cpu ?model":
> 
> These options appear to be completely undocumented. They're also pretty
> ugly syntax and seem to be x86 specific.

Agreed. I wasn't aware it was completely undocumented, I thought there
was documentation somewhere.


> However we can unbreak them
> if we must with a patch like this:
> 
> --- a/vl.c
> +++ b/vl.c
> @@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp)
>       */
>      cpudef_init();
> 
> -    if (cpu_model && is_help_option(cpu_model)) {
> +    /* We have to check for "starts with '?' as well as is_help_option
> +     * to support targets which implement various weird help options
> +     * via '?thingy' syntax.
> +     */
> +    if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) {
>          list_cpus(stdout, &fprintf, cpu_model);
>          exit(0);
>      }
> 
> (will send as a proper patch with commit message and signoff tomorrow).
> 
> Any suggestions for what the sane syntax for these options would be?
> (ie the analogous change to having '?' go to 'help').

What about "-cpu help,dump" or "-cpu help=dump"?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3] Support 'help' as a synonym for '?' in command line options

[Anthony Liguori]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Aug 09, 2012 at 10:02:22PM +0100, Peter Maydell wrote:
>> On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> > On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote:
>> >> Peter Maydell <peter.maydell@linaro.org> writes:
>> >> > For command line options which permit '?' meaning 'please list the
>> >> > permitted values', add support for 'help' as a synonym, by abstracting
>> >> > the check out into a helper function.
>> 
>> >> Applied. Thanks.
>> >
>> > I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and
>> > "-cpu ?model":
>> 
>> These options appear to be completely undocumented. They're also pretty
>> ugly syntax and seem to be x86 specific.
>
> Agreed. I wasn't aware it was completely undocumented, I thought there
> was documentation somewhere.
>
>
>> However we can unbreak them
>> if we must with a patch like this:
>> 
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp)
>>       */
>>      cpudef_init();
>> 
>> -    if (cpu_model && is_help_option(cpu_model)) {
>> +    /* We have to check for "starts with '?' as well as is_help_option
>> +     * to support targets which implement various weird help options
>> +     * via '?thingy' syntax.
>> +     */
>> +    if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) {
>>          list_cpus(stdout, &fprintf, cpu_model);
>>          exit(0);
>>      }
>> 
>> (will send as a proper patch with commit message and signoff tomorrow).
>> 
>> Any suggestions for what the sane syntax for these options would be?
>> (ie the analogous change to having '?' go to 'help').
>
> What about "-cpu help,dump" or "-cpu help=dump"?

Let's just drop the feature.  I doubt a user would ever do this.

For 1.3, I'd like to introduce glib option groups and allow for group
specific help options.  IOW, --help-cpu

Regards,

Anthony Liguori

>
> -- 
> Eduardo