diff mbox

[4/5] New VMstate save/load infrastructure

Message ID e93c4a7becda20d35ae11bd07fe743fa91258364.1250601335.git.quintela@redhat.com
State Superseded
Headers show

Commit Message

Juan Quintela Aug. 18, 2009, 1:34 p.m. UTC 
This patch introduces VMState infrastructure, to convert the save/load
functions of devices to a table approach.  This new approach has the
advantages:
- it is type-safe
- you can't have load/save functions out of sync
- will allows us to have new interesting commands, like dump <device>, that
  shows all its internal state.
- Just now, the only added type is arrays, but we can add structures.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |  124 +++++++++++++++++++++++++
 savevm.c |  309 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 427 insertions(+), 6 deletions(-)

Comments

Blue Swirl Aug. 18, 2009, 5:13 p.m. UTC | #1 
On Tue, Aug 18, 2009 at 4:34 PM, Juan Quintela<quintela@redhat.com> wrote:
> This patch introduces VMState infrastructure, to convert the save/load
> functions of devices to a table approach.  This new approach has the
> advantages:
> - it is type-safe
> - you can't have load/save functions out of sync
> - will allows us to have new interesting commands, like dump <device>, that
>  shows all its internal state.
> - Just now, the only added type is arrays, but we can add structures.

I think all the structures should be 'const'. Also VMStateField and
VMStateDescription should be private to savevm.c.
Juan Quintela Aug. 18, 2009, 5:56 p.m. UTC | #2 
Reply-to: quintela@redhat.com
Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Aug 18, 2009 at 4:34 PM, Juan Quintela<quintela@redhat.com> wrote:
>> This patch introduces VMState infrastructure, to convert the save/load
>> functions of devices to a table approach.  This new approach has the
>> advantages:
>> - it is type-safe
>> - you can't have load/save functions out of sync
>> - will allows us to have new interesting commands, like dump <device>, that
>>  shows all its internal state.
>> - Just now, the only added type is arrays, but we can add structures.
>
> I think all the structures should be 'const'.

This is doable, and will do.

> Also VMStateField and
> VMStateDescription should be private to savevm.c.

They can, as we are trying to create them statically, not
programatically.

Later, Juan.
Gerd Hoffmann Aug. 19, 2009, 7:49 a.m. UTC | #3 
> +enum VMStateType {
> +    VMSTATE_TYPE_UNSPEC = 0,
> +    VMSTATE_TYPE_INT8,
> +    VMSTATE_TYPE_INT16,
> +    VMSTATE_TYPE_INT32,
> +    VMSTATE_TYPE_INT64,
> +    VMSTATE_TYPE_UINT8,
> +    VMSTATE_TYPE_UINT16,
> +    VMSTATE_TYPE_UINT32,
> +    VMSTATE_TYPE_UINT64,
> +    VMSTATE_TYPE_TIMER,
> +};
> +
> +typedef struct VMStateInfo VMStateInfo;
> +
> +struct VMStateInfo {
> +    const char *name;
> +    size_t size;
> +    enum VMStateType type;
> +    void (*get)(QEMUFile *f, VMStateInfo *info, void *pv);
> +    void (*put)(QEMUFile *f, VMStateInfo *info, const void *pv);
> +};
> +
> +typedef struct {
> +    const char *name;
> +    size_t num;
> +    size_t offset;
> +    VMStateInfo *info;
> +    int version_id;
> +} VMStateField;

Hmm, I'm not sure VMStateInfo is that useful here.  All the get/put 
callbacks are simple two-liners, and it is unlikely that the number of 
types ever grows.

I think I would stick "enum VMStateType" directly into VMStateField and 
kill one level of indirection.

> +
> +typedef struct {
> +    const char *name;
> +    int version_id;
> +    int minimum_version_id;
> +    VMStateField *fields;
> +} VMStateDescription;

Add compat_load() callback for version_id < minimum_version_id here?

cheers,
   Gerd
