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.
Kevin
