Re: [Qemu-block] [PATCH v14 10/21] qapi: permit auto-creating nested structs

[Daniel P. Berrange]

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/ :|