[PATCH v3 2/2] vl: use qmp_device_add() in qemu_create_cli_devices()

Stefan Hajnoczi posted 2 patches 2 months, 4 weeks ago
[PATCH v3 2/2] vl: use qmp_device_add() in qemu_create_cli_devices()
Posted by Stefan Hajnoczi 2 months, 4 weeks ago
qemu_create_cli_devices() should use qmp_device_add() to match the
behavior of the QMP monitor. A comment explained that libvirt changes
implementing strict CLI syntax were needed.

Peter Krempa <pkrempa@redhat.com> has confirmed that modern libvirt uses
the same JSON for -device (CLI) and device_add (QMP). Go ahead and use
qmp_device_add().

Cc: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 system/vl.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index 01b8b8e77a..3db63af764 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2652,17 +2652,11 @@ static void qemu_create_cli_devices(void)
     qemu_opts_foreach(qemu_find_opts("device"),
                       device_init_func, NULL, &error_fatal);
     QTAILQ_FOREACH(opt, &device_opts, next) {
-        DeviceState *dev;
+        QObject *ret_data = NULL;
+
         loc_push_restore(&opt->loc);
-        /*
-         * TODO Eventually we should call qmp_device_add() here to make sure it
-         * behaves the same, but QMP still has to accept incorrectly typed
-         * options until libvirt is fixed and we want to be strict on the CLI
-         * from the start, so call qdev_device_add_from_qdict() directly for
-         * now.
-         */
-        dev = qdev_device_add_from_qdict(opt->opts, true, &error_fatal);
-        object_unref(OBJECT(dev));
+        qmp_device_add(opt->opts, &ret_data, &error_fatal);
+        assert(ret_data == NULL); /* error_fatal aborts */
         loc_pop(&opt->loc);
     }
     rom_reset_order_override();
-- 
2.46.0
Re: [PATCH v3 2/2] vl: use qmp_device_add() in qemu_create_cli_devices()
Posted by Daniel P. Berrangé 2 months, 3 weeks ago
On Tue, Aug 27, 2024 at 03:27:51PM -0400, Stefan Hajnoczi wrote:
> qemu_create_cli_devices() should use qmp_device_add() to match the
> behavior of the QMP monitor. A comment explained that libvirt changes
> implementing strict CLI syntax were needed.
> 
> Peter Krempa <pkrempa@redhat.com> has confirmed that modern libvirt uses
> the same JSON for -device (CLI) and device_add (QMP). Go ahead and use
> qmp_device_add().
> 
> Cc: Peter Krempa <pkrempa@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  system/vl.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|