Re: [Qemu-devel] [PATCH 5/5] qdev: Add new '-device help' option, shows all devices and properties

[Markus Armbruster]

I like PATCH 1-3/5, but this one needs discussion.

Amit Shah <address@hidden> writes:

> The new '-device help' option shows all the devices that are registered
> with qdev and prints out all the properties each device has, along with
> the description for each property.
>
> This is useful in creating automatic documentation for all the options
> that we have and support.
>
> Signed-off-by: Amit Shah <address@hidden>
> ---
>  hw/qdev.c |   15 +++++++++++----
>  1 files changed, 11 insertions(+), 4 deletions(-)

Documentation update missing: qemu-options.hx

> diff --git a/hw/qdev.c b/hw/qdev.c
> index 89ba986..4be2f66 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -151,7 +151,7 @@ static int set_property(const char *name, const char 
> *value, void *opaque)
>      return 0;
>  }
>  
> -static int show_device_props(const char *driver)
> +static int show_device_props(const char *driver, const char *prefix)
>  {
>      DeviceInfo *info;
>      Property *prop;
> @@ -161,6 +161,10 @@ static int show_device_props(const char *driver)
>          return 0;
>      }
>  
> +    if (!prefix) {
> +        prefix = "";
> +    }
> +

Just make the caller pass "" instead of NULL.

>      for (prop = info->props; prop && prop->name; prop++) {
>          /*
>           * TODO Properties without a parser are just for dirty hacks.
> @@ -171,7 +175,7 @@ static int show_device_props(const char *driver)
>          if (!prop->info->parse) {
>              continue;           /* no way to set it, don't show */
>          }
> -        error_printf("%s.%s=%s, %s\n", info->name, prop->name,
> +        error_printf("%s%s.%s=%s, %s\n", prefix, info->name, prop->name,
>                       prop->info->name, prop->desc ?: "");
>      }
>      return 1;
> @@ -183,12 +187,15 @@ int qdev_device_help(QemuOpts *opts)
>      DeviceInfo *info;
>  
>      driver = qemu_opt_get(opts, "driver");
> -    if (driver && !strcmp(driver, "?")) {
> +    if (driver && (!strcmp(driver, "?") || !strcmp(driver, "help"))) {
>          for (info = device_info_list; info != NULL; info = info->next) {
>              if (info->no_user) {
>                  continue;       /* not available, don't show */
>              }
>              qdev_print_devinfo(info);
> +            if (!strcmp(driver, "help")) {
> +                show_device_props(info->name, "\t");
> +            }
>          }
>          return 1;
>      }

There is "-device \?" and "-device help", but the user interface
provides no clue about the difference between them.

"-device help" loses when we ever pick up a device model with name
"help".  Not that "?" was a particularly smart choice...

Do we really need two kinds of help output?  Where the second is
basically the same as "-device FOO,\?" for all FOO?  Is that convenience
worth the extra UI complexity?

Doing it in the shell isn't exactly hard:

$ for i in `qemu -device \? 2>&1 | awk -F \" '{ print $2 }'`; do qemu -device 
$i,\?; done

> @@ -197,7 +204,7 @@ int qdev_device_help(QemuOpts *opts)
>          return 0;
>      }
>  
> -    return show_device_props(driver);
> +    return show_device_props(driver, NULL);
>  }
>  
>  DeviceState *qdev_device_add(QemuOpts *opts)