On Fri, Oct 21, 2016 at 01:09:07PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berra...@redhat.com> writes: > > > On Wed, Oct 12, 2016 at 07:46:00PM +0200, Markus Armbruster wrote: > >> "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. > [...] > >> 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).
Right, but that command line syntax is arguably broken already - the app/user invoking QEMU is buggy, so I felt it was justifiable to change from silently ignoring the app error to telling them of their mistake. 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/ :|