From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48638) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCs3F-00031p-1x for qemu-devel@nongnu.org; Tue, 14 Jun 2016 13:22:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCs3B-0007dm-0Y for qemu-devel@nongnu.org; Tue, 14 Jun 2016 13:22:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45738) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCs3A-0007di-OU for qemu-devel@nongnu.org; Tue, 14 Jun 2016 13:22:52 -0400 References: <1463784024-17242-1-git-send-email-eblake@redhat.com> <1463784024-17242-8-git-send-email-eblake@redhat.com> <87ziqnaiz2.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <57603D6B.5020900@redhat.com> Date: Tue, 14 Jun 2016 11:22:51 -0600 MIME-Version: 1.0 In-Reply-To: <87ziqnaiz2.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="H7uIlDs5Itix3GqCVgj4JxVFj3JMBPDIv" Subject: Re: [Qemu-devel] [PATCH v7 07/15] qapi: Implement boxed types for commands/events 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) --H7uIlDs5Itix3GqCVgj4JxVFj3JMBPDIv Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/14/2016 09:27 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Turn on the ability to pass command and event arguments in >> a single boxed parameter. For structs, it makes it possible >> to pass a single qapi type instead of a breakout of all >> struct members; for unions, it is now possible to use a >> union as the data for a command or event. >> >> Generated code is unchanged, as long as no client uses the >> new feature. >> >> @@ -1640,12 +1652,13 @@ extern const char *const %(c_name)s_lookup[]; >> >> >> def gen_params(arg_type, box, extra): >> - if not arg_type: >> + if not arg_type or arg_type.is_empty(): >> return extra >=20 > When arg_type is empty, box gets ignored. That's not wrong, but I'd > skin this cat differently: when box=3Dtrue, pass a single arg_type > argument, no matter what the type is. Except that we don't have a visit function generated for the 'q_empty' type; I'm worried that coming up with the right arg_type for an empty box may be difficult. >> +++ b/tests/test-qmp-commands.c >> @@ -60,6 +60,18 @@ QObject *qmp_guest_sync(QObject *arg, Error **errp)= >> return arg; >> } >> >> +void qmp_boxed_empty(Error **errp) >> +{ >> +} >=20 > Demontrates that 'box': true with an empty type isn't boxed. In other words, an empty type takes precedence over 'box':true, because there is nothing to be passed. I could go the other direction and make it a hard error to use 'box':true on an empty type, if that would be conceptually cleaner. >> >> +By default, the generator creates a marshalling function that convert= s >> +an input QDict into a function call implemented by the user, and >=20 > Well, the called function is implemented by the user. >=20 >> +declares a prototype for the user's function which has a parameter fo= r >> +each member of the argument struct, including boolean arguments that >> +describe whether optional arguments were provided. But if the QAPI >> +description includes the key 'box' with the boolean value true, the >> +user call prototype will have only a single parameter for the overall= >> +generated C structure. The 'box' key is required in order to use a >> +union as an input argument, since it is not possible to list all >> +members of the union as separate parameters. >> + >=20 > Neglects to mention that 'data' is less restricted with 'box': true. >=20 > Suggest: >=20 > The generator emits a prototype for the user's function implementin= g > the command. Normally, 'data' is or names a struct type, and its > members are passed as separate arguments to this function. If the > command definition includes a key 'box' with the boolean value true= , > then the arguments are passed to the function as a single pointer t= o > the QAPI type generated for 'data'. 'data' may name an arbitrary > complex type then. Or maybe arbitrary non-empty complex type, depending on what we decide above. And maybe I still need to make it clear that when using 'box':true, an anonymous type is no longer permitted. >=20 > This still glosses over the has_ arguments, but it's no worse than > before. >=20 > Only then mention the marshalling: >=20 > The generator emits a marshalling function that extracts arguments > for the user's function out of an input QDict, calls the user's > function, and if it succeeded, builds an output QObject from its > return value. Sure, I can reword along those lines. (I may not state it enough, but thanks for your wordsmithing help). >> @@ -147,13 +150,14 @@ >> { 'struct': 'EventStructOne', >> 'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'Enum= One' } } >> >> -{ 'event': 'EVENT_A' } >> +{ 'event': 'EVENT_A', 'box': true } >=20 > This is case "empty". >=20 > Separate tests for both values of box would be cleaner, even though the= y > produce the exact same result. If we decide to obey box even with empt= y > types, they don't. >=20 Or, if we decide to forbid 'box':true on an empty type, then this needs tweaking anyway. >> { 'event': 'EVENT_B', >> 'data': { } } >> { 'event': 'EVENT_C', >> 'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } } >> { 'event': 'EVENT_D', >> 'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3= ': 'EnumOne' } } >> +{ 'event': 'EVENT_E', 'box': true, 'data': 'UserDefZero' } >=20 > This is case "struct". >=20 > Missing: case "union". >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --H7uIlDs5Itix3GqCVgj4JxVFj3JMBPDIv 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/ iQEcBAEBCAAGBQJXYD1rAAoJEKeha0olJ0Nq4vwIAI40POIsaw4OYQCJEv1YX3H2 AkAWulBKan72dDvDWx73xw1MP1PeRxJndjg25phA2Bypj/smIuUnPMDE31dIl8fd hodSlsAPP1J2OzUN3OSCUALmrLbziO1TlGmWajpWY1SkUVdMdvFl1B5Gc/7HBYER SS9pmFc+YkltrWbHhgegJiy1P56h9bHagracNdrqkQT1dsWplRtwjpK1/KmYEG1w 94IXfOM00FKv+h4P4o/lNt+IGkBFlOXWTcjDgOkptNLvaUJzPsAdWIHYOujS/NH2 CUE/kP+l6spVlD4/5mgBU0QfnoEDMM1pSHoDuWL+zd/7pK9Urr4pKecPIP93ddY= =mm1g -----END PGP SIGNATURE----- --H7uIlDs5Itix3GqCVgj4JxVFj3JMBPDIv--