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

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

[Stefan Hajnoczi]

These two patches fix the -device FOO,help output regression that Cole spotted
in QEMU 2.0-rc0.  The problem is that virtio-blk-pci qdev properties have been
converted to QOM alias properties but -device FOO,help shows only qdev
properties.

We simply need to update -device FOO,help code to use both qdev and QOM
properties.  Note that types change because a 'drive' qdev type is actually a
'str' QOM type.  We're moving more and more to QOM properties where the final
type for this property would be 'link<Drive>' or similar.

Cole: please confirm that this fixes the issue

Stefan Hajnoczi (2):
  qmp: hide "hotplugged" device property from device-list-properties
  qdev-monitor: include QOM properties in -device FOO,help output

 qdev-monitor.c | 40 +++++++++++++++++-----------------------
 qmp.c          |  1 +
 2 files changed, 18 insertions(+), 23 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH for-2.1 1/2] qmp: hide "hotplugged" device property from device-list-properties

[Stefan Hajnoczi]

The "hotplugged" device property was not reported before commit
f4eb32b590bf58c1c67570775eb78beb09964fad ("qmp: show QOM properties in
device-list-properties").  Fix this difference.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qmp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qmp.c b/qmp.c
index 0d2553a..c6767c4 100644
--- a/qmp.c
+++ b/qmp.c
@@ -509,6 +509,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
         if (strcmp(prop->name, "type") == 0 ||
             strcmp(prop->name, "realized") == 0 ||
             strcmp(prop->name, "hotpluggable") == 0 ||
+            strcmp(prop->name, "hotplugged") == 0 ||
             strcmp(prop->name, "parent_bus") == 0) {
             continue;
         }
-- 
1.9.3

Re: [Qemu-devel] [PATCH for-2.1 1/2] qmp: hide "hotplugged" device property from device-list-properties

[Eric Blake]

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

On 07/09/2014 06:01 AM, Stefan Hajnoczi wrote:
> The "hotplugged" device property was not reported before commit
> f4eb32b590bf58c1c67570775eb78beb09964fad ("qmp: show QOM properties in
> device-list-properties").  Fix this difference.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qmp.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/qmp.c b/qmp.c
> index 0d2553a..c6767c4 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -509,6 +509,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>          if (strcmp(prop->name, "type") == 0 ||
>              strcmp(prop->name, "realized") == 0 ||
>              strcmp(prop->name, "hotpluggable") == 0 ||
> +            strcmp(prop->name, "hotplugged") == 0 ||
>              strcmp(prop->name, "parent_bus") == 0) {
>              continue;
>          }
> 

-- 
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: 604 bytes --]

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

[Stefan Hajnoczi]

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

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

[Eric Blake]

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

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>

-- 
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: 604 bytes --]

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

[Cole Robinson]

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

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

[Stefan Hajnoczi]

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

On Wed, Jul 09, 2014 at 02:01:30PM +0200, Stefan Hajnoczi wrote:
> These two patches fix the -device FOO,help output regression that Cole spotted
> in QEMU 2.0-rc0.  The problem is that virtio-blk-pci qdev properties have been
> converted to QOM alias properties but -device FOO,help shows only qdev
> properties.
> 
> We simply need to update -device FOO,help code to use both qdev and QOM
> properties.  Note that types change because a 'drive' qdev type is actually a
> 'str' QOM type.  We're moving more and more to QOM properties where the final
> type for this property would be 'link<Drive>' or similar.
> 
> Cole: please confirm that this fixes the issue
> 
> Stefan Hajnoczi (2):
>   qmp: hide "hotplugged" device property from device-list-properties
>   qdev-monitor: include QOM properties in -device FOO,help output
> 
>  qdev-monitor.c | 40 +++++++++++++++++-----------------------
>  qmp.c          |  1 +
>  2 files changed, 18 insertions(+), 23 deletions(-)

CCed qemu-stable since we ought to fix -device FOO,?.  This patch was
missed for QEMU 2.1 but not critical (see Cole's response).

Applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]