From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwD1o-0003Sm-KE for qemu-devel@nongnu.org; Mon, 17 Oct 2016 14:52:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwD1l-0005v6-1q for qemu-devel@nongnu.org; Mon, 17 Oct 2016 14:52:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37810) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bwD1k-0005un-QK for qemu-devel@nongnu.org; Mon, 17 Oct 2016 14:52:48 -0400 Date: Mon, 17 Oct 2016 19:52:44 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20161017185243.GE12934@work-vm> References: <20161011171833.20803-1-dgilbert@redhat.com> <20161011171833.20803-2-dgilbert@redhat.com> <20161017033134.GM25390@umbus.fritz.box> <61d82189-401b-7063-ffa5-4442c7d11d25@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <61d82189-401b-7063-ffa5-4442c7d11d25@linux.vnet.ibm.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: Jianjun Duan Cc: David Gibson , qemu-devel@nongnu.org, amit.shah@redhat.com, quintela@redhat.com, pasic@linux.vnet.ibm.com * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote: > > > On 10/16/2016 08:31 PM, David Gibson wrote: > > On Tue, Oct 11, 2016 at 06:18:30PM +0100, Dr. David Alan Gilbert (git) wrote: > >> From: "Dr. David Alan Gilbert" > >> > >> 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. > >> > >> 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. > >> > >> 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_* > >> > If customized put/get can do transformation and dumping/loading data > to/from the parent structure, you don't have to go through > pre_save/post_load, and may get rid of parent pointer. Yes but I'd rather try and get rid of the customized put/get from every device, because then people start using qemu_put/qemu_get in them all. Dave > > Thanks, > Jianjun > > >> 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(+) > >> > >> 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; > >> > >> @@ -651,6 +652,25 @@ extern const VMStateInfo vmstate_info_qtailq; > >> .offset = offsetof(_state, _field), \ > >> } > >> > >> +/* 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 = "tmp", \ > >> + .size = sizeof(_tmp_type) + \ > >> + QEMU_BUILD_BUG_EXPR(offsetof(_tmp_type, parent) != 0) + \ > >> + type_check_pointer(_state, \ > >> + typeof_field(_tmp_type, parent)), \ > >> + .vmsd = &(_vmsd), \ > >> + .info = &vmstate_info_tmp, \ > >> + .flags = VMS_LINKED, \ > >> +} > >> + > >> #define VMSTATE_UNUSED_BUFFER(_test, _version, _size) { \ > >> .name = "unused", \ > >> .field_exists = (_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 = { > >> .put = put_unused_buffer, > >> }; > >> > >> +/* 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 *field) > >> +{ > >> + int ret; > >> + const VMStateDescription *vmsd = field->vmsd; > >> + int version_id = field->version_id; > >> + void *tmp = g_malloc(size); > >> + > >> + /* Writes the parent field which is at the start of the tmp */ > >> + *(void **)tmp = pv; > >> + ret = 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 *field, > >> + QJSON *vmdesc) > >> +{ > >> + const VMStateDescription *vmsd = field->vmsd; > >> + void *tmp = g_malloc(size); > >> + > >> + /* Writes the parent field which is at the start of the tmp */ > >> + *(void **)tmp = pv; > >> + vmstate_save_state(f, vmsd, tmp, vmdesc); > >> + g_free(tmp); > >> +} > >> + > >> +const VMStateInfo vmstate_info_tmp = { > >> + .name = "tmp", > >> + .get = get_tmp, > >> + .put = 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 > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK