Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio

[Philippe Mathieu-Daudé]

Hi,

On 11/8/22 14:24, Jonah Palmer wrote:
From: Laurent Vivier <lviv...@redhat.com>
This new command lists all the instances of VirtIODevices with
their canonical QOM path and name.

[Jonah: @virtio_list duplicates information that already exists in
  the QOM composition tree. However, extracting necessary information
  from this tree seems to be a bit convoluted.

  Instead, we still create our own list of realized virtio devices
  but use @qmp_qom_get with the device's canonical QOM path to confirm
  that the device exists and is realized. If the device exists but
  is actually not realized, then we remove it from our list (for
  synchronicity to the QOM composition tree).

  Also, the QMP command @x-query-virtio is redundant as @qom-list
  and @qom-get are sufficient to search '/machine/' for realized
  virtio devices. However, @x-query-virtio is much more convenient
  in listing realized virtio devices.]

Signed-off-by: Laurent Vivier <lviv...@redhat.com>
Signed-off-by: Jonah Palmer <jonah.pal...@oracle.com>
---
  hw/virtio/meson.build      |  2 ++
  hw/virtio/virtio-stub.c    | 14 ++++++++
  hw/virtio/virtio.c         | 44 ++++++++++++++++++++++++
  include/hw/virtio/virtio.h |  1 +
  qapi/meson.build           |  1 +
  qapi/qapi-schema.json      |  1 +
  qapi/virtio.json           | 68 ++++++++++++++++++++++++++++++++++++++
  tests/qtest/qmp-cmd-test.c |  1 +
  8 files changed, 132 insertions(+)
  create mode 100644 hw/virtio/virtio-stub.c
  create mode 100644 qapi/virtio.json

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5d607aeaa0..bdfa82e9c0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -13,12 +13,18 @@
  
  #include "qemu/osdep.h"
  #include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qapi-commands-virtio.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-visit-virtio.h"
+#include "qapi/qmp/qjson.h"
  #include "cpu.h"
  #include "trace.h"
  #include "qemu/error-report.h"
  #include "qemu/log.h"
  #include "qemu/main-loop.h"
  #include "qemu/module.h"
+#include "qom/object_interfaces.h"
  #include "hw/virtio/virtio.h"
  #include "migration/qemu-file-types.h"
  #include "qemu/atomic.h"
@@ -29,6 +35,9 @@
  #include "sysemu/runstate.h"
  #include "standard-headers/linux/virtio_ids.h"
  
+/* QAPI list of realized VirtIODevices */
+static QTAILQ_HEAD(, VirtIODevice) virtio_list;
+
  /*
   * The alignment to use between consumer and producer parts of vring.
   * x86 pagesize again. This is the default, used by transports like PCI
@@ -3698,6 +3707,7 @@ 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)
@@ -3712,6 +3722,7 @@ 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;
  }
@@ -3885,6 +3896,8 @@ 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)
@@ -3895,6 +3908,37 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev)
      return virtio_bus_ioeventfd_enabled(vbus);
  }
  
+VirtioInfoList *qmp_x_query_virtio(Error **errp)
+{
+    VirtioInfoList *list = NULL;
+    VirtioInfoList *node;
+    VirtIODevice *vdev;
+
+    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 */

Why not check dev->realized instead of calling qmp_qom_get() & 
qobject_to_json_pretty()?

+            if (!strncmp(is_realized->str, "false", 4)) {
+                QTAILQ_REMOVE(&virtio_list, vdev, next);
+            } else {
+                node = g_new0(VirtioInfoList, 1);
+                node->value = g_new(VirtioInfo, 1);
+                node->value->path = g_strdup(dev->canonical_path);
+                node->value->name = g_strdup(vdev->name);
+                QAPI_LIST_PREPEND(list, node->value);
+            }
+           g_string_free(is_realized, true);
+        }
+        qobject_unref(obj);
+    }
+
+    return list;
+}