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

[Kevin Wolf]

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

"now" in the sense of "in this series" (because this means that there is
no external API to preserve yet).

> 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.

Kevin