From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49462) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brKfJ-0007wV-DW for qemu-devel@nongnu.org; Tue, 04 Oct 2016 04:01:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brKfE-0003yb-EK for qemu-devel@nongnu.org; Tue, 04 Oct 2016 04:01:28 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:44556) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brKfE-0003y5-4l for qemu-devel@nongnu.org; Tue, 04 Oct 2016 04:01:24 -0400 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u947vg8B052593 for ; Tue, 4 Oct 2016 04:01:23 -0400 Received: from e06smtp06.uk.ibm.com (e06smtp06.uk.ibm.com [195.75.94.102]) by mx0a-001b2d01.pphosted.com with ESMTP id 25uy9kuwrt-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 04 Oct 2016 04:01:22 -0400 Received: from localhost by e06smtp06.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 4 Oct 2016 09:01:19 +0100 Received: from b06cxnps4076.portsmouth.uk.ibm.com (d06relay13.portsmouth.uk.ibm.com [9.149.109.198]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 6120017D8024 for ; Tue, 4 Oct 2016 09:03:20 +0100 (BST) Received: from d06av09.portsmouth.uk.ibm.com (d06av09.portsmouth.uk.ibm.com [9.149.37.250]) by b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u9481FII15466968 for ; Tue, 4 Oct 2016 08:01:15 GMT Received: from d06av09.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av09.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u9481EHc030598 for ; Tue, 4 Oct 2016 02:01:14 -0600 References: <20160930142003.53232-1-pasic@linux.vnet.ibm.com> <20160930142003.53232-2-pasic@linux.vnet.ibm.com> <2487d687-7fbe-43f4-b9c5-e03b26b2250d@redhat.com> <5b08c569-813b-d805-833a-210e9c8dc436@linux.vnet.ibm.com> <49f37536-0fa7-a38b-a3fa-7b3050001658@redhat.com> <57b04738-7e95-a8c5-d974-0386cbff1419@redhat.com> From: Halil Pasic Date: Tue, 4 Oct 2016 10:00:48 +0200 MIME-Version: 1.0 In-Reply-To: <57b04738-7e95-a8c5-d974-0386cbff1419@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Vmt6XEhB6ci1EioVq8N0oiffx2R8gcp8q" Message-Id: <17a15fe0-12b5-98d3-b630-a229e96c809b@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH 01/12] virtio: add VIRTIO_DEF_DEVICE_VMSD macro List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: "Michael S . Tsirkin" , "Dr . David Alan Gilbert" , "Aneesh Kumar K . V" , Stefan Hajnoczi , Amit Shah , Gerd Hoffmann This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Vmt6XEhB6ci1EioVq8N0oiffx2R8gcp8q From: Halil Pasic To: Paolo Bonzini , qemu-devel@nongnu.org Cc: "Michael S . Tsirkin" , "Dr . David Alan Gilbert" , "Aneesh Kumar K . V" , Stefan Hajnoczi , Amit Shah , Gerd Hoffmann Message-ID: <17a15fe0-12b5-98d3-b630-a229e96c809b@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH 01/12] virtio: add VIRTIO_DEF_DEVICE_VMSD macro References: <20160930142003.53232-1-pasic@linux.vnet.ibm.com> <20160930142003.53232-2-pasic@linux.vnet.ibm.com> <2487d687-7fbe-43f4-b9c5-e03b26b2250d@redhat.com> <5b08c569-813b-d805-833a-210e9c8dc436@linux.vnet.ibm.com> <49f37536-0fa7-a38b-a3fa-7b3050001658@redhat.com> <57b04738-7e95-a8c5-d974-0386cbff1419@redhat.com> In-Reply-To: <57b04738-7e95-a8c5-d974-0386cbff1419@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/03/2016 05:24 PM, Paolo Bonzini wrote: >=20 >=20 > On 03/10/2016 15:34, Halil Pasic wrote: >> Hi Paolo, >> >> I'm sorry, but I do not get it quite yet, or more exactly I have the >> feeling I did not manage to bring my point over. So I will try with >> more details. >> >> On 10/03/2016 01:29 PM, Paolo Bonzini wrote: >>> >>> >>> On 03/10/2016 12:36, Halil Pasic wrote: >>>>>> #define VMSTATE_PCI_DEVICE(_field, _state) { >> >> This was probably supposed to be VMSTATE_VIRTIO_DEVICE. >=20 > Yes. >=20 >>>>>> .name =3D (stringify(_field)), = \ >>>>>> .size =3D sizeof(VirtIODevice), = \ >>>>>> .vmsd =3D &vmstate_virtio_device, = \ >> >> This one does not exist at least very tricky to write because of the p= eculiarities >> of virtio migration. This one would need to migrate the transport stuf= f too. And >> the specialized device to, which is not normal. >=20 > This is my own typo - this should be .info. Sorry. >=20 >>> Regarding VMSTATE_VIRTIO_FIELD, it's just a matter of not doing thing= s >>> differently for no particular reason. Your macro is already doing >>> exactly the same as other VMSTATE_* macros, just with different >>> conventions for the arguments. I don't see any advantage in changing= that. >> >> In my opinion it is not the same. In the case of VMSTATE_PCI_DEVICE we= have >> (a self contained) parent (in sense of inheritance) device, which is e= mbedded >> as _field in the specialized device and is migrated by the vmstatedesc= ription >> of the parent. The rest of the specialized devices state is represente= d by >> the other fields. >> >> VMSTATE_VIRTIO_FIELD is however just there to make sure virtio_load an= d >> virtio_save are called at the right time with the right arguments. The= specialized >> device is then migrated by the save/load callbacks of the device class= , or >> the vmsd residing in the device class. VMSTATE_VIRTIO_FIELD is suppose= d >> to be the only field, if the virtio device adheres to the virtio-migra= tion >> document. VMSTATE_VIRTIO_FIELD has no arguments because it is >> a virtual field and does not depend on the offset stuff. >> >> To summarize currently I have no idea how to write up the vmstate >> field definition macro VMSTATE_VIRTIO_DEVICE so that it meets your >> expectations.=20 >=20 > As above. >=20 >>> Having everything hidden behind a magic macro makes >>> things harder to follow than other vmstate definitions. =20 >> >> Again in my opinion the virtio migration is different that the rest of= the >> vmstate migration stuff, and it's ugly to some extent. So the idea was= >> to make it look different and hide the ugliness behind the macro.=20 >> If you insist i will create a version where the macros are expanded so= >> it's easier to say if this improves or worsens the readability. >=20 > I agree it is not exactly the same as the other devices. But in my > opinion it's not different-enough to do everything completely more, and= > in the future we should aim at making it less different. >=20 >> I think this is matter of taste, and your taste matters more ;). I do = >> agree that the variadic for the massaging functions is more complicate= d >> that the two function pointers taken by VMSTATE_VIRTIO_DEVICE. My idea= >> was that we end up with more readable code on the caller-side, but if = you >> prefer function pointers and NULLs if no callback is needed needed=20 >> (most cases), I can live with that. >=20 > Well, the third possibility would be expanding the VMStateDescription > definition, :) where .post_load becomes just yet another initializer. >=20 > Paolo >=20 Hi Paolo, clear now. I will come back with a v2 with the suggestions implemented. Thank you very much for your time. Cheers, Halil --Vmt6XEhB6ci1EioVq8N0oiffx2R8gcp8q 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.0.14 (GNU/Linux) iQIcBAEBAgAGBQJX82HJAAoJEA0vhuyXGx0APaQP/153rO9nVij4lztb5zZzUvsu Uk0WjSGZrjv9e9+TkmaqHfwasZ4jmQprX6g9XPHZ2w8gzqWpfJ8KATCDJifHz/0O JX1Zb6g6uva0pGDOOypHO/8dgfzBPwkIg1WnM4Fp+6hylVs2OivYxhcbMLqaMd/I iu48LLabsEIXVFH4C8R5iqqsR8Ja1iJYyiWcYnvewFZ2dq5lKM92xh1ZxdIUv3/y 6p56Q6Th8dn90hDFFi9i+0OkN+Ur2nqSslF9+5lGN3ED0sogmU4FTliimtqz+lKP D8BAyNNnxLeCL3x+R0XSEm3iHQQ4SI0ybCIcFlj5SIfGKRs+Qeo2SSOAHFz4hjEp a52HMFQi4kiJWSr2docKH+miQZRGlfra7OtOThwjv92Erzwq4qkhl3AO7NviRrXc 8ae+Wh3mS1lmE6FGrX/K7WmA3qyceMMzXAUi8wDn4ji0YSUNNPWvc9Htgh+8PEZ0 VZheiTK6MIYDDwGOOua9rCgKwFdL3LBMrzQlMkeC7WCUZcnw2uPum632qUzXCZqE 1aKrga91RRiXgEwoux+T4T+tHKZEnDZNDk7251AJceSzKiwVHT+xllIPt3lyoFXt 5ALEXpt1ja5/FlI3sdRiBRlyu+GY2NXBoLae88U3TUB076VdNhw3yT8LjMDirxl+ wymF69oLwj7MrKm56Xaf =Lr13 -----END PGP SIGNATURE----- --Vmt6XEhB6ci1EioVq8N0oiffx2R8gcp8q--