Re: [Qemu-devel] [PATCH V6 00/10] qapi script: support enum as discriminator and better enum name

[Wenchao Xia]

于 2014/2/13 23:23, Markus Armbruster 写道:
> Wenchao Xia <address@hidden> writes:
> 
>> This series address two issues:
>>
>> 1. support using enum as discriminator in union.
>> For example, if we have following define in qapi schema:
>> { 'enum': 'EnumOne',
>>    'data': [ 'value1', 'value2', 'value3' ] }
>>
>> { 'type': 'UserDefBase0',
>>    'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
>>
>> Before this series, discriminator in union must be a string, and a
>> hidden enum type as discriminator is generated. After this series,
>> qapi schema can directly use predefined enum type:
>> { 'union': 'UserDefEnumDiscriminatorUnion',
>>    'base': 'UserDefBase0',
>>    'discriminator' : 'base-enum0',
>>    'data': { 'value1' : 'UserDefA',
>>              'value2' : 'UserDefInherit',
>>              'value3' : 'UserDefB' } }
>>
>> The benefit is that every thing is defined explicitly in schema file,
>> the discriminator enum type can be used in other API define in schema,
>> and a compile time check will be put to verify the correctness according
>> to enum define. Currently BlockdevOptions used discriminator which can
>> be converted, in the future other union can also use enum discriminator.
>>
>> The implement is done by:
>> 1.1 remember the enum defines by qapi scripts.(patch 1)
>> 1.2 use the remembered enum define to check correctness at compile
>> time.(patch 3), more strict check(patch 2)
>> 1.3 use the same enum name generation rule to avoid C code mismatch,
>> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
>> 1.4 switch the code path, when pre-defined enum type is used as 
>> discriminator,
>> don't generate a hidden enum type, use the enum type instead, add
>> docs/qapi-code-gen.txt.(Patch 6)
>> 1.5 test case shows how it looks like.(Patch 7)
>> 1.6 convert BlockdevOptions. (Patch 8)
>>
>> 2. Better enum name generation
>> Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
>> AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
>> name generation codes into one function, it is done easily by modifying
>> it.(Patch 9)
> 
> Sorry for the lateness of my review.  I like this series, but I had a
> few questions related to error reporting.  Also, the new errors lack
> tests.
> 
> My offer to rebase it on top of my "[PATCH v2 00/13] qapi: Test coverage
> & clean up generated code" stands.
> 
  As patch 9 is the conflict patch and easy to rebase, I am fine if you
want to apply yours first and rebase mine.
  About the error reporting, I am not sure which way is better. If you
like qapi.py do it, I can send a patch later moving the code, since it
is easier than modify multiple patches in this series.
s