From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52268) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WE8u7-00086Z-MU for qemu-devel@nongnu.org; Thu, 13 Feb 2014 21:53:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WE8u1-0006qM-Bw for qemu-devel@nongnu.org; Thu, 13 Feb 2014 21:53:27 -0500 Received: from e28smtp04.in.ibm.com ([122.248.162.4]:50432) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WE8u0-0006qF-Ny for qemu-devel@nongnu.org; Thu, 13 Feb 2014 21:53:21 -0500 Received: from /spool/local by e28smtp04.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 14 Feb 2014 08:23:18 +0530 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 69A54394004E for ; Fri, 14 Feb 2014 08:23:15 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s1E2r7Hv65077480 for ; Fri, 14 Feb 2014 08:23:07 +0530 Received: from d28av03.in.ibm.com (localhost [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s1E2rEp3002554 for ; Fri, 14 Feb 2014 08:23:14 +0530 Message-ID: <52FD8529.5020305@linux.vnet.ibm.com> Date: Fri, 14 Feb 2014 10:53:29 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1392068921-3327-1-git-send-email-xiawenc@linux.vnet.ibm.com> <87wqgzt4sx.fsf@blackfin.pond.sub.org> In-Reply-To: <87wqgzt4sx.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 8bit 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: Markus Armbruster Cc: kwolf@redhat.com, lcapitulino@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com ÓÚ 2014/2/13 23:23, Markus Armbruster дµÀ: > Wenchao Xia 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