Re: [Qemu-block] [Qemu-devel] [PATCH v14 12/21] option: allow qemu_opts_to_qdict to merge repeated options

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> On Wed, Oct 12, 2016 at 07:46:00PM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <address@hidden> writes:
>> 
>> > If given an option string such as
>> >
>> >   size=1024,nodes=10,nodes=4-5,nodes=1-2,policy=bind
>> >
>> > the qemu_opts_to_qdict() method will currently overwrite
>> > the values for repeated option keys, so only the last
>> > value is in the returned dict:
>> >
>> >     size=QString("1024")
>> >     nodes=QString("1-2")
>> >     policy=QString("bind")
>> >
>> > With this change the caller can optionally ask for all
>> > the repeated values to be stored in a QList. In the
>> > above example that would result in 'nodes' being a
>> > QList, so the returned dict would contain
>> >
>> >     size=QString("1024")
>> >     nodes=QList([QString("10"),
>> >                  QString("4-5"),
>> >                  QString("1-2")])
>> >     policy=QString("bind")
>> >
>> > Note that the conversion has no way of knowing whether
>> > any given key is expected to be a list upfront - it can
>> > only figure that out when seeing the first duplicated
>> > key. Thus the caller has to be prepared to deal with the
>> > fact that if a key 'foo' is a list, then the returned
>> > qdict may contain either a QString or a QList for the
>> > key 'foo'.
>> >
>> > In a third mode, it is possible to ask for repeated
>> > options to be reported as an error, rather than silently
>> > dropping all but the last one.
>> 
>> To serve as a replacement for the options visitor, this needs to be able
>> to behave exactly the same together with a suitably hacked up QObject
>> input visitor.  Before I dive into the actual patch, let me summarize
>> QemuOpts and options visitor behavior.
[...]
>> Unlike the options visitor, this patch (judging from your description)
>> makes a list only when keys are repeated.  The QObject visitor will have
>> to cope with finding both scalars and lists.  When it finds a scalar,
>> but needs a list, it'll have to wrap it in a list (PATCH 09, I think).
>> When it finds a list, but needs a scalar, it'll have to fish it out of
>> the list (where is that?).
>
> If my code finds a list but wants a scalar, it is reporting an
> error.

I'm afraid that breaks things like

    --numa node,cpus=0,cpus=2,mem=512,nodeid=0,nodeid=0

The options visitor inteprets cpus=0,cpus=2 as a list [0,2] (because
NumaNodeOptions member @cpus has a list type) and nodeid=0,nodid=0 as
integer 0 (because member @nodeid has a scalar type).