[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does s
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion |
Date: |
Tue, 13 Sep 2016 14:47:34 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
On Tue, Sep 13, 2016 at 03:32:33PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <address@hidden> 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 :|
- Re: [Qemu-devel] [PATCH v11 1/6] qdict: implement a qdict_crumple method for un-flattening a dict, (continued)
Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion, Kevin Wolf, 2016/09/14
[Qemu-devel] [PATCH v11 3/6] qapi: rename QmpInputVisitor to QObjectInputVisitor, Daniel P. Berrange, 2016/09/05
[Qemu-devel] [PATCH v11 4/6] qapi: rename QmpOutputVisitor to QObjectOutputVisitor, Daniel P. Berrange, 2016/09/05
[Qemu-devel] [PATCH v11 6/6] qom: support arbitrary non-scalar properties with -object, Daniel P. Berrange, 2016/09/05