From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54761) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFRUd-0003pb-IZ for qemu-devel@nongnu.org; Mon, 17 Feb 2014 11:56:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFRUY-0000jq-MP for qemu-devel@nongnu.org; Mon, 17 Feb 2014 11:56:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:30147) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFRUY-0000jl-E0 for qemu-devel@nongnu.org; Mon, 17 Feb 2014 11:56:26 -0500 Message-ID: <53023F38.5060406@redhat.com> Date: Mon, 17 Feb 2014 09:56:24 -0700 From: Eric Blake MIME-Version: 1.0 References: <1392604023-20142-1-git-send-email-akong@redhat.com> <53023E43.1000706@redhat.com> In-Reply-To: <53023E43.1000706@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mWBfs4DgCCBFgjkUbIdfrEH4fH2B7l4kI" Subject: Re: [Qemu-devel] [PATCH] virtio-net: only output the vlan table when VIRTIO_NET_F_CTRL_VLAN is negotiated List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong , qemu-devel@nongnu.org Cc: vyasevic@redhat.com, lcapitulino@redhat.com, sf@sfritsch.de, mst@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --mWBfs4DgCCBFgjkUbIdfrEH4fH2B7l4kI Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/17/2014 09:52 AM, Eric Blake wrote: > On 02/16/2014 07:27 PM, Amos Kong wrote: >> Stefan Fritsch just fixed a virtio-net driver bug [1], virtio-net won'= t >> filter out VLAN-tagged packets if VIRTIO_NET_F_CTRL_VLAN isn't negotia= ted. >> >> We should also not send the vlan table to management, this patch makes= >> the vlan-talbe optional. >=20 > s/talbe/table/ >=20 >> @@ -4053,7 +4053,7 @@ >> 'multicast-overflow': 'bool', >> 'unicast-overflow': 'bool', >> 'main-mac': 'str', >> - 'vlan-table': ['int'], >> + '*vlan-table': ['int'], >=20 > Indentation is now off. >=20 >> - "main-mac": main macaddr string (json-string) >> -- "vlan-table": a json-array of active vlan id >> +- "vlan-table": a json-array of active vlan id (optoinal) >=20 > s/optoinal/optional/ >=20 > Those fixes are trivial enough, so I'm okay if you correct them then ad= d: Scratch that. I revoke my R-b, on the following grounds: On 02/17/2014 08:27 AM, Vlad Yasevich wrote: > On 02/16/2014 09:27 PM, Amos Kong wrote: >> Stefan Fritsch just fixed a virtio-net driver bug [1], virtio-net won'= t >> filter out VLAN-tagged packets if VIRTIO_NET_F_CTRL_VLAN isn't negotiated. >> >> We should also not send the vlan table to management, this patch makes= >> the vlan-talbe optional. > ^^^^^^^^^^ > table. > > One question I have from the API perspective is can we suddenly change > something to be optional? If there are any users of this , > wouldn't they have to change now to check the existence of this > list? You are correct. Since the parameter is an output field, older clients may be depending on it existing. It is okay to generate an empty array, but you must not entirely omit the array unless you add further justification in your commit message that you are 100% positive that there are no clients of 1.6 that will be broken when the array no longer appears in the output. Can you rework the patch to just leave the array empty in the case where the bit does not indicate it is used? Or do we need to add a new bool field to the output for new enough management to know whether to use the array? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --mWBfs4DgCCBFgjkUbIdfrEH4fH2B7l4kI 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/ iQEcBAEBCAAGBQJTAj84AAoJEKeha0olJ0NqFNsH/ApwxSMgJstM4pgDSPwyKuiw F7gzn3/8hIrqmgVlYZKiKu2JXDoyOo9quyJ4+xX5kbI3KDZeUsqYLc/egbIaxpXx Dv4lFEOFYmbae3gfEtwbGX1tnL0kRetRajFSiv5b9n461yVqdGoit6GNVy7Hfzel nHjfFo+U4UqPOfVzFzEzMPjD6r0aPxsTqYTs0oVLSR+7J9DbDBaaOOHZABHw/oiZ 7R9RJHOXP3nCiakr133PVVU/03Te8t+EzvMIikPtNMFytV0/uf7vIUQcjQbXC35Q fGSNsljiSGoRYGwdKtvHCb9VQ/bE+RLF+Oh5O/BHr+7X5b3tugulXhZyPV0FTUk= =6hjI -----END PGP SIGNATURE----- --mWBfs4DgCCBFgjkUbIdfrEH4fH2B7l4kI--