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.pal...@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
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