Juan Quintela Aug. 19, 2009, 9:38 a.m. UTC | #4 
Reply-to: quintela@redhat.com
Gerd Hoffmann <kraxel@redhat.com> wrote:
>> +enum VMStateType {
>> +    VMSTATE_TYPE_UNSPEC = 0,
>> +    VMSTATE_TYPE_INT8,
>> +    VMSTATE_TYPE_INT16,
>> +    VMSTATE_TYPE_INT32,
>> +    VMSTATE_TYPE_INT64,
>> +    VMSTATE_TYPE_UINT8,
>> +    VMSTATE_TYPE_UINT16,
>> +    VMSTATE_TYPE_UINT32,
>> +    VMSTATE_TYPE_UINT64,
>> +    VMSTATE_TYPE_TIMER,
>> +};
>> +
>> +typedef struct VMStateInfo VMStateInfo;
>> +
>> +struct VMStateInfo {
>> +    const char *name;
>> +    size_t size;
>> +    enum VMStateType type;
>> +    void (*get)(QEMUFile *f, VMStateInfo *info, void *pv);
>> +    void (*put)(QEMUFile *f, VMStateInfo *info, const void *pv);
>> +};
>> +
>> +typedef struct {
>> +    const char *name;
>> +    size_t num;
>> +    size_t offset;
>> +    VMStateInfo *info;
>> +    int version_id;
>> +} VMStateField;
>
> Hmm, I'm not sure VMStateInfo is that useful here.  All the get/put
> callbacks are simple two-liners, and it is unlikely that the number of
> types ever grows.

Thinking about this.  VMStateType is not needed at all (see that it is
not used in the patch).  But Ilike the VMStateInfo struct approach.
This is the easier way of dealing with structs.  For instance
pci_device_save().  My idea was to just create a new VMStateInfo for it,
and then everything that needs to call pci_device_save() just add a
field like:

VMSTATE_FILED(dev, ThisPCIDevice, verion_id, pci_vmstate_info, PCIDevice)

and here, we go.  The reaso that I don't need VMStateType anymore is
that now load/save are done:

+static void vmstate_save(QEMUFile *f, VMStateDescription *vmsd, void *base)
+{
+    VMStateField *field = vmsd->fields;
+    int i;
+
+    while(field->name) {
+        for (i = 0; i < field->num; i++) {
+            field->info->put(f, field->info, base + field->offset +
+                             field->info->size * i);
+        }
+        field++;
+    }
+}

No need to have a type if we have the propper info struct.  As this
struct is going to grow (at least 2 new pointers for printing into
monitor/read value from monitor), I think it is a good idea to maintain
the sharing.

Later, Juan.

> I think I would stick "enum VMStateType" directly into VMStateField
> and kill one level of indirection.
>
>> +
>> +typedef struct {
>> +    const char *name;
>> +    int version_id;
>> +    int minimum_version_id;
>> +    VMStateField *fields;
>> +} VMStateDescription;
>
> Add compat_load() callback for version_id < minimum_version_id here?

I am not sure where/what to add.

I liked your approach of having the new version clean, and let the old
code to deal with the mess that used to be here.  Will take a look at
implementing soemething like that.

Thanks for the review, Juan.
Gerd Hoffmann Aug. 19, 2009, 12:43 p.m. UTC | #5 
Hi,

>> Hmm, I'm not sure VMStateInfo is that useful here.  All the get/put
>> callbacks are simple two-liners, and it is unlikely that the number of
>> types ever grows.
>
> Thinking about this.  VMStateType is not needed at all (see that it is
> not used in the patch).  But Ilike the VMStateInfo struct approach.
> This is the easier way of dealing with structs.  For instance
> pci_device_save().  My idea was to just create a new VMStateInfo for it,
> and then everything that needs to call pci_device_save() just add a
> field like:
>
> VMSTATE_FILED(dev, ThisPCIDevice, verion_id, pci_vmstate_info, PCIDevice)

Hmm.  I had a completely different approach in mind:

In pci.c:

vmstatefield pci_state[] = {
    VMSTATE_FIELD(...),
    [ ... ]
    VMSTATE_FIELD_END_OF_LIST()
}

In a pci driver:

vmstatefield device_state[] = {
    VMSTATE_INCLUDE(&pci_state, ...),
    VMSTATE_FIELD(...),
    [ ... ]
    VMSTATE_FIELD_END_OF_LIST()
}

Advantage: you don't have to write new code for each struct you want to 
save away state information from.

