From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51920) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDy7J-0006qS-SN for qemu-devel@nongnu.org; Thu, 13 Feb 2014 10:22:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WDy7F-0005Lt-Fo for qemu-devel@nongnu.org; Thu, 13 Feb 2014 10:22:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53785) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDy7F-0005Ln-7Y for qemu-devel@nongnu.org; Thu, 13 Feb 2014 10:22:17 -0500 Date: Thu, 13 Feb 2014 10:22:10 -0500 From: Luiz Capitulino Message-ID: <20140213102210.44a49614@redhat.com> In-Reply-To: <20140211141319.514369cc@redhat.com> References: <1392068921-3327-1-git-send-email-xiawenc@linux.vnet.ibm.com> <20140211141319.514369cc@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V6 00/10] qapi script: support enum as discriminator and better enum name List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: kwolf@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, mdroth@linux.vnet.ibm.com, Wenchao Xia On Tue, 11 Feb 2014 14:13:19 -0500 Luiz Capitulino wrote: > On Tue, 11 Feb 2014 05:48:31 +0800 > Wenchao Xia 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. > > > > 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) > > Applied to the qmp branch, thanks. Just to make it clear, I'm not including this series in today's pull request so that you and Markus can coordinate your work. > > > > > > > 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. > > > > v6: > > rebased on upstream by adding "blgdebug" and "blkverify" in qapi-schema.json > > in patch 7/10. > > > > 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(-) > > >