From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33272) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tszb3-0006s4-Oa for qemu-devel@nongnu.org; Wed, 09 Jan 2013 12:37:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tszb1-0001a4-F3 for qemu-devel@nongnu.org; Wed, 09 Jan 2013 12:37:49 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33620) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tszb1-0001Zp-80 for qemu-devel@nongnu.org; Wed, 09 Jan 2013 12:37:47 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r09HbkHY012124 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 9 Jan 2013 12:37:46 -0500 Message-ID: <50EDAAE9.9060103@redhat.com> Date: Wed, 09 Jan 2013 10:37:45 -0700 From: Eric Blake MIME-Version: 1.0 References: <1357566928-25361-1-git-send-email-kraxel@redhat.com> <1357566928-25361-12-git-send-email-kraxel@redhat.com> In-Reply-To: <1357566928-25361-12-git-send-email-kraxel@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig15D992801622E5EB2C721CC9" Subject: Re: [Qemu-devel] [PATCH 11/11] chardev: add pty chardev support to chardev-add (qmp) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig15D992801622E5EB2C721CC9 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/07/2013 06:55 AM, Gerd Hoffmann wrote: > The ptsname is returned directly, so there is no need to > use query-chardev to figure the pty device path. >=20 > Signed-off-by: Gerd Hoffmann > --- > qapi-schema.json | 7 ++++++- > qemu-char.c | 24 +++++++++++++++++++++--- > 2 files changed, 27 insertions(+), 4 deletions(-) It would be nice to document an example with the return type in qmp-commands.hx. In fact, it might be worth declaring just: { 'union': 'ChardevReturn', 'data': { 'nodata' : 'ChardevDummy' } } earlier in patch 4/11, so that we are consistently documenting the final API all along, rather than changing the return type and having to revisit earlier examples as part of this patch [but see below about an alternate proposal to leave earlier examples untouched]. In the long run, it won't matter as long as the series is applied as a whole - but if someone backports just part of the series, then we want to avoid the case where the backport has different return semantics than upstream, because they didn't pick up the change in return type from this patch. > +++ b/qapi-schema.json > @@ -3054,10 +3054,15 @@ > { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile', > 'port' : 'ChardevPort', > 'socket' : 'ChardevSocket', > + 'pty' : 'ChardevDummy', > 'null' : 'ChardevDummy' } } > =20 > +{ 'union': 'ChardevReturn', 'data': { 'pty' : 'str', > + 'nodata' : 'ChardevDummy' } } > + > { 'command': 'chardev-add', 'data': {'id' : 'str', > - 'backend' : 'ChardevBackend' } } > + 'backend' : 'ChardevBackend' }, > + 'returns' : 'ChardevReturn' } If I'm reading it correctly, this means my earlier example for 4/11 becom= es: -> { "execute" : "chardev-add", "arguments" : { "id" : "foo", "backend" : { "type" : "null" } } } <- { "return": { "type" : "nodata"} } or maybe the more verbose: <- { "return": { "type" : "nodata", "data" : {} } } and for this patch, we add: -> { "execute" : "chardev-add", "arguments" : { "id" : "foo", "backend" : { "type" : "pty", "data" : { } } } <- { "return": { "type" : "pty", "data" : "/dev/pty0" } } It also raises the question of whether unions even work with raw types, or whether you have to always go through a struct, in which case you should have used the 'String' wrapper instead of 'str', looking like: { 'union': 'ChardevReturn', 'data': { 'pty' : 'String', 'nodata' : 'ChardevDummy' } } =2E.. <- { "return": { "type" : "pty", "data" : { "str" : "/dev/pty0" } } } But do we really need it to be that complex? Why not just use a type with an optional member, instead of going through a union: { 'type' : 'ChardevReturn', 'data': { '*pty' : 'str' } } so that adding a null device returns the same as it always did: { "return": {} } and so that adding a pty looks nicer: { "return": { "pty" : "/dev/pty0" } } Libvirt will be able to cope either way, but I'd prefer a less complex solution. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig15D992801622E5EB2C721CC9 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQEcBAEBCAAGBQJQ7arpAAoJEKeha0olJ0NqiCgIAKBnn3LFOVrXZRk3xA2sVGG/ /aBAftYUqPZNnFzyINFxO2Yit5XKIMaHt7kkX4wPAxMcV0aqFrYR/jdIBTMTi2Sl o0kJCDNwkFWKmMCUEmMQMddYWQ6JGn2SnvYeP75OopNAVhwmjL6zL4aP1FV4PoFu avSPw1N26wMw6XLM876CXVm90d+e3hPoiat98qA4C4zvEKcJPQlvSRoa3+z2NNyQ x+9y+mD/f0+tSYj736fgUBKCJnN8jPSbKjmq5Sq67feG2t3PlMw3l0McyWwK68Mk rXPCReeyLmZ0LmBveqhyabDeTjeYbt1x6Ya5g4/xtCnkg7pAImhJw6xe3NNguic= =B6Jd -----END PGP SIGNATURE----- --------------enig15D992801622E5EB2C721CC9--