diff mbox series

[v4,1/3] qmp: remove virtio_list, search QOM tree instead

Message ID 20230818171926.3136840-2-jonah.palmer@oracle.com (mailing list archive)
State New, archived
Headers show
Series qmp, vhost-user: Remove virtio_list & update virtio introspection | expand

Commit Message

Jonah Palmer Aug. 18, 2023, 5:19 p.m. UTC
The virtio_list duplicates information about virtio devices that already
exist in the QOM composition tree. Instead of creating this list of
realized virtio devices, search the QOM composition tree instead.

This patch modifies the QMP command qmp_x_query_virtio to instead
recursively search the QOM composition tree for devices of type
'TYPE_VIRTIO_DEVICE'. The device is also checked to ensure it's
realized.

Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
---

 Jonah: In the v2 patches, the qmp_x_query_virtio function was
 iterating through devices found via. qmp_qom_list and appending
 "/virtio-backend" to devices' paths to check if they were a virtio
 device.

 This method was messy and involved unneeded string manipulation.

 Instead, we can use recursion with object_get_root to iterate through
 all parent and child device paths to find virtio devices.

 The qmp_find_virtio_device function was also updated to simplify the
 method of determining if a path is to a valid and realized virtio
 device.

 hw/virtio/virtio-qmp.c | 88 +++++++++++++++---------------------------
 hw/virtio/virtio-qmp.h |  7 ----
 hw/virtio/virtio.c     |  6 ---
 3 files changed, 32 insertions(+), 69 deletions(-)

Comments

Daniel P. Berrangé Aug. 21, 2023, 10:05 a.m. UTC | #1
On Fri, Aug 18, 2023 at 01:19:24PM -0400, Jonah Palmer wrote:
> The virtio_list duplicates information about virtio devices that already
> exist in the QOM composition tree. Instead of creating this list of
> realized virtio devices, search the QOM composition tree instead.
> 
> This patch modifies the QMP command qmp_x_query_virtio to instead
> recursively search the QOM composition tree for devices of type
> 'TYPE_VIRTIO_DEVICE'. The device is also checked to ensure it's
> realized.
> 
> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> ---
> 
>  Jonah: In the v2 patches, the qmp_x_query_virtio function was
>  iterating through devices found via. qmp_qom_list and appending
>  "/virtio-backend" to devices' paths to check if they were a virtio
>  device.
> 
>  This method was messy and involved unneeded string manipulation.
> 
>  Instead, we can use recursion with object_get_root to iterate through
>  all parent and child device paths to find virtio devices.
> 
>  The qmp_find_virtio_device function was also updated to simplify the
>  method of determining if a path is to a valid and realized virtio
>  device.
> 
>  hw/virtio/virtio-qmp.c | 88 +++++++++++++++---------------------------
>  hw/virtio/virtio-qmp.h |  7 ----
>  hw/virtio/virtio.c     |  6 ---
>  3 files changed, 32 insertions(+), 69 deletions(-)
> 
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index 7515b0947b..ac5f0ee0ee 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -667,70 +667,46 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
>      return features;
>  }
>  
> -VirtioInfoList *qmp_x_query_virtio(Error **errp)
> +static int query_dev_child(Object *child, void *opaque)
>  {
> -    VirtioInfoList *list = NULL;
> -    VirtioInfo *node;
> -    VirtIODevice *vdev;
> +    VirtioInfoList **vdevs = opaque;
> +    Object *dev = object_dynamic_cast(child, TYPE_VIRTIO_DEVICE);
> +    if (dev != NULL && DEVICE(dev)->realized) {
> +        VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +
> +        VirtioInfo *info = g_new(VirtioInfo, 1);
> +
> +        /* Get canonical path of device */
> +        g_autofree char *path = object_get_canonical_path(dev);
>  
> -    QTAILQ_FOREACH(vdev, &virtio_list, next) {
> -        DeviceState *dev = DEVICE(vdev);
> -        Error *err = NULL;
> -        QObject *obj = qmp_qom_get(dev->canonical_path, "realized", &err);
> -
> -        if (err == NULL) {
> -            GString *is_realized = qobject_to_json_pretty(obj, true);
> -            /* virtio device is NOT realized, remove it from list */
> -            if (!strncmp(is_realized->str, "false", 4)) {
> -                QTAILQ_REMOVE(&virtio_list, vdev, next);
> -            } else {
> -                node = g_new(VirtioInfo, 1);
> -                node->path = g_strdup(dev->canonical_path);
> -                node->name = g_strdup(vdev->name);
> -                QAPI_LIST_PREPEND(list, node);
> -            }
> -           g_string_free(is_realized, true);
> -        }
> -        qobject_unref(obj);
> +        info->path = g_strdup(path);

Just call object_get_canonical_path(dev) directly and avoid
duplicating & freeing the intermediate 'path' variable

> +        info->name = g_strdup(vdev->name);
> +        QAPI_LIST_PREPEND(*vdevs, info);
>      }
> +    return 0;
> +}
>  


With regards,
Daniel
diff mbox series

Patch

diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 7515b0947b..ac5f0ee0ee 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -667,70 +667,46 @@  VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
     return features;
 }
 
