On Tue, Sep 13, 2016 at 03:32:33PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" <berra...@redhat.com> writes: > > When using QemuOpts w/ qdict_crumple + QmpInputVisitor, you would > > do list of scalars in different manner 'foo.1=3,foo.2=5,foo.3=533' > > since this syntax is extendable to deal with arbitrary nesting of > > dicts + lists, where as the OptsVisitor syntax cannot be extended > > in a back-compatible manner. > > > > This series converts -object to use QmpInputVisitor and this is > > safe todo right now, since no currently created QOM object has > > a property which is a list of scalars and this the changed syntax > > is not going to affect any existing usage. > > Peeking at the patch... aha! Instead of having the options visitor > visit the QemuOpts, you convert the QemuOpts to a QDict, which you then > visit with your new visitor. Less efficient, because you have to build > and destroy the intermediate QDict. Not an issue when processing > configuration or even monitor commands, of course. > > I guess you convert to QDict so that the work of going from a > QemuOpts-style string using -drive conventions to a visit splits into > manageable chunks more easily, preferably into chunks that already > exist: parse string into QemuOpts, convert to QDict, crumple, visit, > destroy QDict.
FWIW, I did originally try to modify the QemuOpts visitor directly to support visiting nested data structure, but I basically ended up re-inventing alot of code from qdict and indeed having to use a qdict internally to QemuOpts to store intermediate bits. At that point I realized it was clearly silly to try to shoe horn it into QemuOpts visitor directly, since I was basically creating qdict_crumple() indirectly and then essentially making QemuOpts visitor have the same logic as QmpInputVisitor. > Still, I take the conversion as a signal that our data structures are > wrong. Wild idea: should QemuOpts use a QDict rather than a QTAILQ of > QemuOpt to store options? If QemuOpts used a QDict internally, then you avoid the first qemu_opts_to_qdict() call before qdict_crumple(), but everything else basically stays the same. So a minor improvement, but nothing ground-shaking. > The pipeline then becomes parse string into QemuOpts, clone its QDict, > crumple, visit, destroy QDict. Next step would be crumpling in place, > i.e. parse string into nested QemuOpts, get its QDict, visit. Mind, I'm > not demanding you to do that now, I'm just trying to figure out whether > this series is shoveling into or out of the QemuOpts hole :) Yes, I guess the interesting win would be if qemu_opts_do_parse() could directly populate a qdict in crumpled format from the start, thus avoiding the late qdict_crumple call altogether. > > Other args would have to be considered on a case-by-case basis > > to see if they rely on the magic list of scalars syntax used > > by OptsVisitor. Probably only a few of them need this. > > Obvious maintainer question: what would it take to get rid of the > options visitor entirely? Because this maintainer looks on additions to > his fiefdom much more kindly when they're balanced by subtractions. > > I guess all the uses that don't rely on the "list of scalars" magic are > easy prey: replace by conversion to intermediate QDict + your new > visitor. > > What about the ones that do rely on it? Which ones do? Any ideas on > how to change them so that they don't require a complete options > visitor? I guess you could have some code that re-processes the stored QemuOpt in-place, changing repeated keys such as 'foo=400,foo=342' into the preferred format 'foo.1=400,foo.2=342', at which point you can use the qdict_crumple facility directly. That's probably not even very hard. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|