diff mbox

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

Message ID 1404907292-20442-3-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi July 9, 2014, 12:01 p.m. UTC
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(-)

Comments

Eric Blake July 9, 2014, 12:49 p.m. UTC | #1
On 07/09/2014 06: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(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Cole Robinson July 9, 2014, 3:20 p.m. UTC | #2
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
diff mbox

Patch

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;
 }