Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio
On 12/7/22 08:22, Markus Armbruster wrote:
Jonah Palmer writes:
On 12/2/22 10:21, Markus Armbruster wrote:
Philippe Mathieu-Daudé writes:
On 2/12/22 13:23, Jonah Palmer wrote:
On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:
Hi,
On 11/8/22 14:24, Jonah Palmer wrote:
From: Laurent Vivier
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).
How could this happen?
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
Signed-off-by: Jonah Palmer
---
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(>listener, vdev->dma_as);
+ QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
+ DeviceState *dev = DEVICE(vdev);
+ Error *err = NULL;
+ QObject *obj = qmp_qom_get(dev->canonical_path, "realized", );
+
+ 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()?
This check queries the QOM composition tree to check that the device actually
exists and is
realized. In other words, we just want to confirm with the QOM composition tree
for the device.
Again, how could this happen?
If @virtio_list isn't reliable, why have it in the first place?
Honestly, I'm not sure how this even could happen, since the @virtio_list is
managed at the realization
and unrealization of a virtio device. Given this, I do feel as though the list
is reliable, although
this might just benaïve of me to say. After giving this a second
Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio
Jonah Palmer writes:
> On 12/2/22 10:21, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé writes:
>>
>>> On 2/12/22 13:23, Jonah Palmer wrote:
On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:
> Hi,
>
> On 11/8/22 14:24, Jonah Palmer wrote:
>> From: Laurent Vivier
>>
>> 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).
>>
>> How could this happen?
>>
>> 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
>> Signed-off-by: Jonah Palmer
>> ---
>> 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(>listener, vdev->dma_as);
>> + QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
>> + DeviceState *dev = DEVICE(vdev);
>> + Error *err = NULL;
>> + QObject *obj = qmp_qom_get(dev->canonical_path, "realized",
>> );
>> +
>> + if (err == NULL) {
>> + GString *is_realized = qobject_to_json_pretty(obj, true);
>> + /* virtio device is NOT
Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio
On 12/2/22 10:21, Markus Armbruster wrote:
Philippe Mathieu-Daudé writes:
On 2/12/22 13:23, Jonah Palmer wrote:
On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:
Hi,
On 11/8/22 14:24, Jonah Palmer wrote:
From: Laurent Vivier
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).
How could this happen?
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
Signed-off-by: Jonah Palmer
---
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(>listener, vdev->dma_as);
+ QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
+ DeviceState *dev = DEVICE(vdev);
+ Error *err = NULL;
+ QObject *obj = qmp_qom_get(dev->canonical_path, "realized", );
+
+ 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()?
This check queries the QOM composition tree to check that the device actually
exists and is
realized. In other words, we just want to confirm with the QOM composition tree
for the device.
Again, how could this happen?
If @virtio_list isn't reliable, why have it in the first place?
Honestly, I'm not sure how this even could happen, since the @virtio_list is
managed at the realization
and unrealization of a virtio device. Given this, I do feel as though the list
is reliable, although
this might just benaïve of me to say. After giving this a second look, the @virtio_list is
only really needed to provide a nice list of all realized virtio devices
on the system, along with
Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio
Philippe Mathieu-Daudé writes:
> On 2/12/22 13:23, Jonah Palmer wrote:
>> On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:
>>> Hi,
>>>
>>> On 11/8/22 14:24, Jonah Palmer wrote:
From: Laurent Vivier
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).
How could this happen?
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
Signed-off-by: Jonah Palmer
---
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(>listener, vdev->dma_as);
+ QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
+ DeviceState *dev = DEVICE(vdev);
+ Error *err = NULL;
+ QObject *obj = qmp_qom_get(dev->canonical_path, "realized", );
+
+ 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()?
>>
>> This check queries the QOM composition tree to check that the device
>> actually exists and is
>> realized. In other words, we just want to confirm with the QOM composition
>> tree for the device.
Again, how could this happen?
Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio
On 2/12/22 13:23, Jonah Palmer wrote:
On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:
Hi,
On 11/8/22 14:24, Jonah Palmer wrote:
From: Laurent Vivier
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
Signed-off-by: Jonah Palmer
---
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(>listener, vdev->dma_as);
+ QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
+ DeviceState *dev = DEVICE(vdev);
+ Error *err = NULL;
+ QObject *obj = qmp_qom_get(dev->canonical_path, "realized",
);
+
+ 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()?
This check queries the QOM composition tree to check that the device actually
exists and is
realized. In other words, we just want to confirm with the QOM composition tree
for the device.
This was done to have some kind of synchronicity between @virtio_list and the
QOM composition
tree, since the list duplicates information that already exists in the tree.
This check was recommended in v10 and added in v11 of this patch series.
Thanks, I found Markus comments:
v10:
https://lore.kernel.org/qemu-devel/87ee6ogbiw@dusky.pond.sub.org/
v11:
https://lore.kernel.org/qemu-devel/87ee4abtdu@pond.sub.org/
Having the justification from v11 in the code rather than the commit
description could help.
+ if (!strncmp(is_realized->str, "false", 4)) {
+ QTAILQ_REMOVE(_list, vdev, next);
+
Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio
On 11/30/22 11:16, Philippe Mathieu-Daudé wrote:
Hi,
On 11/8/22 14:24, Jonah Palmer wrote:
From: Laurent Vivier
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
Signed-off-by: Jonah Palmer
---
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(>listener, vdev->dma_as);
+ QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
+ DeviceState *dev = DEVICE(vdev);
+ Error *err = NULL;
+ QObject *obj = qmp_qom_get(dev->canonical_path, "realized",
);
+
+ 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()?
This check queries the QOM composition tree to check that the device actually
exists and is
realized. In other words, we just want to confirm with the QOM composition tree
for the device.
This was done to have some kind of synchronicity between @virtio_list and the
QOM composition
tree, since the list duplicates information that already exists in the tree.
This check was recommended in v10 and added in v11 of this patch series.
+ if (!strncmp(is_realized->str, "false", 4)) {
+ QTAILQ_REMOVE(_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);
+
Re: [PATCH v15 1/6] qmp: add QMP command x-query-virtio
Hi,
On 11/8/22 14:24, Jonah Palmer wrote:
From: Laurent Vivier
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
Signed-off-by: Jonah Palmer
---
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(>listener, vdev->dma_as);
+QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
+DeviceState *dev = DEVICE(vdev);
+Error *err = NULL;
+QObject *obj = qmp_qom_get(dev->canonical_path, "realized", );
+
+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(_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;
+}
[PATCH v15 1/6] qmp: add QMP command x-query-virtio
From: Laurent Vivier
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
Signed-off-by: Jonah Palmer
---
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/meson.build b/hw/virtio/meson.build
index 7e8877fd64..e16f1b22d4 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -60,4 +60,6 @@ virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true:
virtio_pci_ss)
specific_ss.add_all(when: 'CONFIG_VIRTIO', if_true: virtio_ss)
softmmu_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss)
softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO', if_false: files('virtio-stub.c'))
softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c'))
+softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('virtio-stub.c'))
diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
new file mode 100644
index 00..05a81edc92
--- /dev/null
+++ b/hw/virtio/virtio-stub.c
@@ -0,0 +1,14 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-virtio.h"
+
+static void *qmp_virtio_unsupported(Error **errp)
+{
+error_setg(errp, "Virtio is disabled");
+return NULL;
+}
+
+VirtioInfoList *qmp_x_query_virtio(Error **errp)
+{
+return qmp_virtio_unsupported(errp);
+}
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(>listener, vdev->dma_as);
+QTAILQ_INSERT_TAIL(_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(_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(_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, _list, next) {
+DeviceState *dev = DEVICE(vdev);
+Error *err = NULL;
+QObject *obj = qmp_qom_get(dev->canonical_path, "realized", );
+
+if (err == NULL) {
+GString *is_realized = qobject_to_json_pretty(obj, true);
+/* virtio device is NOT realized, remove
