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

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Oct 12, 2016 at 07:46:00PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> 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.
> 
> Warning, this is going to get ugly.
> 
> QemuOpts faithfully represents a key=value,... string as a list of
> QemuOpt.  Each QemuOpt represents one key=value.  They are in the same
> order.  If key occurs multiple times in the string, it occurs just the
> same in the list.
> 
> *Except* key "id" is special: it's stored outside the list, and all but
> the first one are silently ignored.
> 
> Most users only ever get the last value of a key.  Any non-last
> key=value are silently ignored.
> 
> We actually exploit this behavior to do defaults, by *prepending* them
> to the list.  See the use of qemu_opts_set_defaults() in main().
> 
> A few users get all values of keys (other than key "id"):
> 
> * -device, in qdev_device_add() with callback set_property().
> 
>   We first get "driver" and "bus" normally (silently ignoring non-last
>   values, as usual).  All other keys are device properties.  To set
>   them, we get all (key, value), ignore keys "driver" and "bus", and set
>   the rest.  If a key occurs multiple times, it gets set multiple times.
>   This effectively ignores all but the last one, silently.
> 
> * -semihosting-config, in main() with callback add_semihosting_arg().
> 
>   We first get a bunch of keys normally.  Key "arg" is special: it may
>   be repeated to build a list.  To implement that, we get all (key,
>   value), ignore keys other than "arg", and accumulate the values.
> 
> * -machine & friends, in main() with callback machine_set_property()
> 
>   Similar to -device, only for machines, with "type" instead of "driver"
>   and "bus".
> 
> * -spice, in qemu_spice_init() with callback add_channel()
> 
>   Keys "tls-channel" and "plaintext-channel" may be used repeated to
>   specify multiple channels.  To implement that, we get all (key,
>   value), ignore keys other than "tls-channel" and "plaintext-channel",
>   and set up a channel for each of the others.
> 
> * -writeconfig, in config_write_opts() with callback config_write_opt()
> 
>   We write out all keys in order.
> 
> * The options visitor, in opts_start_struct()
> 
>   We convert the list of (key, value) to a hash table of (key, list of
>   values).  Most of the time, the list of values has exactly one
>   element.
> 
>   When the visitor's user asks for a scalar, we return the last element
>   of the list of values, in lookup_scalar().
> 
>   When the user asks for list elements, we return the elements of the
>   list of values in order, in opts_next_list(), or if there are none,
>   the empty list in opts_start_list().
> 
> 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.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|