> +static void vmstate_save(QEMUFile *f, VMStateDescription *vmsd, void *base)
> +{
> +    VMStateField *field = vmsd->fields;
> +    int i;
> +
> +    while(field->name) {
> +        for (i = 0; i<  field->num; i++) {
> +            field->info->put(f, field->info, base + field->offset +
> +                             field->info->size * i);
> +        }

The other way would be dropping info and have
"switch(field->type) { ... }" here.

You indeed don't need both info and type.

Properties need the additional type field for typechecking in 
qemu_prop_set_$type() which doesn't use the print()/parse() callbacks.

cheers,
   Gerd
diff mbox

Patch

diff --git a/hw/hw.h b/hw/hw.h
index 1e5783d..19ebef2 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -269,4 +269,128 @@  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
 void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
 int qemu_boot_set(const char *boot_devices);

+enum VMStateType {
+    VMSTATE_TYPE_UNSPEC = 0,
+    VMSTATE_TYPE_INT8,
+    VMSTATE_TYPE_INT16,
+    VMSTATE_TYPE_INT32,
+    VMSTATE_TYPE_INT64,
+    VMSTATE_TYPE_UINT8,
+    VMSTATE_TYPE_UINT16,
+    VMSTATE_TYPE_UINT32,
+    VMSTATE_TYPE_UINT64,
+    VMSTATE_TYPE_TIMER,
+};
+
+typedef struct VMStateInfo VMStateInfo;
+
+struct VMStateInfo {
+    const char *name;
+    size_t size;
+    enum VMStateType type;
+    void (*get)(QEMUFile *f, VMStateInfo *info, void *pv);
+    void (*put)(QEMUFile *f, VMStateInfo *info, const void *pv);
+};
+
+typedef struct {
+    const char *name;
+    size_t num;
+    size_t offset;
+    VMStateInfo *info;
+    int version_id;
+} VMStateField;
+
+typedef struct {
+    const char *name;
+    int version_id;
+    int minimum_version_id;
+    VMStateField *fields;
+} VMStateDescription;
+
+extern VMStateInfo vmstate_info_int8;
+extern VMStateInfo vmstate_info_int16;
+extern VMStateInfo vmstate_info_int32;
+extern VMStateInfo vmstate_info_int64;
+
+extern VMStateInfo vmstate_info_uint8;
+extern VMStateInfo vmstate_info_uint16;
+extern VMStateInfo vmstate_info_uint32;
+extern VMStateInfo vmstate_info_uint64;
+
+extern VMStateInfo vmstate_info_timer;
+
+#define typeof_field2(type, field) typeof(((type *)0)->field)
+#define type_check2(t1,t2) ((t1*)0 - (t2*)0)
+#define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
+
+#define VMSTATE_FIELD(_field, _state, _version, _info, _type) {           \
+    .name       = (stringify(_field)),                                     \
+    .num        = 1,                                                       \
+    .version_id = (_version),                                              \
+    .info       = &(_info),                                                \
+    .offset     = offsetof(_state, _field)                                 \
+            + type_check2(_type,typeof_field2(_state, _field))            \
+}
+
+#define VMSTATE_FIELD_A(_field, _state, _version, _num, _info, _type) {   \
+    .name       = (stringify(_field)),                                     \
+    .num        = (_num),                                                  \
+    .version_id = (_version),                                              \
+    .info       = &(_info),                                                \
+    .offset     = offsetof(_state, _field)                                 \
+            + type_check_array(_type,typeof_field2(_state, _field),_num)  \
+}
+
+/* _f : field name
+   _s : struct state name
+   _v : version
+   _n : num of elements
+*/
+
+
+#define VMSTATE_INT8_ARRAY(_f, _s, _v, _n)                            \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_int8, int8_t)
+#define VMSTATE_INT16_ARRAY(_f, _s, _v, _n)                           \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_int16, int16_t)
+#define VMSTATE_INT32_ARRAY(_f, _s, _v, _n)                           \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_int32, int32_t)
+#define VMSTATE_INT64_ARRAY(_f, _s, _v, _n)                           \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_int64, int64_t)
+
+#define VMSTATE_UINT8_ARRAY(_f, _s, _v, _n)                           \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_uint8, uint8_t)
+#define VMSTATE_UINT16_ARRAY(_f, _s, _v, _n)                          \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_uint16, uint16_t)
+#define VMSTATE_UINT32_ARRAY(_f, _s, _v, _n)                          \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_uint32, uint32_t)
+#define VMSTATE_UINT64_ARRAY(_f, _s, _v, _n)                          \
+    VMSTATE_FIELD_A(_f, _s, _v, _n, vmstate_info_uint64, uint64_t)
+
+#define VMSTATE_INT8(_f, _s, _v)                                      \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_int8, int8_t)
+#define VMSTATE_INT16(_f, _s, _v)                                     \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_int16, int16_t)
+#define VMSTATE_INT32(_f, _s, _v)                                     \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_int32, int32_t)
+#define VMSTATE_INT64(_f, _s, _v)                                     \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_int64, int64_t)
+
+#define VMSTATE_UINT8(_f, _s, _v)                                     \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_uint8, uint8_t)
+#define VMSTATE_UINT16(_f, _s, _v)                                    \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_uint16, uint16_t)
+#define VMSTATE_UINT32(_f, _s, _v)                                    \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_uint32, uint32_t)
+#define VMSTATE_UINT64(_f, _s, _v)                                    \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_uint64, uint64_t)
+
+#define VMSTATE_TIMER(_f, _s, _v)                                     \
+    VMSTATE_FIELD(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
+
+#define VMSTATE_END_OF_LIST()               \
+    {}
+
+extern int vmstate_register(int instance_id, VMStateDescription *vmsd,
+                            void *base);
+extern void vmstate_unregister(const char *idstr, void *opaque);
 #endif
diff --git a/savevm.c b/savevm.c
index 55a1b50..aff4c82 100644
--- a/savevm.c
+++ b/savevm.c
@@ -610,6 +610,204 @@  uint64_t qemu_get_be64(QEMUFile *f)
     return v;
 }

