From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46300) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adKW7-0004vW-Ow for qemu-devel@nongnu.org; Tue, 08 Mar 2016 11:29:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adKW3-0000hu-Es for qemu-devel@nongnu.org; Tue, 08 Mar 2016 11:29:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48862) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adKW3-0000hn-7p for qemu-devel@nongnu.org; Tue, 08 Mar 2016 11:29:47 -0500 References: <1457194595-16189-1-git-send-email-eblake@redhat.com> <1457194595-16189-9-git-send-email-eblake@redhat.com> <87si015474.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56DEFDFA.5050701@redhat.com> Date: Tue, 8 Mar 2016 09:29:46 -0700 MIME-Version: 1.0 In-Reply-To: <87si015474.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="dHfnjJpsHDB5OeaNVi36dMLJ4fIEmiXnB" Subject: Re: [Qemu-devel] [PATCH v4 08/10] qapi: Allow anonymous base for flat union 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) --dHfnjJpsHDB5OeaNVi36dMLJ4fIEmiXnB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 03/08/2016 09:23 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Rather than requiring all flat unions to explicitly create >> a separate base struct, we can allow the qapi schema to specify >> the common members via an inline dictionary. This is similar to >> how commands can specify an inline anonymous type for its 'data'. >> We already have several struct types that only exist to serve as >> a single flat union's base; the next commit will clean them up. >> >=20 > I think you also >=20 > * fixed a missing allow_optional=3DTrue in check_union() More on that below. >=20 > * fixed a missing "non-optional" in qapi-code-gen.txt (mention in commi= t > message? separate patch?) >=20 > * renamed readonly to read-only in an example in qapi-code-gen.txt to b= e > closer to the code (separate patch?) Could separate those two cleanups if it helps. >> @@ -560,9 +562,10 @@ def check_union(expr, expr_info): >> >> # Else, it's a flat union. >> else: >> - # The object must have a string member 'base'. >> + # The object must have a string or dictionary 'base'. >> check_type(expr_info, "'base' for union '%s'" % name, >> - base, allow_metas=3D['struct']) >> + base, allow_dict=3DTrue, allow_optional=3DTrue, >> + allow_metas=3D['struct']) >=20 > This is where you added allow_optional=3DTrue. allow_optional only matters if allow_dict is True. We have places where we allow a dict but do not allow optionals (namely, simple unions); but where we are creating an anonymous type, we want to allow optionals at the same time we allow a dict. I think this is just a case where the commit message needs to be beefed up. >> +A flat union definition avoids nesting on the wire, and specifies a >> +set of common members that occur in all variants of the union. The >> +'base' key must specifiy either a type name (the type must be a >> +struct, not a union), or a dictionary representing an anonymous type.= >> +All branches of the union must be complex types, and the top-level >> +members of the union dictionary on the wire will be combination of >> +members from both the base type and the appropriate branch type (when= >> +merging two dictionaries, there must be no keys in common). The >> +'discriminator' member must be the name of a non-optional enum-typed >=20 > This is where you supplied the missing "non-optional". >=20 >> +member of the base struct. >> >=20 > And below, you rename readonly to read-only. They touch the same paragraph, but I can separate them if it would make this patch feel cleaner. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --dHfnjJpsHDB5OeaNVi36dMLJ4fIEmiXnB 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/ iQEcBAEBCAAGBQJW3v36AAoJEKeha0olJ0Nqq5IH/3sC06JPbyiiCmnCB6DlszRb SMFeGaIG3MQvmBo6DIxSrrBxo+RM+G2IaZLszfnsrjvvppOdukGwvaXbDxLEnRNk wXBP70BXazX4b0KSP8crn9sYo9J6/lb4v84v5ZIAxCOhTs/r93RtHqbNd2YwE9oV QdVOrEevuyOdM6Fr/tUHVN+XAG3cgO01kmHeT3ILXh3fEHpcGG+F5ta4D5vE52fQ vusSGq78XpOf5Uv1Fy5RxOl3vgi1H+nwtMDj6ExD1a4grpWnDCksrUhUcJvW8QjH 7PLEtR6eOL7qJs2sw5xkyt/pNTl6oTXU8xlvcEiKOLJ9srS7OCVWMPCp0fg7Qz4= =XRCt -----END PGP SIGNATURE----- --dHfnjJpsHDB5OeaNVi36dMLJ4fIEmiXnB--