[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: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP |
Date: |
Tue, 18 Oct 2016 09:06:24 +0100 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
* Jianjun Duan (address@hidden) wrote:
>
>
> On 10/17/2016 12:16 PM, Dr. David Alan Gilbert wrote:
> > * Jianjun Duan (address@hidden) wrote:
> >>
> >>
> >> 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?
> >
> > 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 <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
> >>>
> >>
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
> >
>
--
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, 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, 2016/10/17
- Re: [Qemu-devel] [very-WIP 1/7] migration: Add VMSTATE_WITH_TMP,
Dr. David Alan Gilbert <=
[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
[Qemu-devel] [very-WIP 4/4] virtio/migration: Migrate virtio-net to VMState, Dr. David Alan Gilbert (git), 2016/10/11