+/* 8 bit int */
+
+static void get_int8(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    int8_t *v = pv;
+    qemu_get_s8s(f, v);
+}
+
+static void put_int8(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const int8_t *v = pv;
+    qemu_put_s8s(f, v);
+}
+
+VMStateInfo vmstate_info_int8 = {
+    .name  = "int8",
+    .type  = VMSTATE_TYPE_INT8,
+    .size  = sizeof(int8_t),
+    .get = get_int8,
+    .put = put_int8,
+};
+
+/* 16 bit int */
+
+static void get_int16(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    int16_t *v = pv;
+    qemu_get_sbe16s(f, v);
+}
+
+static void put_int16(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const int16_t *v = pv;
+    qemu_put_sbe16s(f, v);
+}
+
+VMStateInfo vmstate_info_int16 = {
+    .name  = "int16",
+    .type  = VMSTATE_TYPE_INT16,
+    .size  = sizeof(int16_t),
+    .get = get_int16,
+    .put = put_int16,
+};
+
+/* 32 bit int */
+
+static void get_int32(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    int32_t *v = pv;
+    qemu_get_sbe32s(f, v);
+}
+
+static void put_int32(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const int32_t *v = pv;
+    qemu_put_sbe32s(f, v);
+}
+
+VMStateInfo vmstate_info_int32 = {
+    .name  = "int32",
+    .type  = VMSTATE_TYPE_INT32,
+    .size  = sizeof(int32_t),
+    .get = get_int32,
+    .put = put_int32,
+};
+
+/* 64 bit int */
+
+static void get_int64(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    int64_t *v = pv;
+    qemu_get_sbe64s(f, v);
+}
+
+static void put_int64(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const int64_t *v = pv;
+    qemu_put_sbe64s(f, v);
+}
+
+VMStateInfo vmstate_info_int64 = {
+    .name  = "int64",
+    .type  = VMSTATE_TYPE_INT64,
+    .size  = sizeof(int64_t),
+    .get = get_int64,
+    .put = put_int64,
+};
+
+/* 8 bit unsigned int */
+
+static void get_uint8(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    uint8_t *v = pv;
+    qemu_get_8s(f, v);
+}
+
+static void put_uint8(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const uint8_t *v = pv;
+    qemu_put_8s(f, v);
+}
+
+VMStateInfo vmstate_info_uint8 = {
+    .name  = "uint8",
+    .type  = VMSTATE_TYPE_UINT8,
+    .size  = sizeof(uint8_t),
+    .get = get_uint8,
+    .put = put_uint8,
+};
+
+/* 16 bit unsigned int */
+
+static void get_uint16(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    uint16_t *v = pv;
+    qemu_get_be16s(f, v);
+}
+
+static void put_uint16(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const uint16_t *v = pv;
+    qemu_put_be16s(f, v);
+}
+
+VMStateInfo vmstate_info_uint16 = {
+    .name  = "uint16",
+    .type  = VMSTATE_TYPE_UINT16,
+    .size  = sizeof(uint16_t),
+    .get = get_uint16,
+    .put = put_uint16,
+};
+
+/* 32 bit unsigned int */
+
+static void get_uint32(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    uint32_t *v = pv;
+    qemu_get_be32s(f, v);
+}
+
+static void put_uint32(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const uint32_t *v = pv;
+    qemu_put_be32s(f, v);
+}
+
+VMStateInfo vmstate_info_uint32 = {
+    .name  = "uint32",
+    .type  = VMSTATE_TYPE_UINT32,
+    .size  = sizeof(uint32_t),
+    .get = get_uint32,
+    .put = put_uint32,
+};
+
+/* 64 bit unsigned int */
+
+static void get_uint64(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    uint64_t *v = pv;
+    qemu_get_be64s(f, v);
+}
+
+static void put_uint64(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    const uint64_t *v = pv;
+    qemu_put_be64s(f, v);
+}
+
+VMStateInfo vmstate_info_uint64 = {
+    .name  = "uint64",
+    .type  = VMSTATE_TYPE_UINT64,
+    .size  = sizeof(uint64_t),
+    .get = get_uint64,
+    .put = put_uint64,
+};
+
+/* timers  */
+
+static void get_timer(QEMUFile *f, VMStateInfo *info, void *pv)
+{
+    QEMUTimer **v = pv;
+    qemu_get_timer(f, *v);
+}
+
+static void put_timer(QEMUFile *f, VMStateInfo *info, const void *pv)
+{
+    QEMUTimer **v = (void *)pv;
+    qemu_put_timer(f, *v);
+}
+
+VMStateInfo vmstate_info_timer = {
+    .name  = "timer",
+    .type  = VMSTATE_TYPE_TIMER,
+    .size  = sizeof(uint64_t),
+    .get = get_timer,
+    .put = put_timer,
+};
+
 typedef struct SaveStateEntry {
     char idstr[256];
     int instance_id;
@@ -618,11 +816,13 @@  typedef struct SaveStateEntry {
     SaveLiveStateHandler *save_live_state;
     SaveStateHandler *save_state;
     LoadStateHandler *load_state;
+    VMStateDescription *vmsd;
     void *opaque;
     struct SaveStateEntry *next;
 } SaveStateEntry;

 static SaveStateEntry *first_se;
+static int global_section_id;

 /* TODO: Individual devices generally have very little idea about the rest
    of the system, so instance_id should be removed/replaced.
@@ -637,7 +837,6 @@  int register_savevm_live(const char *idstr,
                          void *opaque)
 {
     SaveStateEntry *se, **pse;
-    static int global_section_id;

     se = qemu_malloc(sizeof(SaveStateEntry));
     pstrcpy(se->idstr, sizeof(se->idstr), idstr);
@@ -648,6 +847,7 @@  int register_savevm_live(const char *idstr,
     se->save_state = save_state;
     se->load_state = load_state;
     se->opaque = opaque;
+    se->vmsd = NULL;
     se->next = NULL;

     /* add at the end of list */
@@ -690,6 +890,88 @@  void unregister_savevm(const char *idstr, void *opaque)
     }
 }

