Re: [Qemu-devel] [PATCH v2 03/49] qapi: convert NumaOptions into a flat union

[Kővágó Zoltán]

2015-08-22 01:13 keltezéssel, Eduardo Habkost írta:
> On Fri, Aug 21, 2015 at 05:36:59PM +0200, Kővágó, Zoltán wrote:
> > Signed-off-by: Kővágó, Zoltán <address@hidden>
>
> I don't understand QAPI enough to understand why exactly this is needed
> (so I would like to get feedback from somebody who actually understands
> QAPI unions), but I have one comment below.

It's needed so the option visitor can support nested structures properly 
without flattening them (or breaking backward compatibility).

There is some discussion here:
http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg04438.html

But basically the thing is that with new new opts visitor, unless we 
convert NumaOptions into a flat union, the user would have to type -numa 
node,node.nodeid=foo instead of -numa node,nodeid=foo which would break 
backward compatibility.

> [...]
> >   ##
> > +# @NumaDriver
> > +#
> > +# List of possible numa drivers.
> > +#
> > +# Since: 2.5
> > +##
> > +{ 'enum': 'NumaDriver',
> > +  'data': [ 'node' ] }
>
> Why is the name "NumaDriver"? Below, the field is called "type", so why
> not something like "NumaOptionType"?

No particular reason other than the example in docs/qapi-code-gen.txt 
used driver.  The field is called type because in the non-flat union the 
discriminator is called type.

> > +
> > +##
> > +# @NumaCommonOptions
> > +#
> > +# Common set of numa options.
> > +#
> > +# @type: the numa driver to use
> > +#
> > +# Since: 2.5
> > +##
> > +{ 'struct': 'NumaCommonOptions',
> > +  'data': {
> > +    'type': 'NumaDriver' } }
> > +
> > +##
> > +# @NumaOptions
> > +#
> > +# A discriminated record of NUMA options. (for OptsVisitor)
> > +#
> > +# Since 2.1
> > +##
> > +{ 'union': 'NumaOptions',
> > +  'base': 'NumaCommonOptions',
> > +  'discriminator': 'type',
> > +  'data': {
> > +    'node': 'NumaNodeOptions' }}
> > +
> > +##
> >   # @HostMemPolicy
> >   #
> >   # Host memory policy types
> > --
> > 2.5.0