[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP
From: |
Jianjun Duan |
Subject: |
Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP |
Date: |
Mon, 17 Oct 2016 12:02:30 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 10/17/2016 11:52 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (address@hidden) 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" <address@hidden>
>>>>
>>>> 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 <address@hidden>
>>>
>>> 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 <address@hidden>
>>>
>>>> ---
>>>> 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 / address@hidden / Manchester, UK
>
- [Qemu-devel] [very-WIP 0/4] Migration: VMSTATE_WITH_TMP, Dr. David Alan Gilbert (git), 2016/10/11
- [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP, Dr. David Alan Gilbert (git), 2016/10/11
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP, David Gibson, 2016/10/16
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP, Jianjun Duan, 2016/10/17
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP, Dr. David Alan Gilbert, 2016/10/17
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP,
Jianjun Duan <=
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP, Dr. David Alan Gilbert, 2016/10/17
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP, Jianjun Duan, 2016/10/17
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP, Dr. David Alan Gilbert, 2016/10/18
[Qemu-devel] [very-WIP 2/7] tests/migration: Add test for VMSTATE_WITH_TMP, Dr. David Alan Gilbert (git), 2016/10/11
[Qemu-devel] [very-WIP 3/4] slirp: VMStatify sbuf, Dr. David Alan Gilbert (git), 2016/10/11