From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44184) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwDBL-0003X5-37 for qemu-devel@nongnu.org; Mon, 17 Oct 2016 15:02:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwDBI-0001rP-DT for qemu-devel@nongnu.org; Mon, 17 Oct 2016 15:02:43 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:37456) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bwDBI-0001qn-4l for qemu-devel@nongnu.org; Mon, 17 Oct 2016 15:02:40 -0400 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9HIwkRT093120 for ; Mon, 17 Oct 2016 15:02:38 -0400 Received: from e19.ny.us.ibm.com (e19.ny.us.ibm.com [129.33.205.209]) by mx0a-001b2d01.pphosted.com with ESMTP id 265465rsw6-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 17 Oct 2016 15:02:37 -0400 Received: from localhost by e19.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 17 Oct 2016 15:02:36 -0400 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> <20161017185243.GE12934@work-vm> From: Jianjun Duan Date: Mon, 17 Oct 2016 12:02:30 -0700 MIME-Version: 1.0 In-Reply-To: <20161017185243.GE12934@work-vm> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Message-Id: <5a464924-aae1-1219-c30b-034d14a143ea@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: "Dr. David Alan Gilbert" Cc: David Gibson , qemu-devel@nongnu.org, amit.shah@redhat.com, quintela@redhat.com, pasic@linux.vnet.ibm.com On 10/17/2016 11:52 AM, Dr. David Alan Gilbert wrote: > * 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. > Then customized handling need to happen in pre_save/post_load. I think you need a way to pass TMP pointer around? Thanks, Jianjun > 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 >