[libvirt] [RFC PATCH v3 3/9] qemu: Prefer qom-list-properties to device-list-properties

Andrea Bolognani posted 9 patches 7 years, 2 months ago
[libvirt] [RFC PATCH v3 3/9] qemu: Prefer qom-list-properties to device-list-properties
Posted by Andrea Bolognani 7 years, 2 months ago
The former command is only implemented in the very latest QEMU
version, but it's a proper superset of the latter and can thus
be used as a drop-in replacement when available.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_capabilities.c                       |  10 +-
 src/qemu/qemu_monitor.c                            |   5 +-
 src/qemu/qemu_monitor.h                            |   1 +
 src/qemu/qemu_monitor_json.c                       |  13 +-
 src/qemu/qemu_monitor_json.h                       |   3 +-
 .../caps_2.12.0-gicv2.aarch64.replies              | 364 +++++++++++++-
 .../caps_2.12.0-gicv2.aarch64.xml                  |   2 +-
 .../caps_2.12.0-gicv3.aarch64.replies              | 364 +++++++++++++-
 .../caps_2.12.0-gicv3.aarch64.xml                  |   2 +-
 .../qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 384 ++++++++++++++-
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |   2 +-
 .../caps_2.12.0.x86_64.replies                     | 544 ++++++++++++++++++++-
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |   2 +-
 13 files changed, 1625 insertions(+), 71 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 42ede1db3e..fb6b492454 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2883,6 +2883,12 @@ virQEMUCapsProbeQMPObjects(virQEMUCapsPtr qemuCaps,
     int nvalues;
     char **values;
     size_t i;
+    const char *impl = "device-list-properties";
+
+    /* Prefer qom-list-properties if available, since it allows us to gather
+     * information about objects that device-list-properties doesn't support */
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QOM_LIST_PROPERTIES))
+        impl = "qom-list-properties";
 
     if ((nvalues = qemuMonitorGetObjectTypes(mon, &values)) < 0)
         return -1;
@@ -2899,9 +2905,7 @@ virQEMUCapsProbeQMPObjects(virQEMUCapsPtr qemuCaps,
         if (cap >= 0 && !virQEMUCapsGet(qemuCaps, cap))
             continue;
 
-        if ((nvalues = qemuMonitorGetObjectProps(mon,
-                                                 type,
-                                                 &values)) < 0)
+        if ((nvalues = qemuMonitorGetObjectProps(mon, impl, type, &values)) < 0)
             return -1;
         virQEMUCapsProcessStringFlags(qemuCaps,
                                       virQEMUCapsObjectProps[i].nprops,
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ad5c572aee..56aaffc632 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3943,14 +3943,15 @@ qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
 
 int
 qemuMonitorGetObjectProps(qemuMonitorPtr mon,
+                          const char *impl,
                           const char *type,
                           char ***props)
 {
-    VIR_DEBUG("type=%s props=%p", type, props);
+    VIR_DEBUG("impl=%s type=%s props=%p", impl, type, props);
 
     QEMU_CHECK_MONITOR_JSON(mon);
 
-    return qemuMonitorJSONGetObjectProps(mon, type, props);
+    return qemuMonitorJSONGetObjectProps(mon, impl, type, props);
 }
 
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 954ae88e4f..45ead25b7a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1083,6 +1083,7 @@ int qemuMonitorGetKVMState(qemuMonitorPtr mon,
 int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
                               char ***types);
 int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
+                              const char *impl,
                               const char *type,
                               char ***props);
 char *qemuMonitorGetTargetArch(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a09e93e464..ddeb27587b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6023,6 +6023,7 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
 
 
 int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
+                                  const char *impl,
                                   const char *type,
                                   char ***props)
 {
@@ -6036,7 +6037,7 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
 
     *props = NULL;
 
-    if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
+    if (!(cmd = qemuMonitorJSONMakeCommand(impl,
                                            "s:typename", type,
                                            NULL)))
         return -1;
@@ -6054,8 +6055,9 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
 
     if (!(data = virJSONValueObjectGetArray(reply, "return")) ||
         (n = virJSONValueArraySize(data)) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("device-list-properties reply data was not an array"));
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("%s reply data was not an array"),
+                       impl);
         goto cleanup;
     }
 
@@ -6068,8 +6070,9 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
         const char *tmp;
 
         if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("device-list-properties reply data was missing 'name'"));
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("%s reply data was missing 'name'"),
+                           impl);
             goto cleanup;
         }
 
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ec243becc4..13ca49efd6 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -442,9 +442,10 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
     ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
 
 int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
+                                  const char *impl,
                                   const char *type,
                                   char ***props)
-    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
 char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon);
 
 int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon,
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies
index 2bccdaf586..bc591b24e9 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.replies
@@ -3402,10 +3402,18 @@
       "description": "on/off",
       "type": "bool"
     },
+    {
+      "name": "legacy-addr",
+      "type": "str"
+    },
     {
       "name": "min_io_size",
       "type": "uint16"
     },
+    {
+      "name": "type",
+      "type": "string"
+    },
     {
       "name": "event_idx",
       "description": "on/off",
[...]
@@ -4075,7 +4147,7 @@
   "id": "libvirt-13",
   "error": {
     "class": "DeviceNotFound",
-    "desc": "Device 'virtio-blk-ccw' not found"
+    "desc": "Class 'virtio-blk-ccw' not found"
   }
 }
 
@@ -4083,7 +4155,7 @@
   "id": "libvirt-14",
   "error": {
     "class": "DeviceNotFound",
-    "desc": "Device 'virtio-net-ccw' not found"
+    "desc": "Class 'virtio-net-ccw' not found"
   }
 }
 
@@ -4091,7 +4163,7 @@
   "id": "libvirt-15",
   "error": {
     "class": "DeviceNotFound",
-    "desc": "Device 'virtio-scsi-ccw' not found"
+    "desc": "Class 'virtio-scsi-ccw' not found"
   }
 }
[...]
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
index b4e5d0eddf..f6cd790480 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0-gicv2.aarch64.xml
@@ -190,7 +190,7 @@
   <flag name='qom-list-properties'/>
   <version>2011050</version>
   <kvmVersion>0</kvmVersion>
-  <microcodeVersion>312776</microcodeVersion>
+  <microcodeVersion>317921</microcodeVersion>
   <package> (v2.11.0-1777-g2e0cc0f)</package>
   <arch>aarch64</arch>
   <cpu type='kvm' name='pxa262'/>
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v3 3/9] qemu: Prefer qom-list-properties to device-list-properties
Posted by Andrea Bolognani 7 years, 2 months ago
On Thu, 2018-03-01 at 19:03 +0100, Andrea Bolognani wrote:
> @@ -2883,6 +2883,12 @@ virQEMUCapsProbeQMPObjects(virQEMUCapsPtr qemuCaps,
>      int nvalues;
>      char **values;
>      size_t i;
> +    const char *impl = "device-list-properties";
> +
> +    /* Prefer qom-list-properties if available, since it allows us to gather
> +     * information about objects that device-list-properties doesn't support */
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QOM_LIST_PROPERTIES))
> +        impl = "qom-list-properties";
>
>      if ((nvalues = qemuMonitorGetObjectTypes(mon, &values)) < 0)
>          return -1;
> @@ -2899,9 +2905,7 @@ virQEMUCapsProbeQMPObjects(virQEMUCapsPtr qemuCaps,
>          if (cap >= 0 && !virQEMUCapsGet(qemuCaps, cap))
>              continue;
>
> -        if ((nvalues = qemuMonitorGetObjectProps(mon,
> -                                                 type,
> -                                                 &values)) < 0)
> +        if ((nvalues = qemuMonitorGetObjectProps(mon, impl, type, &values)) < 0)
>              return -1;

I should note that I don't love this approach: I would much rather
pass qemuCaps all the way down to qemuMonitorJSONGetObjectProps()
and make the decision there for better encapsulation.

However, qemu_capabilities depends on qemu_monitor, so that would
introduce a circular dependency and I'm not sure trying to break it
up is worth the effort.

So if anyone fells like weighing in on the subject, or provide
alternative suggestions, I'm all ears.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list