Re: [Qemu-devel] [PATCH for-2.1 2/2] qdev-monitor: include QOM properties in -device FOO, help output

[Cole Robinson <crobinso@redhat.com>]

On 07/09/2014 08:01 AM, Stefan Hajnoczi wrote:
> Update -device FOO,help to include QOM properties in addition to qdev
> properties.  Devices are gradually adding more QOM properties that are
> not reflected as qdev properties.
> 
> It is important to report all device properties since management tools
> like libvirt use this information (and device-list-properties QMP) to
> detect the presence of QEMU features.
> 
> This patch reuses the device-list-properties QMP machinery to avoid code
> duplication.
> 
> Reported-by: Cole Robinson <crobinso@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qdev-monitor.c | 40 +++++++++++++++++-----------------------
>  1 file changed, 17 insertions(+), 23 deletions(-)
> 
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index f87f3d8..5fe5e75 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -182,9 +182,10 @@ static const char *find_typename_by_alias(const char *alias)
>  
>  int qdev_device_help(QemuOpts *opts)
>  {
> +    Error *local_err = NULL;
>      const char *driver;
> -    Property *prop;
> -    ObjectClass *klass;
> +    DevicePropertyInfoList *prop_list;
> +    DevicePropertyInfoList *prop;
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (driver && is_help_option(driver)) {
> @@ -196,35 +197,28 @@ int qdev_device_help(QemuOpts *opts)
>          return 0;
>      }
>  
> -    klass = object_class_by_name(driver);
> -    if (!klass) {
> +    if (!object_class_by_name(driver)) {
>          const char *typename = find_typename_by_alias(driver);
>  
>          if (typename) {
>              driver = typename;
> -            klass = object_class_by_name(driver);
>          }
>      }
>  
> -    if (!object_class_dynamic_cast(klass, TYPE_DEVICE)) {
> -        return 0;
> +    prop_list = qmp_device_list_properties(driver, &local_err);
> +    if (!prop_list) {
> +        error_printf("%s\n", error_get_pretty(local_err));
> +        error_free(local_err);
> +        return 1;
>      }
> -    do {
> -        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
> -            /*
> -             * TODO Properties without a parser are just for dirty hacks.
> -             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
> -             * for removal.  This conditional should be removed along with
> -             * it.
> -             */
> -            if (!prop->info->set) {
> -                continue;           /* no way to set it, don't show */
> -            }
> -            error_printf("%s.%s=%s\n", driver, prop->name,
> -                         prop->info->legacy_name ?: prop->info->name);
> -        }
> -        klass = object_class_get_parent(klass);
> -    } while (klass != object_class_by_name(TYPE_DEVICE));
> +
> +    for (prop = prop_list; prop; prop = prop->next) {
> +        error_printf("%s.%s=%s\n", driver,
> +                     prop->value->name,
> +                     prop->value->type);
> +    }
> +
> +    qapi_free_DevicePropertyInfoList(prop_list);
>      return 1;
>  }
>  
> 

Ah, I was a bit confused here. The actual issue I was hitting is fixed by the
commit that Paolo pointed to:

commit f4eb32b590bf58c1c67570775eb78beb09964fad
Author: Stefan Hajnoczi <stefanha@redhat.com>
Date:   Tue May 20 14:29:01 2014 +0200

    qmp: show QOM properties in device-list-properties

All libvirt versions since 1.0.0 use device-list-properties over the command
line introspection. I tried testing with libvirt 0.10.2.8 but there are other
issues that trip it up, like changed -help output, so not sure if we even care
about that case.

I did verify that -device virtio-blk,? now lists bootindex again, so:

Tested-by: Cole Robinson <crobinso@redhat.com>

Thanks,
Cole