+int vmstate_register(int instance_id, VMStateDescription *vmsd,
+                     void *opaque)
+{
+    SaveStateEntry *se, **pse;
+
+    se = qemu_malloc(sizeof(SaveStateEntry));
+    pstrcpy(se->idstr, sizeof(se->idstr), vmsd->name);
+    se->instance_id = (instance_id == -1) ? 0 : instance_id;
+    se->version_id = vmsd->version_id;
+    se->section_id = global_section_id++;
+    se->save_live_state = NULL;
+    se->save_state = NULL;
+    se->load_state = NULL;
+    se->opaque = opaque;
+    se->vmsd = vmsd;
+    se->next = NULL;
+
+    /* add at the end of list */
+    pse = &first_se;
+    while (*pse != NULL) {
+        if (instance_id == -1
+                && strcmp(se->idstr, (*pse)->idstr) == 0
+                && se->instance_id <= (*pse)->instance_id)
+            se->instance_id = (*pse)->instance_id + 1;
+        pse = &(*pse)->next;
+    }
+    *pse = se;
+    return 0;
+}
+
+void vmstate_unregister(const char *idstr,  void *opaque)
+{
+    SaveStateEntry **pse;
+
+    pse = &first_se;
+    while (*pse != NULL) {
+        if (strcmp((*pse)->idstr, idstr) == 0 && (*pse)->opaque == opaque) {
+            SaveStateEntry *next = (*pse)->next;
+            qemu_free(*pse);
+            *pse = next;
+            continue;
+        }
+        pse = &(*pse)->next;
+    }
+}
+
+static void vmstate_save(QEMUFile *f, VMStateDescription *vmsd, void *base)
+{
+    VMStateField *field = vmsd->fields;
+    int i;
+
+    while(field->name) {
+        for (i = 0; i < field->num; i++) {
+            field->info->put(f, field->info, base + field->offset +
+                             field->info->size * i);
+        }
+        field++;
+    }
+}
+
+static int vmstate_load(QEMUFile *f, VMStateDescription *vmsd, void *base,
+                        int version_id)
+{
+    VMStateField *field = vmsd->fields;
+    int i;
+
+    if (version_id > vmsd->version_id ||
+        version_id < vmsd->minimum_version_id)
+        return -EINVAL;
+
+    while(field->name) {
+        for (i = 0; i < field->num; i++) {
+            if (field->version_id <= version_id) {
+                field->info->get(f, field->info, base + field->offset +
+                                 field->info->size * i);
+            }
+        }
+        field++;
+    }
+    return 0;
+}
+
 #define QEMU_VM_FILE_MAGIC           0x5145564d
 #define QEMU_VM_FILE_VERSION_COMPAT  0x00000002
 #define QEMU_VM_FILE_VERSION         0x00000003
