Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/03/21 18:30, Kevin Wolf wrote:
> Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben:
>> On 10/03/21 15:22, Peter Krempa wrote:
>>> I've stumbled upon a regression with this patchset applied:
>>>
>>> error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -object memory-backend-ram,id=pc.ram,size=1048576000,host-nodes=0,policy=bind: Invalid parameter type for 'host-nodes', expected: array
>>
>> This is the magic conversion of "string containing a list of integers"
>> to "list of integers".
> 
> Ah, crap. This one wouldn't have been a problem when converting only
> object-add, and I trusted your audit that user creatable objects don't
> depend on any QemuOpts magic. I should have noticed it, too, of course,
> but during the convertion I didn't have QemuOpts in mind, only QOM and
> QAPI.

Yeah, let's just drop the -object conversion for now. It will just 
remove a few patches.

Who is going to include this series in the next pull request, Markus or 
myself?  The time is ticking for soft freeze.

Paolo

> I checked all object types, and it seems this is the only one that is
> affected. We have a second list in AuthZListProperties, but it contains
> structs, so it was never supported on the command line anyway.
> 
>> The relevant code is in qapi/string-input-visitor.c, but I'm not sure where
>> to stick it in the keyval-based parsing flow (i.e.
>> qobject_input_visitor_new_keyval).  Markus, any ideas?
> 
> The best I can think of at the moment would be adding a flag to the
> keyval parser that would enable the feature only for -object (and only
> in the system emulator, because memory-backend-ram doesn't exist in the
> tools):
> 
> The keyval parser would create a list if multiple values are given for
> the same key. Some care needs to be taken to avoid mixing the magic
> list feature with the normal indexed list options.
> 
> The QAPI schema would then use an alternate between 'int' and ['int'],
> with the the memory-backend-ram implementation changed accordingly.
> 
> We could consider immediately deprecating the syntax and printing a
> warning in the keyval parser when it automatically creates a list from
> two values for a key, so that users don't start using this syntax
> instead of the normal list syntax in other places. We'd probably still
> leave the implementation around for -device and other users of the same
> magic.
> 
> Kevin
>