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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Fri, 20 Dec 2013 13:23:06 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

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

Applied to the qmp branch, thanks.

> 
> 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)
> 
> 
> Changes from RFC:
>   Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
> enum value in union, add related test case, remove direct inherit support "_base"
> and related test case. RFC series at:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
> 
> v2:
>   General:
>   3/8: use Raise exception instead of sys.error.write in qapi.py.
>   Address Eric's comments:
>   2/8,3/8: more check for enum value at compile time, not allow partly mapping.
>   8/8: correspond test case change.
> 
> v3:
>   General:
>   move enum name generation patch to last in the series, add convert patch
> 8/9.
>   Address Luiz and Kevin's comments:
>   Better introduction.
>   6/9: renamed this patch, add docs/qapi-code-gen.txt part.
> 
> v4:
>   Address Eric's comments:
>   5/9: better commit message.
>   6/9: typo fix in doc.
>   9/9: typo fix, fix indentation, better incode comment.
> 
> v5:
>   Address Eric's comments:
>   6/10: doc typo fix.
>   8/10: new patch to remove string discriminator.
>   9/10: removed the string discriminator test case.
> 
> Wenchao Xia (10):
>   1 qapi script: remember enum values
>   2 qapi script: add check for duplicated key
>   3 qapi script: check correctness of discriminator values in union
>   4 qapi script: code move for generate_enum_name()
>   5 qapi script: use same function to generate enum string
>   6 qapi script: support pre-defined enum type as discriminator in union
>   7 qapi: convert BlockdevOptions to use enum discriminator
>   8 qapi script: do not allow string discriminator
>   9 tests: add cases for inherited struct and union with discriminator
>   10 qapi script: do not add "_" for every capitalized char in enum
> 
>  docs/qapi-code-gen.txt                  |    8 ++-
>  include/qapi/qmp/qerror.h               |    2 +-
>  qapi-schema.json                        |   14 ++++-
>  scripts/qapi-types.py                   |   34 +++++-----
>  scripts/qapi-visit.py                   |   61 ++++++++++++++---
>  scripts/qapi.py                         |   84 +++++++++++++++++++++--
>  target-i386/cpu.c                       |    2 +-
>  tests/qapi-schema/comments.out          |    2 +-
>  tests/qapi-schema/qapi-schema-test.json |   22 ++++++
>  tests/qapi-schema/qapi-schema-test.out  |   13 +++-
>  tests/test-qmp-input-visitor.c          |   93 +++++++++++++++++++++++++
>  tests/test-qmp-output-visitor.c         |  116 +++++++++++++++++++++++++++++++
>  12 files changed, 411 insertions(+), 40 deletions(-)
>