diff mbox

[very-WIP,1/7] migration: Add VMSTATE_WITH_TMP

Message ID 20161011171833.20803-2-dgilbert@redhat.com
State New
Headers show

Commit Message

Dr. David Alan Gilbert Oct. 11, 2016, 5:18 p.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@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_*

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/vmstate.h | 20 ++++++++++++++++++++
 migration/vmstate.c         | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

Comments

David Gibson Oct. 17, 2016, 3:31 a.m. UTC | #1
On Tue, Oct 11, 2016 at 06:18:30PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@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_*
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@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 <david@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
Jianjun Duan Oct. 17, 2016, 6:49 p.m. UTC | #2
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" <dgilbert@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.

Thanks,
Jianjun

>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@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 <david@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 Oct. 17, 2016, 6:52 p.m. UTC | #3
* 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" <dgilbert@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.

Dave

> 
> Thanks,
> Jianjun
> 
> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@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 <david@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 / dgilbert@redhat.com / Manchester, UK
Jianjun Duan Oct. 17, 2016, 7:02 p.m. UTC | #4
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" <dgilbert@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 <dgilbert@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 <david@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 / dgilbert@redhat.com / Manchester, UK
>
Dr. David Alan Gilbert Oct. 17, 2016, 7:16 p.m. UTC | #5
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> 
> 
> 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" <dgilbert@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?

But then why is that better than having the parent pointer?

Dave

> 
> Thanks,
> Jianjun
> > Dave
> > 
> >>
> >> Thanks,
> >> Jianjun
> >>
> >>>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@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 <david@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 / dgilbert@redhat.com / Manchester, UK
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Jianjun Duan Oct. 17, 2016, 7:30 p.m. UTC | #6
On 10/17/2016 12:16 PM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>
>>
>> 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" <dgilbert@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?
> 
> But then why is that better than having the parent pointer?
> 
IIUC, from the put_tmp, I didn't see how tmp is filled with data. I
suppose it is to be filled by pre_save. So tmp pointer needs to find a
way from inside pre_save to put_tmp. How does it happen?

Thanks,
Jianjun


> Dave
> 
>>
>> Thanks,
>> Jianjun
>>> Dave
>>>
>>>>
>>>> Thanks,
>>>> Jianjun
>>>>
>>>>>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@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 <david@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 / dgilbert@redhat.com / Manchester, UK
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
Dr. David Alan Gilbert Oct. 18, 2016, 8:06 a.m. UTC | #7
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> 
> 
> On 10/17/2016 12:16 PM, Dr. David Alan Gilbert wrote:
> > * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> >>
> >>
> >> 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" <dgilbert@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?
> > 
> > But then why is that better than having the parent pointer?
> > 
> IIUC, from the put_tmp, I didn't see how tmp is filled with data. I
> suppose it is to be filled by pre_save. So tmp pointer needs to find a
> way from inside pre_save to put_tmp. How does it happen?

The tmp.parent pointer is filled by the '*(void **)tmp = pv;' in the put_tmp and
get_tmp.
Only after that is the child vmsd run and it's passed the tmp pointer; it's the
child's pre_save/pre_load that has the chance to do whatever it likes to fill the
tmp.

Dave

> Thanks,
> Jianjun
> 
> 
> > Dave
> > 
> >>
> >> Thanks,
> >> Jianjun
> >>> Dave
> >>>
> >>>>
> >>>> Thanks,
> >>>> Jianjun
> >>>>
> >>>>>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@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 <david@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 / dgilbert@redhat.com / Manchester, UK
> >>>
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

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