Re: [PATCH 08/25] hmp: replace "O" parser with keyval

[Paolo Bonzini]

On 25/01/21 10:00, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > HMP is using QemuOpts to parse free-form commands device_add,
> > netdev_add and object_add.  However, none of these need QemuOpts
> > for validation (these three QemuOptsLists are all of the catch-all
> > kind), and keyval is already able to parse into QDict.  So use
> > keyval directly, avoiding the detour from
> > string to QemuOpts to QDict.
> >
> > The args_type now stores the implied key.  This arguably makes more
> > sense than storing the QemuOptsList name; at least, it _is_ a key
> > that might end up in the arguments QDict.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Switching from QemuOpts to keyval changes the accepted language.  We may
> change it, because HMP is not a stable interface.  The commit message
> should point out the change, though.  Maybe even release notes, not
> sure.
>
> Let's recap the differences briefly:
>
> * Boolean sugar: deprecated in QemuOpts, nonexistent in keyval
>
> * QemuOpts accepts a number of more or less crazy corner cases keyval
>    rejects: invalid key, long key (silently truncated), first rather than
>    last id= wins (unlike other keys), implied key with empty value.
>
> * QemuOpts rejects anti-social ID such as id=666, keyval leaves this to
>    the caller, because key "id" is not special in keyval.
>
>    Are these still rejected with your patch?

Back here... No, and that's a feature.  There's no reason to reject 
those ids.  However, this shows that Kevin's series to move --object to 
keyval propagates a bug from qemu-storage-daemon to QEMU:

$ storage-daemon/qemu-storage-daemon --object 
authz-simple,id=123/546,identity=abc --chardev stdio,id=foo --monitor 
chardev=foo
> {'execute':'qmp_capabilities'}
> {'execute':'qom-list', 'arguments': {'path':'/objects'}}
< {"return": [{"name": "type", "type": "string"}, {"name": "123/546", 
"type": "child<authz-simple>"}]}

Good luck using that object anywhere. :)

> * device_add help,e1000
>
>      {
>          "e1000": "on",
>          "driver": "help"
>      }
>
>    Afterwards:
>    upstream-qemu: ../util/error.c:59: error_setv: Assertion `*errp == NULL' 
> failed.

I cannot reproduce it:

$ ./qemu-system-x86_64 -M none -monitor stdio -display none
QEMU 5.2.50 monitor - type 'help' for more information
(qemu) device_add help,e1000
Error: Parameter 'driver' is missing

Paolo