"Daniel P. Berrange" <berra...@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?). > All existing callers are all converted to explicitly > request the historical behaviour of only reporting the > last key. Later patches will make use of the new modes. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Out of steam for today.