Re: [PATCH v3 27/30] hmp: QAPIfy object_add

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15/03/21 12:38, Dr. David Alan Gilbert wrote:
* Kevin Wolf (kwolf@xxxxxxxxxx) wrote:
Am 15.03.2021 um 10:39 hat Markus Armbruster geschrieben:
Paolo Bonzini <pbonzini@xxxxxxxxxx> writes:

On 13/03/21 14:28, Markus Armbruster wrote:
Kevin Wolf <kwolf@xxxxxxxxxx> writes:

This switches the HMP command object_add from a QemuOpts-based parser to
user_creatable_add_from_str() which uses a keyval parser and enforces
the QAPI schema.

Apart from being a cleanup, this makes non-scalar properties and help
accessible. In order for help to be printed to the monitor instead of
stdout, the printf() calls in the help functions are changed to
qemu_printf().

Signed-off-by: Kevin Wolf <kwolf@xxxxxxxxxx>
Acked-by: Peter Krempa <pkrempa@xxxxxxxxxx>
Reviewed-by: Eric Blake <eblake@xxxxxxxxxx>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@xxxxxxxxxx>
---
   monitor/hmp-cmds.c      | 17 ++---------------
   qom/object_interfaces.c | 11 ++++++-----
   hmp-commands.hx         |  2 +-
   3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 3c88a4faef..652cf9ff21 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
   {
+    const char *options = qdict_get_str(qdict, "object");
       Error *err = NULL;
-    QemuOpts *opts;
-    Object *obj = NULL;
-
-    opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
-    if (err) {
-        goto end;
-    }
- obj = user_creatable_add_opts(opts, &err);
-    qemu_opts_del(opts);
-
-end:
+    user_creatable_add_from_str(options, &err);
       hmp_handle_error(mon, err);
-
-    if (obj) {
-        object_unref(obj);
-    }
   }

Doesn't this break the list-valued properties (Memdev member host-nodes,
NumaNodeOptions member cpus) exactly the same way that made us keep
QemuOpts for qemu-system-FOO -object?

Yes, it does.  I guess it can just be documented, unlike for the command
line?

Maybe.  Judgement call, not mine to make.

Do people create such objects in HMP?  I figure we don't really know.
Educated guess?

If you try, how does it break?  Is it confusing?  Can you show an
example?

(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=0
Error: Invalid parameter type for 'host-nodes', expected: array
(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes.0=0
(qemu)

HMP is not a stable interface, so changing the syntax didn't feel like a
problem to me. I doubt many people do HMP memory hotplug while setting a
specific NUMA policy, but it wouldn't change my assessment anyway. I
should have made this explicit in the commit message, though.

I'm OK for it to change, but yes I'd like to have the before/after
syntax listed somewhere as easy references for people confused.

I think we should try to improve the string-value QObject visitor to also allow JSON values in some places, for example to allow

object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=[0,1,2,3]

Paolo




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux