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

[Eduardo Habkost]

On Sat, Aug 22, 2015 at 05:56:40PM +0200, Kővágó Zoltán wrote:
> 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.

As long as somebody who understands QAPI says the changes make sense and
should work, I am OK with them if the following is changed:

> >[...]
> >>  ##
> >>+# @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.

In the docs/qapi-code-gen.txt example, the option is about an actual
blockdev driver, and the discriminator is really called "driver".

In this case, the field is called "type" and has nothing to do with
drivers, so something like NumaOptionType makes more sense.

-- 
Eduardo