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

[Eric Blake]

On 08/26/2015 09:31 AM, Eduardo Habkost wrote:
> 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:

That would be me, right? See my other mail for my tentative R-b;
although if you start renaming things due to this discussion, maybe I
should review v3 more closely after all.

>>>> +##
>>>> +{ '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.

The enum name is not ABI (we can name it whatever makes sense), but does
show up in the C code that interacts with the generated type.  In fact,
if you could name the enum type 'NumaOptionsKind', then the C enum name
would not change (staying at NUMA_OPTIONS_KIND_NODE instead of your
change to NUMA_DRIVER_NODE).  Except that right now, the qapi generator
doesn't allow user-defined types ending in 'Kind'.  :(

I don't know if naming it NumaDriver makes sense; and so your suggestion
of naming it NumaOptionsType may be better in the long run.  It would
make your C enum become NUMA_OPTIONS_TYPE_NODE.

Or maybe you wait for me to do a followup patch to the qapi generator to
allow user-defined types ending in 'Kind'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature