From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59259) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjo3q-000795-VB for qemu-devel@nongnu.org; Tue, 13 Sep 2016 09:47:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjo3m-00027O-HK for qemu-devel@nongnu.org; Tue, 13 Sep 2016 09:47:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40070) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjo3m-00026f-9N for qemu-devel@nongnu.org; Tue, 13 Sep 2016 09:47:38 -0400 Date: Tue, 13 Sep 2016 14:47:34 +0100 From: "Daniel P. Berrange" Message-ID: <20160913134733.GV30949@redhat.com> Reply-To: "Daniel P. Berrange" References: <1473088600-17930-1-git-send-email-berrange@redhat.com> <1473088600-17930-6-git-send-email-berrange@redhat.com> <87mvjccgqj.fsf@dusky.pond.sub.org> <20160913093321.GG30949@redhat.com> <8760q07wni.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <8760q07wni.fsf@dusky.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Paolo Bonzini , =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , qemu-devel@nongnu.org, Andreas =?utf-8?Q?F=C3=A4rber?= , Max Reitz On Tue, Sep 13, 2016 at 03:32:33PM +0200, Markus Armbruster wrote: > "Daniel P. Berrange" 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 :|