Re: [PATCH v3 1/3] qmp: remove virtio_list, search QOM tree instead

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Thu, 03 Aug 2023 17:54, Jonah Palmer <jonah.palmer@oracle.com> wrote:
>-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 */
>+        gchar *path = object_get_canonical_path(dev);

(You can use g_autofree char * here)

>+
>+        info->path = g_strdup(path);
>+        info->name = g_strdup(vdev->name);
>+        QAPI_LIST_PREPEND(*vdevs, info);
> 
>-    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);
>+        g_free(path);
>+    } else {
>+        object_unref(dev);
>     }

The object_unref should not happen only in the else branch, no? Though 
it's not clear to me where the ref count was previously incremented.

>+    object_child_foreach_recursive(object_get_root(), query_dev_child, 
>&vdevs);
>+    if (vdevs == NULL) {
>+        error_setg(errp, "No virtio devices found");
>+        return NULL;

(No need for early return here)

>     }
>-    return NULL;
>+    return vdevs;
>+}
>+
>+VirtIODevice *qmp_find_virtio_device(const char *path)
>+{
>+    /* 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) {
>+        object_unref(dev);

Same as before with object refs