From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37807) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsARZ-0006AC-M6 for qemu-devel@nongnu.org; Thu, 06 Oct 2016 11:18:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsARX-0000ju-B8 for qemu-devel@nongnu.org; Thu, 06 Oct 2016 11:18:44 -0400 Date: Thu, 6 Oct 2016 16:18:30 +0100 From: "Daniel P. Berrange" Message-ID: <20161006151830.GL14680@redhat.com> Reply-To: "Daniel P. Berrange" References: <1475246744-29302-1-git-send-email-berrange@redhat.com> <1475246744-29302-11-git-send-email-berrange@redhat.com> <20161006151042.GG5188@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161006151042.GG5188@noname.redhat.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v14 10/21] qapi: permit auto-creating nested structs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Markus Armbruster , Max Reitz , Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= On Thu, Oct 06, 2016 at 05:10:42PM +0200, Kevin Wolf wrote: > Am 30.09.2016 um 16:45 hat Daniel P. Berrange geschrieben: > > Some of the historical command line opts that had their > > keys in in a completely flat namespace are now represented > > by QAPI schemas that use a nested structs. When converting > > the QemuOpts to QObject, there is no information about > > compound types available, so the QObject will be completely > > flat, even after the qdict_crumple() call. So when starting > > a struct, we may not have a QDict available in the input > > data, so we auto-create a QDict containing all the currently > > unvisited input data keys. Not all historical command line > > opts require this, so the behaviour is opt-in, by specifying > > how many levels of structs are permitted to be auto-created. > > > > Note that this only works if the child struct is the last > > field to the visited in the parent struct. This is always > > the case for currently existing legacy command line options. > > > > The example is the NetLegacy type which has 3 levels of > > structs. The modern way to represent this in QemuOpts would > > be the dot-separated component approach > > > > -net vlan=1,id=foo,name=bar,opts.type=tap,\ > > opts.data.fd=3,opts.data.script=ifup > > > > The legacy syntax will just be presenting > > > > -net vlan=1,id=foo,name=bar,type=tap,fd=3,script=ifup > > > > So we need to auto-create 3 levels of struct when visiting. > > > > The implementation here will enable visiting in both the > > modern and legacy syntax, compared to OptsVisitor which > > only allows the legacy syntax. > > So you're actually introducing the modern syntax only now? No, the modern syntax is fully implemented by patch 8. This patch is about adding hacks for the legacy syntax used by the OptsVisitor. The OptsVisitor didn't interpret struct nesting at all, so everything looked flat - this only works as long as you don't have the same key used in multiple structs at different levels, so is not useful as a general approach - it only works by luck really. > I don't think an "opts.data." prefix makes a lot of sense. I suspect the > schema looks this way only because we didn't have flat unions in 1.2. > > So, considering that it is a purely internally used type not visible in > QMP, would it make sense to change NetLegacy to be a flat union instead, > with NetLegacyOptions as the common base? Then you get the same flat > namespace that we always had and that makes much more sense as an API. Changing that will impact on the QMP data structure, so I don't think we can do that. 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/ :|