@@ -777,7 +1059,7 @@  int qemu_savevm_state_complete(QEMUFile *f)
     for(se = first_se; se != NULL; se = se->next) {
         int len;

-	if (se->save_state == NULL)
+	if (se->save_state == NULL && se->vmsd == NULL)
 	    continue;

         /* Section type */
@@ -792,7 +1074,10 @@  int qemu_savevm_state_complete(QEMUFile *f)
         qemu_put_be32(f, se->instance_id);
         qemu_put_be32(f, se->version_id);

-        se->save_state(f, se->opaque);
+        if (se->vmsd != NULL)
+            vmstate_save(f, se->vmsd, se->opaque);
+        else
+            se->save_state(f, se->opaque);
     }

     qemu_put_byte(f, QEMU_VM_EOF);
@@ -878,7 +1163,11 @@  static int qemu_loadvm_state_v2(QEMUFile *f)
             fprintf(stderr, "qemu: warning: instance 0x%x of device '%s' not present in current VM\n",
                     instance_id, idstr);
         } else {
-            ret = se->load_state(f, se->opaque, version_id);
+            if (se->vmsd) {
+                ret = vmstate_load(f, se->vmsd, se->opaque, version_id);
+            } else {
+                ret = se->load_state(f, se->opaque, version_id);
+            }
             if (ret < 0) {
                 fprintf(stderr, "qemu: warning: error while loading state for instance 0x%x of device '%s'\n",
                         instance_id, idstr);
@@ -955,7 +1244,11 @@  int qemu_loadvm_state(QEMUFile *f)
             le->next = first_le;
             first_le = le;

-            ret = le->se->load_state(f, le->se->opaque, le->version_id);
+            if (le->se->vmsd) {
+                ret = vmstate_load(f, le->se->vmsd, le->se->opaque, le->version_id);
+            } else {
+                ret = le->se->load_state(f, le->se->opaque, le->version_id);
+            }
             if (ret < 0) {
                 fprintf(stderr, "qemu: warning: error while loading state for instance 0x%x of device '%s'\n",
                         instance_id, idstr);
@@ -974,7 +1267,11 @@  int qemu_loadvm_state(QEMUFile *f)
                 goto out;
             }

-            ret = le->se->load_state(f, le->se->opaque, le->version_id);
+            if (le->se->vmsd) {
+                ret = vmstate_load(f, le->se->vmsd, le->se->opaque, le->version_id);
+            } else {
+                ret = le->se->load_state(f, le->se->opaque, le->version_id);
+            }
             if (ret != 0) {
                 fprintf(stderr, "qemu: warning: error while loading state section '%d'\n",
                         section_id);