On 6/23/23 01:47, Michael S. Tsirkin wrote:
On Fri, Jun 09, 2023 at 09:20:39AM -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 search
the partial paths of '/machine/peripheral/' &
'/machine/peripheral-anon/' in the QOM composition tree for virtio
devices.
A device is found to be a valid virtio device if (1) its canonical path
is of 'TYPE_VIRTIO_DEVICE' and (2) the device has been realized.
[Jonah: In the previous commit I had written that a device is found to
be a valid virtio device if (1) it has a canonical path ending with
'virtio-backend'.
The code now determines if it's a virtio device by appending
'virtio-backend' (if needed) to a given canonical path and then
checking that path to see if the device is of type
'TYPE_VIRTIO_DEVICE'.
The patch also instead now checks to make sure it's a virtio device
before attempting to check whether the device is realized or not.]
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Could one of QMP maintainers comment on this please?
---
hw/virtio/virtio-qmp.c | 128 ++++++++++++++++++++++++++---------------
hw/virtio/virtio-qmp.h | 8 +--
hw/virtio/virtio.c | 6 --
3 files changed, 82 insertions(+), 60 deletions(-)
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index b5e1835299..e936cc8ce5 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -668,67 +668,101 @@ VirtioDeviceFeatures *qmp_decode_features(uint16_t device_id, uint64_t bitmap)
VirtioInfoList *qmp_x_query_virtio(Error **errp)
{
VirtioInfoList *list = NULL;
- VirtioInfo *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 */
- 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);
+ /* Query the QOM composition tree for virtio devices */
+ qmp_set_virtio_device_list("/machine/peripheral/", &list);
+ qmp_set_virtio_device_list("/machine/peripheral-anon/", &list);
How sure are we these will forever be the only two places where virtio
can live?
A virtio device will always be considered a peripheral device, right?
Since peripheral devices are input and/or output devices by definition.
+ if (list == NULL) {
+ error_setg(errp, "No virtio devices found");
+ return NULL;
+ }
+ return list;
+}
+
+/* qmp_set_virtio_device_list:
+ * @ppath: An incomplete peripheral path to search from.
+ * @list: A list of realized virtio devices.
+ * Searches a given incomplete peripheral path (e.g. '/machine/peripheral/'
+ * or '/machine/peripheral-anon/') for realized virtio devices and adds them
+ * to a given list of virtio devices.
+ */
+void qmp_set_virtio_device_list(const char *ppath, VirtioInfoList **list)
+{
+ ObjectPropertyInfoList *plist;
+ VirtioInfoList *node;
+ Error *err = NULL;
+
+ /* Search an incomplete path for virtio devices */
+ plist = qmp_qom_list(ppath, &err);
+ if (err == NULL) {
+ ObjectPropertyInfoList *start = plist;
+ while (plist != NULL) {
+ ObjectPropertyInfo *value = plist->value;
+ GString *path = g_string_new(ppath);
+ g_string_append(path, value->name);
+ g_string_append(path, "/virtio-backend");
+
+ /* Determine if full path is a realized virtio device */
+ VirtIODevice *vdev = qmp_find_virtio_device(path->str);
+ if (vdev != NULL) {
+ node = g_new0(VirtioInfoList, 1);
+ node->value = g_new(VirtioInfo, 1);
+ node->value->path = g_strdup(path->str);
+ node->value->name = g_strdup(vdev->name);
+ QAPI_LIST_PREPEND(*list, node->value);
}
- g_string_free(is_realized, true);
+ g_string_free(path, true);
+ plist = plist->next;
}
- qobject_unref(obj);
+ qapi_free_ObjectPropertyInfoList(start);
}
-
- return list;
}
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;
+ char *basename;
+
+ /* Append 'virtio-backend' to path if needed */
+ basename = g_path_get_basename(path);
+ if (strcmp(basename, "virtio-backend")) {
+ GString *temp = g_string_new(path);
+ char *last = strrchr(path, '/');
+ if (g_strcmp0(last, "/")) {
+ g_string_append(temp, "/virtio-backend");
+ } else {
+ g_string_append(temp, "virtio-backend");
}
+ path = g_strdup(temp->str);
+ g_string_free(temp, true);
+ }
I don't much like the string operations. We should be able to
check object types instead.
I don't either but in order for us to check if the object is a
virtio device type, we need to use the device's path ending
with '/virtio-backend'.
If there's a better method to checking this though, or perhaps
checking a different type, that doesn't involve string
manipulation, then I'm all for it.