From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39402) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqoYl-0008If-1i for qemu-devel@nongnu.org; Mon, 26 Oct 2015 16:40:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZqoYj-0004gW-7Q for qemu-devel@nongnu.org; Mon, 26 Oct 2015 16:40:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54944) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqoYi-0004fs-WD for qemu-devel@nongnu.org; Mon, 26 Oct 2015 16:40:01 -0400 References: <1445576998-2921-1-git-send-email-eblake@redhat.com> <1445576998-2921-14-git-send-email-eblake@redhat.com> <874mhdsglw.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <562E8F9A.6090904@redhat.com> Date: Mon, 26 Oct 2015 14:39:54 -0600 MIME-Version: 1.0 In-Reply-To: <874mhdsglw.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EOiXUmvd5bGGFmlcrH2Ax6bURAvw5t4Kf" Subject: Re: [Qemu-devel] [PATCH v10 13/25] qapi-visit: Convert to new qapi union layout List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EOiXUmvd5bGGFmlcrH2Ax6bURAvw5t4Kf Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/26/2015 11:07 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> We have two issues with our qapi union layout: >> 1) Even though the QMP wire format spells the tag 'type', the >> C code spells it 'kind', requiring some hacks in the generator. >> 2) The C struct uses an anonymous union, which places all tag >> values in the same namespace as all non-variant members. This >> leads to spurious collisions if a tag value matches a QMP name. >> >> Make the conversion to the new layout for qapi-visit.py. >=20 > This part is nicely mechanical: ->kind becomes ->type, ->variant become= s > ->u.variant, and variants.tag_name or 'kind' becomes > variants.tag_member.name. >=20 >> Also >> make a change in qapi.py to quit using the name 'kind', and >> the corresponding change in the testsuite. >=20 > This isn't really part of "qapi-visit: Convert to new qapi union > layout". Could be made a separate patch, but I'd simply squash it into= > the previous one "qapi: Start converting to new qapi union layout". Se= e > also my remark inline. >=20 It _was_ a part of the previous patch in v9 :) [...] >> +++ b/scripts/qapi.py >> @@ -548,7 +548,7 @@ def check_union(expr, expr_info): >> base =3D expr.get('base') >> discriminator =3D expr.get('discriminator') >> members =3D expr['data'] >> - values =3D {'MAX': '(automatic)', 'KIND': '(automatic)'} >> + values =3D {'MAX': '(automatic)', 'TYPE': '(automatic tag)'} >> >> # Two types of unions, determined by discriminator. >> >=20 > Took me a minute to remember what's going on here. A simple union's ta= g > values, become an enum type. Here, we catch two differend kinds of > clashes: >=20 > 1. Tag value with the implicitly defined enum member _MAX, i.e. a clash= > in the enum member name space >=20 > 2. Member of the (as of yet still) unnamed union holding the variants > with the tag variable type, formerly kind (i.e. a clash in the struct > member name space). >=20 > The previous patch added type to the struct member name space without > updating clash detection. >=20 > A future patch will remove the unnamed union, thus clash kind 2. It'll= > also remove kind from the struct member name space, but that doesn't > matter. >=20 > I believe the following would be slightly cleaner: amend the previous > patch to *add* key 'TYPE' to values. Drop both 'KIND' and 'TYPE' when > you remove the unnamed union. Okay, that works for me. Reserve additional namespace in 12, then drop the reservation in the later patch (now in subset C, see my comments to 24/25)... >=20 >> diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schem= a/union-clash-type.err >> index a5dead1..eca7c1d 100644 >> --- a/tests/qapi-schema/union-clash-type.err >> +++ b/tests/qapi-schema/union-clash-type.err >> @@ -1 +1 @@ >> -tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member '= kind' clashes with '(automatic)' >> +tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member '= type' clashes with '(automatic tag)' >=20 > You might be able to avoid this change by adding 'TYPE' in the right > spot. =2E..and see if I can avoid churn in union-clash-type.json up until the point in subset C where it disappears. I'll see how it plays out. Expect v11 soon. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --EOiXUmvd5bGGFmlcrH2Ax6bURAvw5t4Kf Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWLo+aAAoJEKeha0olJ0NqITQH/Agx9hy5+UC0TxX6OYG/uKm/ p85V0cqwsXkzUB8ejAbNnCNByOA2P7P0wSUYakOdHlfkHAorVv38d2faZZrLhGZT cEH2HOBJJ4IDgCuATM7UqiHci6jcf4sQRgq8X7U80S+cRLSfWqwwPQJbuvKgMKt+ OoYHrR3sx3OUVf8TZdamVB74CcThjYMMgXePrrxg50Rh3cIoOcYEzsXOPUqoOE1m mYtydAgsUlU26+TD0tlge2xzJgFMoojRWmYbtOR2qrwQPVcycu1m6qSWGZSyV5g1 BuhBi3LoLiHfXJ9uq1IlnmwXiybIWMh30n7Ic/1DQeYghEipKYC9iRWu1iOF4Gk= =G6if -----END PGP SIGNATURE----- --EOiXUmvd5bGGFmlcrH2Ax6bURAvw5t4Kf--