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

[Kevin Wolf]

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.

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