From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37476) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bvyjw-0000Mx-QS for qemu-devel@nongnu.org; Sun, 16 Oct 2016 23:37:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bvyjt-0007Fy-KZ for qemu-devel@nongnu.org; Sun, 16 Oct 2016 23:37:28 -0400 Received: from ozlabs.org ([103.22.144.67]:41219) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bvyjs-0007EZ-LT for qemu-devel@nongnu.org; Sun, 16 Oct 2016 23:37:25 -0400 Date: Mon, 17 Oct 2016 14:31:34 +1100 From: David Gibson Message-ID: <20161017033134.GM25390@umbus.fritz.box> References: <20161011171833.20803-1-dgilbert@redhat.com> <20161011171833.20803-2-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="64LDleNqNegJ4g97" Content-Disposition: inline In-Reply-To: <20161011171833.20803-2-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: qemu-devel@nongnu.org, amit.shah@redhat.com, quintela@redhat.com, duanj@linux.vnet.ibm.com, pasic@linux.vnet.ibm.com --64LDleNqNegJ4g97 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 11, 2016 at 06:18:30PM +0100, Dr. David Alan Gilbert (git) wrot= e: > From: "Dr. David Alan Gilbert" >=20 > VMSTATE_WITH_TMP is for handling structures where some calculation > or rearrangement of the data needs to be performed before the data > hits the wire. > For example, where the value on the wire is an offset from a > non-migrated base, but the data in the structure is the actual pointer. >=20 > To use it, a temporary type is created and a vmsd used on that type. > The first element of the type must be 'parent' a pointer back to the > type of the main structure. VMSTATE_WITH_TMP takes care of allocating > and freeing the temporary before running the child vmsd. >=20 > The post_load/pre_save on the child vmsd can copy things from the parent > to the temporary using the parent pointer and do any other calculations > needed; it can then use normal VMSD entries to do the actual data > storage without having to fiddle around with qemu_get_*/qemu_put_* >=20 > Signed-off-by: Dr. David Alan Gilbert The requirement for the parent pointer is a little clunky, but I don't quickly see a better way, and it is compile-time verified. As noted elsewhere I think this is a really useful approach which could allow a bunch of internal state cleanups while preserving migration. Reviewed-by: David Gibson > --- > include/migration/vmstate.h | 20 ++++++++++++++++++++ > migration/vmstate.c | 38 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 58 insertions(+) >=20 > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 9500da1..efb0e90 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -259,6 +259,7 @@ extern const VMStateInfo vmstate_info_cpudouble; > extern const VMStateInfo vmstate_info_timer; > extern const VMStateInfo vmstate_info_buffer; > extern const VMStateInfo vmstate_info_unused_buffer; > +extern const VMStateInfo vmstate_info_tmp; > extern const VMStateInfo vmstate_info_bitmap; > extern const VMStateInfo vmstate_info_qtailq; > =20 > @@ -651,6 +652,25 @@ extern const VMStateInfo vmstate_info_qtailq; > .offset =3D offsetof(_state, _field), \ > } > =20 > +/* Allocate a temporary of type 'tmp_type', set tmp->parent to _state > + * and execute the vmsd on the temporary. Note that we're working with > + * the whole of _state here, not a field within it. > + * We compile time check that: > + * That _tmp_type contains a 'parent' member that's a pointer to the > + * '_state' type > + * That the pointer is right at the start of _tmp_type. > + */ > +#define VMSTATE_WITH_TMP(_state, _tmp_type, _vmsd) { \ > + .name =3D "tmp", \ > + .size =3D sizeof(_tmp_type) + \ > + QEMU_BUILD_BUG_EXPR(offsetof(_tmp_type, parent) !=3D= 0) + \ > + type_check_pointer(_state, \ > + typeof_field(_tmp_type, parent)), \ > + .vmsd =3D &(_vmsd), \ > + .info =3D &vmstate_info_tmp, \ > + .flags =3D VMS_LINKED, \ > +} > + > #define VMSTATE_UNUSED_BUFFER(_test, _version, _size) { \ > .name =3D "unused", \ > .field_exists =3D (_test), \ > diff --git a/migration/vmstate.c b/migration/vmstate.c > index 2157997..f2563c5 100644 > --- a/migration/vmstate.c > +++ b/migration/vmstate.c > @@ -925,6 +925,44 @@ const VMStateInfo vmstate_info_unused_buffer =3D { > .put =3D put_unused_buffer, > }; > =20 > +/* vmstate_info_tmp, see VMSTATE_WITH_TMP, the idea is that we allocate > + * a temporary buffer and the pre_load/pre_save methods in the child vmsd > + * copy stuff from the parent into the child and do calculations to fill > + * in fields that don't really exist in the parent but need to be in the > + * stream. > + */ > +static int get_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *fie= ld) > +{ > + int ret; > + const VMStateDescription *vmsd =3D field->vmsd; > + int version_id =3D field->version_id; > + void *tmp =3D g_malloc(size); > + > + /* Writes the parent field which is at the start of the tmp */ > + *(void **)tmp =3D pv; > + ret =3D vmstate_load_state(f, vmsd, tmp, version_id); > + g_free(tmp); > + return ret; > +} > + > +static void put_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *fi= eld, > + QJSON *vmdesc) > +{ > + const VMStateDescription *vmsd =3D field->vmsd; > + void *tmp =3D g_malloc(size); > + > + /* Writes the parent field which is at the start of the tmp */ > + *(void **)tmp =3D pv; > + vmstate_save_state(f, vmsd, tmp, vmdesc); > + g_free(tmp); > +} > + > +const VMStateInfo vmstate_info_tmp =3D { > + .name =3D "tmp", > + .get =3D get_tmp, > + .put =3D put_tmp, > +}; > + > /* bitmaps (as defined by bitmap.h). Note that size here is the size > * of the bitmap in bits. The on-the-wire format of a bitmap is 64 > * bit words with the bits in big endian order. The in-memory format --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --64LDleNqNegJ4g97 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYBEYWAAoJEGw4ysog2bOSBe8QALzmskNPLSRNfyrMY3EDZ3K9 BNHxWkd4yUWTafuy7bydrQQM7ioeanvnO/NJIjmQG9xEbJuan4E6YlIcSY1wgGtG iuMX6TZuROVBcqgNeYB35ZmLzJnJvyLecf1+KtosQXsSdvgZfHioxPcoKwbwJe8r 8GTeMb30GMneWQiRdJu5G2d6SA71o3tlXURtyWO9FpIXrZfCKyEBjPo9kv2Ing/N Mmvc18CHDsSXSMsyQgsyjS/Vk9VPGRjIvY74TGojAHR+YSWo1qM0wGB2TNN+Q4V+ 4/xgtGO8Y+VFxqZuBpxdMM0HDsEuQfXVybPHkmHg+qIAmRDPf7D1duFH4VIbya+p vwjktCH/NWtuGxUFM7dES6TIP+gpHJlOWbqMzdDMddJmIxEgOBnhG4QPzAjZ1/uO LmpNXRB8c+IpGbJzcinlt/j0gureQPwJJvU3O6NFqKX7bMJZo7uk52xRGbLjQI/V LIA3PWUXA/JotQiG/b466VcQv8GzXly0rW0Lx+6xxrnmy3ABCN8Y1Nv6TaYjwJN5 n5vpUTghgHLS0MhIEYi4p4ZiTx+sIO/H4PEHC+uTCOensVizy9eYjOpbVTyBFwno +jNTp2Nc5Wpxx0Guvm3NyMmDCcogiUcPJG//6FfmkdIlO8C6nNm4DpeT71aAuJwP 5fbVmLXdLxOLM6J2MKCi =pb2F -----END PGP SIGNATURE----- --64LDleNqNegJ4g97--