From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49318) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buIC0-0000NB-4z for qemu-devel@nongnu.org; Wed, 12 Oct 2016 07:59:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buIBv-0001HM-77 for qemu-devel@nongnu.org; Wed, 12 Oct 2016 07:59:27 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:40711 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buIBv-0001Gw-15 for qemu-devel@nongnu.org; Wed, 12 Oct 2016 07:59:23 -0400 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9CBwaIX110572 for ; Wed, 12 Oct 2016 07:59:22 -0400 Received: from e06smtp08.uk.ibm.com (e06smtp08.uk.ibm.com [195.75.94.104]) by mx0b-001b2d01.pphosted.com with ESMTP id 261e54jr2n-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 12 Oct 2016 07:59:22 -0400 Received: from localhost by e06smtp08.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 12 Oct 2016 12:59:20 +0100 References: <1475519097-27611-1-git-send-email-duanj@linux.vnet.ibm.com> <1475519097-27611-4-git-send-email-duanj@linux.vnet.ibm.com> From: Halil Pasic Date: Wed, 12 Oct 2016 13:59:02 +0200 MIME-Version: 1.0 In-Reply-To: <1475519097-27611-4-git-send-email-duanj@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="IXmHNq3pd7Wdc8pwBM1KKXoRSF4Tnwkib" Message-Id: <60b19618-e725-710f-f512-3f6df471f6f2@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [QEMU PATCH v5 3/6] migration: extend VMStateInfo List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jianjun Duan , qemu-devel@nongnu.org Cc: veroniabahaa@gmail.com, peter.maydell@linaro.org, dgilbert@redhat.com, mst@redhat.com, quintela@redhat.com, mark.cave-ayland@ilande.co.uk, mdroth@linux.vnet.ibm.com, mreitz@redhat.com, blauwirbel@gmail.com, amit.shah@redhat.com, qemu-ppc@nongnu.org, kraxel@redhat.com, kwolf@redhat.com, dmitry@daynix.com, pbonzini@redhat.com, rth@twiddle.net, leon.alrae@imgtec.com, aurelien@aurel32.net, david@gibson.dropbear.id.au This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --IXmHNq3pd7Wdc8pwBM1KKXoRSF4Tnwkib From: Halil Pasic To: Jianjun Duan , qemu-devel@nongnu.org Cc: veroniabahaa@gmail.com, peter.maydell@linaro.org, dgilbert@redhat.com, mst@redhat.com, quintela@redhat.com, mark.cave-ayland@ilande.co.uk, mdroth@linux.vnet.ibm.com, mreitz@redhat.com, blauwirbel@gmail.com, amit.shah@redhat.com, qemu-ppc@nongnu.org, kraxel@redhat.com, kwolf@redhat.com, dmitry@daynix.com, pbonzini@redhat.com, rth@twiddle.net, leon.alrae@imgtec.com, aurelien@aurel32.net, david@gibson.dropbear.id.au Message-ID: <60b19618-e725-710f-f512-3f6df471f6f2@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [QEMU PATCH v5 3/6] migration: extend VMStateInfo References: <1475519097-27611-1-git-send-email-duanj@linux.vnet.ibm.com> <1475519097-27611-4-git-send-email-duanj@linux.vnet.ibm.com> In-Reply-To: <1475519097-27611-4-git-send-email-duanj@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 10/03/2016 08:24 PM, Jianjun Duan wrote: > Current migration code cannot handle some data structures such as > QTAILQ in qemu/queue.h. Here we extend the signatures of put/get > in VMStateInfo so that customized handling is supported. >=20 > Signed-off-by: Jianjun Duan > --- > hw/net/vmxnet3.c | 18 ++++++--- > hw/nvram/eeprom93xx.c | 6 ++- > hw/nvram/fw_cfg.c | 6 ++- > hw/pci/msix.c | 6 ++- > hw/pci/pci.c | 12 ++++-- > hw/pci/shpc.c | 5 ++- > hw/scsi/scsi-bus.c | 6 ++- > hw/timer/twl92230.c | 6 ++- > hw/usb/redirect.c | 18 ++++++--- > hw/virtio/virtio-pci.c | 6 ++- > hw/virtio/virtio.c | 6 ++- > include/migration/vmstate.h | 10 +++-- > migration/savevm.c | 5 ++- > migration/vmstate.c | 95 ++++++++++++++++++++++++++++---------= -------- > target-alpha/machine.c | 5 ++- > target-arm/machine.c | 12 ++++-- > target-i386/machine.c | 21 ++++++---- > target-mips/machine.c | 10 +++-- > target-ppc/machine.c | 10 +++-- > target-sparc/machine.c | 5 ++- > 20 files changed, 171 insertions(+), 97 deletions(-) Hi Jianjun! I'm not happy with the intrusive nature of this patch. We end up with a bunch of unused parameters.=20 Have you considered something like: typedef struct { [..] union{ const VMStateInfo *info; const VMStateLinked *linked; }; enum VMStateFlags flags; [..] } VMStateField; /** * Handle linked datastructures. VMStateField.liked s to be used * in conjunction with VMStateField.vmsd which describes a node of * the datastucture without the pointers representing the links. * The links are embedded in the node starting at VMStateField.start. * The on wire representation of the information contained in links * and the head element if the responsibility of a particular VMStateField= * instance. VMStateLinked is responsible for saving/loading the whole=20 * sub-tree whose root is the field in question including the allocation o= f * memory for the nodes. The presence of VMStateField.linked is indicated * by the VMS_LINKED flag in VMStateField.flags. */ struct VMStateLinked { =20 const char *name; = =20 void (*save)(QEMUFile *f, void *pv, VMStateField *field, QJSON *vmdes= c); int (*load)(QEMUFile *f, void *pv, VMStateField *field); = =20 /* Maybe: size_t offset_links; */ = =20 }; IMHO this would: * allow us to keep the good old MVStateInfo objects unmodified and the semantic of VMStateInfo unchanged * make clear that VMStateLinked does not care about the calculated size and provide a new anchor for documentation * if overloading the semantic of VMStateField.start is not desired we could put it into VMStateLinked, if needed we could also put more stuff in there * have clearer separation between special handling for (linked/certain) data structures and the usual scenario with the DAG. I would also suggest unit tests in test/test-vmstate.c. Maybe with that the vmstate migration of QTAILQ could be factored out as a separate patch series. Cheers, Halil --IXmHNq3pd7Wdc8pwBM1KKXoRSF4Tnwkib 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) iQIcBAEBAgAGBQJX/iWTAAoJEA0vhuyXGx0As9QP/2NyjghMAsuwP4LimUROwVcb bNsWUXTbGW4JO2JuqZ9jsYoeTSYq0dr1Uk7kwJRLmXTTuZxK6XhIPUCMppncEc/u 6BBF6muLgQzQ2O1ODtadx420D7FloNUEULCAac/JhG1UOdGlKd1+pUg8jU7JFZk1 XowH2P1MC4CyQLVpaz+ZUgcuyXk3DQC5wB4aFYKLUoYsbLL476n3yYnAk9SuEw6+ e1jnNjJH6asoNztydD4MI2NalZG8Ik+fbD7k1Qp0cusqSW15eqw9LAhb77mk+85n oQczzT0fTpai6siBn0S1SYbdFzPPMo6FqIIRyEkxnJK4mUmVAf4FRvdBn0YCsUtC 3/gmLCvEAqCM9Zb6RskmwNBJLcb7JbNlrqDnwGMgY+PyOgopWJ9Iii/3CnEt4CsE IerFddQg2bV9W4ZFZhAp/lCn1U+5CeGTViqGhfGXmMi1Ej4VrSoWs+wIgjHfRZ4/ 7l6vOgtC3ebGotmEO5hoIJG9ZuerHdstKx/4EDjYziUQaYAlTDM1YElG21UpBogM D8cjio0FMei1I7kbebwBTOdysH4lIBkONTEiVqHYjGYkhGeH1QhA0Sb+vjbqc232 3aSW7uNAG3ypmcaTnJT6SeGZtdQbkCkbflaBZgCmGMlnFnFRUqJCBvEiyf2jTbVT xwkt9/aE/+5W5r1kMRz8 =hxZo -----END PGP SIGNATURE----- --IXmHNq3pd7Wdc8pwBM1KKXoRSF4Tnwkib--