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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 13/03/21 14:28, Markus Armbruster wrote:
>> Kevin Wolf <kwolf@redhat.com> 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@redhat.com>
>>> Acked-by: Peter Krempa <pkrempa@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> ---
>>>   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?