From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsB3C-0006pj-DM for qemu-devel@nongnu.org; Thu, 06 Oct 2016 11:57:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsB3A-0000cq-59 for qemu-devel@nongnu.org; Thu, 06 Oct 2016 11:57:37 -0400 Date: Thu, 6 Oct 2016 16:57:22 +0100 From: "Daniel P. Berrange" Message-ID: <20161006155722.GP14680@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> <20161006151830.GL14680@redhat.com> <20161006153005.GH5188@noname.redhat.com> <20161006153904.GN14680@redhat.com> <20161006155157.GI5188@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161006155157.GI5188@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:51:57PM +0200, Kevin Wolf wrote: > Am 06.10.2016 um 17:39 hat Daniel P. Berrange geschrieben: > > On Thu, Oct 06, 2016 at 05:30:05PM +0200, Kevin Wolf wrote: > > > Am 06.10.2016 um 17:18 hat Daniel P. Berrange geschrieben: > > > > 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: > > > > > > 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. > > > > > > "now" in the sense of "in this series" (because this means that there is > > > no external API to preserve yet). > > > > Well the syntax implemented in patch 8 is designed to > > 100% mirror the QAPI schema structure nesting. I don't > > think we want to change that behaviour. > > I think patch 8 is fine as it is. > > > If there are certain QAPI schemas structs can still > > have freedom to change though, its fine to reconsider > > them > > > > > > > > > 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. > > > > > > I don't see this type used in QMP at all. It's only used for command > > > line parsing, and only with the OptsVisitor, so I think we're fine if we > > > flatten it now. > > > > Ok, yes, it seems "NetLegacy" is only used in CLI arg parsing, so we > > do have some freedom there. > > > > This patch was also needed for -numa handling too - again we might have > > some flexibility to flatten that. > > NumaOptions is also unused in QMP, so it's the same thing. > > If these two are the only options that need the behaviour, I would > prefer if we changed the QAPI schema to flatten them, and then we could > save ourselves some complexity by dropping this patch. Agreed, the fewer hacks like this that we need the better. 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/ :|