-VirtioInfoList *qmp_x_query_virtio(Error **errp)
+static int query_dev_child(Object *child, void *opaque)
 {
-    VirtioInfoList *list = NULL;
-    VirtioInfo *node;
-    VirtIODevice *vdev;
+    VirtioInfoList **vdevs = opaque;
+    Object *dev = object_dynamic_cast(child, TYPE_VIRTIO_DEVICE);
+    if (dev != NULL && DEVICE(dev)->realized) {
+        VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+        VirtioInfo *info = g_new(VirtioInfo, 1);
+
+        /* Get canonical path of device */
+        g_autofree char *path = object_get_canonical_path(dev);
 
-    QTAILQ_FOREACH(vdev, &virtio_list, next) {
-        DeviceState *dev = DEVICE(vdev);
-        Error *err = NULL;
-        QObject *obj = qmp_qom_get(dev->canonical_path, "realized", &err);
-
-        if (err == NULL) {
-            GString *is_realized = qobject_to_json_pretty(obj, true);
-            /* virtio device is NOT realized, remove it from list */
-            if (!strncmp(is_realized->str, "false", 4)) {
-                QTAILQ_REMOVE(&virtio_list, vdev, next);
-            } else {
-                node = g_new(VirtioInfo, 1);
-                node->path = g_strdup(dev->canonical_path);
-                node->name = g_strdup(vdev->name);
-                QAPI_LIST_PREPEND(list, node);
-            }
-           g_string_free(is_realized, true);
-        }
-        qobject_unref(obj);
+        info->path = g_strdup(path);
+        info->name = g_strdup(vdev->name);
+        QAPI_LIST_PREPEND(*vdevs, info);
     }
+    return 0;
+}
 
-    return list;
+VirtioInfoList *qmp_x_query_virtio(Error **errp)
+{
+    VirtioInfoList *vdevs = NULL;
+
+    /* Query the QOM composition tree recursively for virtio devices */
+    object_child_foreach_recursive(object_get_root(), query_dev_child, &vdevs);
+    if (vdevs == NULL) {
+        error_setg(errp, "No virtio devices found");
+    }
+    return vdevs;
 }
 
 VirtIODevice *qmp_find_virtio_device(const char *path)
 {
-    VirtIODevice *vdev;
-
-    QTAILQ_FOREACH(vdev, &virtio_list, next) {
-        DeviceState *dev = DEVICE(vdev);
-
-        if (strcmp(dev->canonical_path, path) != 0) {
-            continue;
-        }
-
-        Error *err = NULL;
-        QObject *obj = qmp_qom_get(dev->canonical_path, "realized", &err);
-        if (err == NULL) {
-            GString *is_realized = qobject_to_json_pretty(obj, true);
-            /* virtio device is NOT realized, remove it from list */
-            if (!strncmp(is_realized->str, "false", 4)) {
-                g_string_free(is_realized, true);
-                qobject_unref(obj);
-                QTAILQ_REMOVE(&virtio_list, vdev, next);
-                return NULL;
-            }
-            g_string_free(is_realized, true);
-        } else {
-            /* virtio device doesn't exist in QOM tree */
-            QTAILQ_REMOVE(&virtio_list, vdev, next);
-            qobject_unref(obj);
-            return NULL;
-        }
-        /* device exists in QOM tree & is realized */
-        qobject_unref(obj);
-        return vdev;
+    /* Verify the canonical path is a realized virtio device */
+    Object *dev = object_dynamic_cast(object_resolve_path(path, NULL),
+                                      TYPE_VIRTIO_DEVICE);
+    if (!dev || !DEVICE(dev)->realized) {
+        return NULL;
     }
-    return NULL;
+    return VIRTIO_DEVICE(dev);
 }
 
 VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
@@ -740,7 +716,7 @@  VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
 
     vdev = qmp_find_virtio_device(path);
     if (vdev == NULL) {
-        error_setg(errp, "Path %s is not a VirtIODevice", path);
+        error_setg(errp, "Path %s is not a realized VirtIODevice", path);
         return NULL;
     }
 
diff --git a/hw/virtio/virtio-qmp.h b/hw/virtio/virtio-qmp.h
index 8af5f5e65a..245a446a56 100644
--- a/hw/virtio/virtio-qmp.h
+++ b/hw/virtio/virtio-qmp.h
@@ -15,13 +15,6 @@ 
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 
-#include "qemu/queue.h"
-
-typedef QTAILQ_HEAD(QmpVirtIODeviceList, VirtIODevice) QmpVirtIODeviceList;
-
-/* QAPI list of realized VirtIODevices */
-extern QmpVirtIODeviceList virtio_list;
-
 VirtIODevice *qmp_find_virtio_device(const char *path);
 VirtioDeviceStatus *qmp_decode_status(uint8_t bitmap);
 VhostDeviceProtocols *qmp_decode_protocols(uint64_t bitmap);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 309038fd46..6cca41ff4f 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -45,8 +45,6 @@ 
 #include "standard-headers/linux/virtio_mem.h"
 #include "standard-headers/linux/virtio_vsock.h"
 
-QmpVirtIODeviceList virtio_list;
-
 /*
  * Maximum size of virtio device config space
  */
@@ -3616,7 +3614,6 @@  static void virtio_device_realize(DeviceState *dev, Error **errp)
     vdev->listener.commit = virtio_memory_listener_commit;
     vdev->listener.name = "virtio";
     memory_listener_register(&vdev->listener, vdev->dma_as);
-    QTAILQ_INSERT_TAIL(&virtio_list, vdev, next);
 }
 
 static void virtio_device_unrealize(DeviceState *dev)
@@ -3631,7 +3628,6 @@  static void virtio_device_unrealize(DeviceState *dev)
         vdc->unrealize(dev);
     }
 
-    QTAILQ_REMOVE(&virtio_list, vdev, next);
     g_free(vdev->bus_name);
     vdev->bus_name = NULL;
 }
@@ -3805,8 +3801,6 @@  static void virtio_device_class_init(ObjectClass *klass, void *data)
     vdc->stop_ioeventfd = virtio_device_stop_ioeventfd_impl;
 
     vdc->legacy_features |= VIRTIO_LEGACY_FEATURES;
-
-    QTAILQ_INIT(&virtio_list);
 }
 
 bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)