From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51767) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wfsn3-00063f-Ch for qemu-devel@nongnu.org; Thu, 01 May 2014 11:20:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wfsmy-0005jF-Db for qemu-devel@nongnu.org; Thu, 01 May 2014 11:20:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54630) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wfsmx-0005ir-VL for qemu-devel@nongnu.org; Thu, 01 May 2014 11:20:44 -0400 Message-ID: <53626181.2000505@redhat.com> Date: Thu, 01 May 2014 09:00:17 -0600 From: Eric Blake MIME-Version: 1.0 References: <1398918422-3019-1-git-send-email-wenchaoqemu@gmail.com> <1398918422-3019-6-git-send-email-wenchaoqemu@gmail.com> In-Reply-To: <1398918422-3019-6-git-send-email-wenchaoqemu@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ixbe42oPHiNKJefbAN6M4sJ98nkTwtlnJ" Subject: Re: [Qemu-devel] [PATCH V5 05/28] qapi: define events in qapi schema List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia , qemu-devel@nongnu.org Cc: mdroth@linux.vnet.ibm.com, armbru@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --ixbe42oPHiNKJefbAN6M4sJ98nkTwtlnJ Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/30/2014 10:26 PM, Wenchao Xia wrote: > Some old type defines for spice and vnc are changed to let new > event defines use them instead of redefine. Note that define of > BlockErrorAction is moved from block.h to qapi schema, and it is > not merged with BlockdevOnError. In schema NIC_RX_FILTER_CHANGED's > param 'name' is changed as optional one, since in caller it is > optional. >=20 > Signed-off-by: Wenchao Xia > --- This is a big patch. See my comments on 7/28; do you have to do ALL of the qapi conversion here, or can you split it so that you are adding qapi one event at a time, in the same patch as that event also uses the generated code? Is the code motion of BlockErrorAction something that can be split into its own patch, to make the review focus easier? (Code motion and renaming fallout being separated from new additions is always easier than having both in one commit) > +++ b/docs/qmp/qmp-events.txt > @@ -1,39 +1,14 @@ > - QEMU Machine Protocol Events > - =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > + QEMU Machine Protocol Events Examples > + =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =20 > BALLOON_CHANGE > -------------- > - > -Emitted when the guest changes the actual BALLOON level. This > -value is equivalent to the 'actual' field return by the > -'query-balloon' command > - > -Data: > - > -- "actual": actual level of the guest memory balloon in bytes (json-nu= mber) > - > -Example: > - > { "event": "BALLOON_CHANGE", > "data": { "actual": 944766976 }, > "timestamp": { "seconds": 1267020223, "microseconds": 435656 } } I'm wondering if we still need this file, or if (by the end of the conversion to qapi) we can just drop it. Showing only an example usage, when the qapi already documents everything, isn't adding much value from my perspective. On the other hand, if you are able to rebase this patch to do one event at a time, then keep this file around until the end of the series. Then, for each event converted, you remove one chunk of this file, add one chunk to the schema.json file, and update all places to generate that new event, all in a single commit, where it becomes much easier to track that the conversion for that event was correct (here, there are so many events converted from .txt to .json at once that it is harder to correlate that the conversion of each event was correct). > +# @VncBasicInfo > +# > +# The basic information for vnc network connection > # > -# @host: The host name of the client. QEMU tries to resolve this to a= DNS name > -# when possible. > +# @host: IP address > # > -# @family: 'ipv6' if the client is connected via IPv6 and TCP > -# 'ipv4' if the client is connected via IPv4 and TCP > -# 'unix' if the client is connected via a unix domain socket > -# 'unknown' otherwise > +# @service: port number > # > -# @service: The service name of the client's port. This may depends o= n the > -# host system's service database so symbolic names should no= t be > -# relied on. Why are you reducing the information about @service? At least you got rid of the typo (s/depends/depend/). > +## > +# @VncClientInfo: > +# > +# Information about a connected VNC client. > # > # @x509_dname: #optional If x509 authentication is in use, the Disting= uished > # Name of the client. > @@ -1180,8 +1217,8 @@ > # Since: 0.14.0 > ## > { 'type': 'VncClientInfo', > - 'data': {'host': 'str', 'family': 'str', 'service': 'str', > - '*x509_dname': 'str', '*sasl_username': 'str'} } > + 'base': 'VncBasicInfo', > + 'data': { '*x509_dname' : 'str', '*sasl_username': 'str' } } All this work on refactoring the existing vnc QMP types should be its own patch, prior to any patch that introduces an event that also uses the shared types created by your refactoring. > + > +## > +# Event defines > +## If Llu=C3=ADs' series goes in first, it might make sense to have all of t= he event descriptions in their own file which gets included from the main qemu-schema.json. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --ixbe42oPHiNKJefbAN6M4sJ98nkTwtlnJ 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTYmGBAAoJEKeha0olJ0NqjlkH/0sV69rUEqH7HVAK5o0eIVLZ DTGYIDZJsqrTanWsmv1Mun0C/PCjUep20XOALEK9BhPJeRiN4XT0sBRSRlch5LSS dCqrPCFSpALdTdCFfhMRvWfO8iQ0LmLXyMtNChnznTBONvDwq9Fnn9DQ26DseFvg FxEMLhNiuK3P8F0BQSQ4bCGrfsL+Wz6MvCCxCFZcNh6B0RCQVuU3VNGhXAbNMaKb FKyXUGL0TnV6Ozf0gupBErTMUN9DVctn6ylhNciXeXy9cu00qFRxUKZj1tVpY/5s sEX+xQAxSNuk/zN7A1N9KVFVOzWLt5EDmNXS931s3RZoye4Y0pxnJ42XRp5L+sM= =UIDe -----END PGP SIGNATURE----- --ixbe42oPHiNKJefbAN6M4sJ98nkTwtlnJ--