On 10/17/2016 11:52 AM, Dr. David Alan Gilbert wrote: > * Jianjun Duan (du...@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" <dgilb...@redhat.com> >>>> >>>> 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 <dgilb...@redhat.com> >>> >>> 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 <da...@gibson.dropbear.id.au> >>> >>>> --- >>>> 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 / dgilb...@redhat.com / Manchester, UK >