[Qemu-devel] [PATCH 00/23] New VMState table based load/save infrastructure

[Qemu-devel] [PATCH 00/23] New VMState table based load/save infrastructure

[Juan Quintela]

This this is 3rd version of the series, I changed:

v3:
* Patches are again on top of my loadvm changes, as they use the new
  loadvm functionality.
* Add size paramente to VMState field, it is needed for buffers
* Redo all the STRUCT and ARRAY support. It makes easier to have combinations of
  things.
* Once there created:
  * POINTER: field is a pointer to the real thing
  * VARRAY: This is one array whose size is another field
  * BUFFER: A traditional array. So far it is only for static sized buffers.
            No need for variable sized buffers yet.
* PCIBUS ported to new API
* PCIDevice ported to new API. Here is an example of how to create your own
  function in the config array.  Basically PCI wants to read the array
  and sanitize it before doing anything else with it.
* Add run_after_load() callback.
* Port PS2 devices
* Port ACPI (uses PCIDevice)
* Put const left, right, and center. 

ToDo:
- still not optional test function.  Notice that this can be done
  with a local VMStateInfo struct.  I still think that this is not
  needed.
- Start/end functions: We have run_after_load(), problaby will ned some more
  uhci_save() calls uhci_async_cancel_all() before saving its state.
- Better/Different design for containers?  Just now we can have things like:
  an array of structs pointed from a pointer of dynamic size.
  This means having to create a new a new VMSTATE_FOO() construct.
  Any idea about how to improve this?  I would like something that is componable:
  ARRAY(POINTER(INT32(field, struct), extra), extra2) or somesuch.
  Haven't found a good way to abuse cpp to get this.
- I tested that it loads qemu/master images. It can save/load new ones.
- Next try will be to convert the virtio-* drivers. Basically everything else
  should work with current abstractions.  virtion_load() is a mess, but I think
  that I can get it working with before/after loading functions.
- If you know of any device that does strange things with state, please, let me
  know, to start for there.
- i440fx_load()
  Can we put pci_irq_levels[] array in any struct?  This is the only driver
  that I have found that uses a global variable.  Gerd, can you take a look?
  It is not possible to have the PCIDevice struct inside a PIIXDevice or similar?

Comments, suggestions?

v2:
* Add _V() constructors.  We almost never need the version field.
* Add const to VMStateDescription uses (BlueWirl suggestion)
* Add const to VMStateInfo uses
* Remove VMStateInfo parameter from get/put callbacks. Not needed.
* Load of old versions is done with old foo_load function (Kraxel suggestion)
* Add struct support
* Move i8254 to new infrastructure, to test struct support
* I removed the autostart patches, updated version sent to list

i8254 note:
There is only one timer in the 1st channel, in the other two channels,
the timer is not created ever, this is the reason why I sent the irq_timer
not in the irq channels.

Juan Quintela (23):
  move useful type definitons to osdep.h
  split do_loadvm() into do_loadvm() and load_vmstate()
  move do_loadvm() to monitor.c
  make load_vmstate() return errors
  Use return value from load_state() call back
  Add vmstate_load() and vmstate_save() functions
  New VMstate save/load infrastructure
  Add VMState support for pointers
  Add VMState support for arrays
  Port apic to new VMState design
  Add VMState support for structs
  Add VMState support for arrays of structs
  Port i8254 to new VMState design
  Add VMState support for int32_t check value
  Add VMState support for variable sized arrays
  Port PCI Bus to VMState design
  Add VMState support for static sized buffers (uint_8)
  Port PS2 devices to VMState design
  Add VMState support for int32_t check value
  Add version_id to PCIDevice.
  Port PCIDevice state to VMState
  Add VMState support to run a function after load
  Port ACPI to VMState

 hw/acpi.c  |   55 +++-----
 hw/apic.c  |   67 +++++-----
 hw/hw.h    |  234 ++++++++++++++++++++++++++++++++
 hw/i8254.c |   66 +++++----
 hw/pci.c   |  107 ++++++++-------
 hw/pci.h   |    2 +
 hw/ps2.c   |  119 +++++++---------
 hw/qdev.h  |    3 -
 monitor.c  |   10 ++
 osdep.h    |    3 +
 savevm.c   |  434 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 sysemu.h   |    2 +-
 vl.c       |    7 +-
 13 files changed, 869 insertions(+), 240 deletions(-)

[Qemu-devel] [PATCH 01/23] move useful type definitons to osdep.h

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/qdev.h |    3 ---
 osdep.h   |    3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/qdev.h b/hw/qdev.h
index 204c4e5..a75de6f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -163,9 +163,6 @@ extern PropertyInfo qdev_prop_macaddr;
 extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_pci_devfn;

-#define typeof_field(type, field) typeof(((type *)0)->field)
-#define type_check(t1,t2) ((t1*)0 - (t2*)0)
-
 #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
         .name      = (_name),                                    \
         .info      = &(_prop),                                   \
diff --git a/osdep.h b/osdep.h
index 2517b1a..32ee94b 100644
--- a/osdep.h
+++ b/osdep.h
@@ -36,6 +36,9 @@
         (type *) ((char *) __mptr - offsetof(type, member));})
 #endif

+#define typeof_field(type, field) typeof(((type *)0)->field)
+#define type_check(t1,t2) ((t1*)0 - (t2*)0)
+
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
 #endif
-- 
1.6.2.5

[Qemu-devel] [PATCH 02/23] split do_loadvm() into do_loadvm() and load_vmstate()

[Juan Quintela]

do_loadvm() is now called from the monitor.
load_vmstate() is called by do_loadvm() and when -loadvm command line is used.
Command line don't have to play games with vmstop()/vmstart()

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   22 +++++++++++++---------
 sysemu.h |    1 +
 vl.c     |    2 +-
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/savevm.c b/savevm.c
index 570377f..a321136 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1145,14 +1145,13 @@ void do_savevm(Monitor *mon, const char *name)
         vm_start();
 }

-void do_loadvm(Monitor *mon, const char *name)
+void load_vmstate(Monitor *mon, const char *name)
 {
     DriveInfo *dinfo;
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo sn;
     QEMUFile *f;
     int ret;
-    int saved_vm_running;

     bs = get_bs_snapshots();
     if (!bs) {
@@ -1163,9 +1162,6 @@ void do_loadvm(Monitor *mon, const char *name)
     /* Flush all IO requests so they don't interfere with the new state.  */
     qemu_aio_flush();

-    saved_vm_running = vm_running;
-    vm_stop(0);
-
     TAILQ_FOREACH(dinfo, &drives, next) {
         bs1 = dinfo->bdrv;
         if (bdrv_has_snapshot(bs1)) {
@@ -1191,7 +1187,7 @@ void do_loadvm(Monitor *mon, const char *name)
                 }
                 /* fatal on snapshot block device */
                 if (bs == bs1)
-                    goto the_end;
+                    return;
             }
         }
     }
@@ -1199,20 +1195,28 @@ void do_loadvm(Monitor *mon, const char *name)
     /* Don't even try to load empty VM states */
     ret = bdrv_snapshot_find(bs, &sn, name);
     if ((ret >= 0) && (sn.vm_state_size == 0))
-        goto the_end;
+        return;

     /* restore the VM state */
     f = qemu_fopen_bdrv(bs, 0);
     if (!f) {
         monitor_printf(mon, "Could not open VM state file\n");
-        goto the_end;
+        return;
     }
     ret = qemu_loadvm_state(f);
     qemu_fclose(f);
     if (ret < 0) {
         monitor_printf(mon, "Error %d while loading VM state\n", ret);
     }
- the_end:
+}
+
+void do_loadvm(Monitor *mon, const char *name)
+{
+    int saved_vm_running  = vm_running;
+
+    vm_stop(0);
+
+    load_vmstate(mon, name);
     if (saved_vm_running)
         vm_start();
 }
diff --git a/sysemu.h b/sysemu.h
index dffb2f1..a20025d 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -52,6 +52,7 @@ void qemu_system_reset(void);

 void do_savevm(Monitor *mon, const char *name);
 void do_loadvm(Monitor *mon, const char *name);
+void load_vmstate(Monitor *mon, const char *name);
 void do_delvm(Monitor *mon, const char *name);
 void do_info_snapshots(Monitor *mon);

diff --git a/vl.c b/vl.c
index 8b2b289..13a2b74 100644
--- a/vl.c
+++ b/vl.c
@@ -6037,7 +6037,7 @@ int main(int argc, char **argv, char **envp)
     }

     if (loadvm)
-        do_loadvm(cur_mon, loadvm);
+        load_vmstate(cur_mon, loadvm);

     if (incoming) {
         autostart = 0;
-- 
1.6.2.5

[Qemu-devel] [PATCH 03/23] move do_loadvm() to monitor.c

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 monitor.c |   11 +++++++++++
 savevm.c  |   11 -----------
 sysemu.h  |    1 -
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/monitor.c b/monitor.c
index 362322b..b8a47ca 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1776,6 +1776,17 @@ static void do_closefd(Monitor *mon, const char *fdname)
                    fdname);
 }

+static void do_loadvm(Monitor *mon, const char *name)
+{
+    int saved_vm_running  = vm_running;
+
+    vm_stop(0);
+
+    load_vmstate(mon, name);
+    if (saved_vm_running)
+        vm_start();
+}
+
 int monitor_get_fd(Monitor *mon, const char *fdname)
 {
     mon_fd_t *monfd;
diff --git a/savevm.c b/savevm.c
index a321136..d0ed2ad 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1210,17 +1210,6 @@ void load_vmstate(Monitor *mon, const char *name)
     }
 }

-void do_loadvm(Monitor *mon, const char *name)
-{
-    int saved_vm_running  = vm_running;
-
-    vm_stop(0);
-
-    load_vmstate(mon, name);
-    if (saved_vm_running)
-        vm_start();
-}
-
 void do_delvm(Monitor *mon, const char *name)
 {
     DriveInfo *dinfo;
diff --git a/sysemu.h b/sysemu.h
index a20025d..330386c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -51,7 +51,6 @@ extern qemu_irq qemu_system_powerdown;
 void qemu_system_reset(void);

 void do_savevm(Monitor *mon, const char *name);
-void do_loadvm(Monitor *mon, const char *name);
 void load_vmstate(Monitor *mon, const char *name);
 void do_delvm(Monitor *mon, const char *name);
 void do_info_snapshots(Monitor *mon);
-- 
1.6.2.5

[Qemu-devel] [PATCH 04/23] make load_vmstate() return errors

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 monitor.c |    3 +--
 savevm.c  |   12 +++++++-----
 sysemu.h  |    2 +-
 vl.c      |    7 +++++--
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/monitor.c b/monitor.c
index b8a47ca..ea5c33a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1782,8 +1782,7 @@ static void do_loadvm(Monitor *mon, const char *name)

     vm_stop(0);

-    load_vmstate(mon, name);
-    if (saved_vm_running)
+    if (load_vmstate(mon, name) >= 0 && saved_vm_running)
         vm_start();
 }

diff --git a/savevm.c b/savevm.c
index d0ed2ad..4868285 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1145,7 +1145,7 @@ void do_savevm(Monitor *mon, const char *name)
         vm_start();
 }

-void load_vmstate(Monitor *mon, const char *name)
+int load_vmstate(Monitor *mon, const char *name)
 {
     DriveInfo *dinfo;
     BlockDriverState *bs, *bs1;
@@ -1156,7 +1156,7 @@ void load_vmstate(Monitor *mon, const char *name)
     bs = get_bs_snapshots();
     if (!bs) {
         monitor_printf(mon, "No block device supports snapshots\n");
-        return;
+        return -EINVAL;
     }

     /* Flush all IO requests so they don't interfere with the new state.  */
@@ -1187,7 +1187,7 @@ void load_vmstate(Monitor *mon, const char *name)
                 }
                 /* fatal on snapshot block device */
                 if (bs == bs1)
-                    return;
+                    return 0;
             }
         }
     }
@@ -1195,19 +1195,21 @@ void load_vmstate(Monitor *mon, const char *name)
     /* Don't even try to load empty VM states */
     ret = bdrv_snapshot_find(bs, &sn, name);
     if ((ret >= 0) && (sn.vm_state_size == 0))
-        return;
+        return -EINVAL;

     /* restore the VM state */
     f = qemu_fopen_bdrv(bs, 0);
     if (!f) {
         monitor_printf(mon, "Could not open VM state file\n");
-        return;
+        return -EINVAL;
     }
     ret = qemu_loadvm_state(f);
     qemu_fclose(f);
     if (ret < 0) {
         monitor_printf(mon, "Error %d while loading VM state\n", ret);
+        return ret;
     }
+    return 0;
 }

 void do_delvm(Monitor *mon, const char *name)
diff --git a/sysemu.h b/sysemu.h
index 330386c..e0338ce 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -51,7 +51,7 @@ extern qemu_irq qemu_system_powerdown;
 void qemu_system_reset(void);

 void do_savevm(Monitor *mon, const char *name);
-void load_vmstate(Monitor *mon, const char *name);
+int load_vmstate(Monitor *mon, const char *name);
 void do_delvm(Monitor *mon, const char *name);
 void do_info_snapshots(Monitor *mon);

diff --git a/vl.c b/vl.c
index 13a2b74..0a104a6 100644
--- a/vl.c
+++ b/vl.c
@@ -6036,8 +6036,11 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }

-    if (loadvm)
-        load_vmstate(cur_mon, loadvm);
+    if (loadvm) {
+        if (load_vmstate(cur_mon, loadvm) < 0) {
+            autostart = 0;
+        }
+    }

     if (incoming) {
         autostart = 0;
-- 
1.6.2.5

[Qemu-devel] [PATCH 05/23] Use return value from load_state() call back

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/savevm.c b/savevm.c
index 4868285..ffa68da 100644
--- a/savevm.c
+++ b/savevm.c
@@ -955,7 +955,12 @@ int qemu_loadvm_state(QEMUFile *f)
             le->next = first_le;
             first_le = le;

-            le->se->load_state(f, le->se->opaque, le->version_id);
+            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);
+                goto out;
+            }
             break;
         case QEMU_VM_SECTION_PART:
         case QEMU_VM_SECTION_END:
@@ -968,7 +973,12 @@ int qemu_loadvm_state(QEMUFile *f)
                 goto out;
             }

-            le->se->load_state(f, le->se->opaque, le->version_id);
+            ret = le->se->load_state(f, le->se->opaque, le->version_id);
+            if (ret < 0) {
+                fprintf(stderr, "qemu: warning: error while loading state section id %d\n",
+                        section_id);
+                goto out;
+            }
             break;
         default:
             fprintf(stderr, "Unknown savevm section type %d\n", section_type);
-- 
1.6.2.5

[Qemu-devel] [PATCH 06/23] Add vmstate_load() and vmstate_save() functions

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/savevm.c b/savevm.c
index ffa68da..2edea5e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -690,6 +690,17 @@ void unregister_savevm(const char *idstr, void *opaque)
     }
 }

+static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
+{
+    return se->load_state(f, se->opaque, version_id);
+}
+
+static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
+{
+    se->save_state(f, se->opaque);
+}
+
+
 #define QEMU_VM_FILE_MAGIC           0x5145564d
 #define QEMU_VM_FILE_VERSION_COMPAT  0x00000002
 #define QEMU_VM_FILE_VERSION         0x00000003
@@ -792,7 +803,7 @@ 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);
+        vmstate_save(f, se);
     }

     qemu_put_byte(f, QEMU_VM_EOF);
@@ -878,7 +889,7 @@ 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);
+            ret = vmstate_load(f, se, 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 +966,7 @@ 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);
+            ret = vmstate_load(f, le->se, 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);
@@ -973,7 +984,7 @@ int qemu_loadvm_state(QEMUFile *f)
                 goto out;
             }

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

[Qemu-devel] [PATCH 07/23] New VMstate save/load infrastructure

[Juan Quintela]

This patch introduces VMState infrastructure, to convert the save/load
functions of devices to a table approach.  This new approach has the
following 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.
- Uses old load_state() function for loading old state.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |  102 +++++++++++++++++++++++
 savevm.c |  273 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 370 insertions(+), 5 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 1e5783d..c58b416 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -269,4 +269,106 @@ 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);

+typedef struct VMStateInfo VMStateInfo;
+typedef struct VMStateDescription VMStateDescription;
+
+struct VMStateInfo {
+    const char *name;
+    int (*get)(QEMUFile *f, void *pv, size_t size);
+    void (*put)(QEMUFile *f, const void *pv, size_t size);
+};
+
+enum VMStateFlags {
+    VMS_SINGLE  = 0x001,
+};
+
+typedef struct {
+    const char *name;
+    size_t offset;
+    size_t size;
+    const VMStateInfo *info;
+    enum VMStateFlags flags;
+    int version_id;
+} VMStateField;
+
+struct VMStateDescription {
+    const char *name;
+    int version_id;
+    int minimum_version_id;
+    int minimum_version_id_old;
+    LoadStateHandler *load_state_old;
+    VMStateField *fields;
+};
+
+extern const VMStateInfo vmstate_info_int8;
+extern const VMStateInfo vmstate_info_int16;
+extern const VMStateInfo vmstate_info_int32;
+extern const VMStateInfo vmstate_info_int64;
+
+extern const VMStateInfo vmstate_info_uint8;
+extern const VMStateInfo vmstate_info_uint16;
+extern const VMStateInfo vmstate_info_uint32;
+extern const VMStateInfo vmstate_info_uint64;
+
+#define VMSTATE_SINGLE(_field, _state, _version, _info, _type) {     \
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .size       = sizeof(_type),                                     \
+    .info       = &(_info),                                          \
+    .flags      = VMS_SINGLE,                                        \
+    .offset     = offsetof(_state, _field)                           \
+            + type_check(_type,typeof_field(_state, _field))         \
+}
+
+/* _f : field name
+   _s : struct state name
+   _v : version
+*/
+
+#define VMSTATE_INT8_V(_f, _s, _v)                                    \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int8, int8_t)
+#define VMSTATE_INT16_V(_f, _s, _v)                                   \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int16, int16_t)
+#define VMSTATE_INT32_V(_f, _s, _v)                                   \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int32, int32_t)
+#define VMSTATE_INT64_V(_f, _s, _v)                                   \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_int64, int64_t)
+
+#define VMSTATE_UINT8_V(_f, _s, _v)                                   \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint8, uint8_t)
+#define VMSTATE_UINT16_V(_f, _s, _v)                                  \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint16, uint16_t)
+#define VMSTATE_UINT32_V(_f, _s, _v)                                  \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint32, uint32_t)
+#define VMSTATE_UINT64_V(_f, _s, _v)                                  \
+    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint64, uint64_t)
+
+#define VMSTATE_INT8(_f, _s)                                          \
+    VMSTATE_INT8_V(_f, _s, 0)
+#define VMSTATE_INT16(_f, _s)                                         \
+    VMSTATE_INT16_V(_f, _s, 0)
+#define VMSTATE_INT32(_f, _s)                                         \
+    VMSTATE_INT32_V(_f, _s, 0)
+#define VMSTATE_INT64(_f, _s)                                         \
+    VMSTATE_INT64_V(_f, _s, 0)
+
+#define VMSTATE_UINT8(_f, _s)                                         \
+    VMSTATE_UINT8_V(_f, _s, 0)
+#define VMSTATE_UINT16(_f, _s)                                        \
+    VMSTATE_UINT16_V(_f, _s, 0)
+#define VMSTATE_UINT32(_f, _s)                                        \
+    VMSTATE_UINT32_V(_f, _s, 0)
+#define VMSTATE_UINT64(_f, _s)                                        \
+    VMSTATE_UINT64_V(_f, _s, 0)
+
+#define VMSTATE_END_OF_LIST()                                         \
+    {}
+
+extern int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
+                              void *opaque, int version_id);
+extern void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
+                               const void *opaque);
+extern int vmstate_register(int instance_id, const VMStateDescription *vmsd,
+                            void *base);
+extern void vmstate_unregister(const char *idstr, void *opaque);
 #endif
diff --git a/savevm.c b/savevm.c
index 2edea5e..400d1c0 100644
--- a/savevm.c
+++ b/savevm.c
@@ -610,6 +610,174 @@ uint64_t qemu_get_be64(QEMUFile *f)
     return v;
 }

+/* 8 bit int */
+
+static int get_int8(QEMUFile *f, void *pv, size_t size)
+{
+    int8_t *v = pv;
+    qemu_get_s8s(f, v);
+    return 0;
+}
+
+static void put_int8(QEMUFile *f, const void *pv, size_t size)
+{
+    const int8_t *v = pv;
+    qemu_put_s8s(f, v);
+}
+
+const VMStateInfo vmstate_info_int8 = {
+    .name = "int8",
+    .get  = get_int8,
+    .put  = put_int8,
+};
+
+/* 16 bit int */
+
+static int get_int16(QEMUFile *f, void *pv, size_t size)
+{
+    int16_t *v = pv;
+    qemu_get_sbe16s(f, v);
+    return 0;
+}
+
+static void put_int16(QEMUFile *f, const void *pv, size_t size)
+{
+    const int16_t *v = pv;
+    qemu_put_sbe16s(f, v);
+}
+
+const VMStateInfo vmstate_info_int16 = {
+    .name = "int16",
+    .get  = get_int16,
+    .put  = put_int16,
+};
+
+/* 32 bit int */
+
+static int get_int32(QEMUFile *f, void *pv, size_t size)
+{
+    int32_t *v = pv;
+    qemu_get_sbe32s(f, v);
+    return 0;
+}
+
+static void put_int32(QEMUFile *f, const void *pv, size_t size)
+{
+    const int32_t *v = pv;
+    qemu_put_sbe32s(f, v);
+}
+
+const VMStateInfo vmstate_info_int32 = {
+    .name = "int32",
+    .get  = get_int32,
+    .put  = put_int32,
+};
+
+/* 64 bit int */
+
+static int get_int64(QEMUFile *f, void *pv, size_t size)
+{
+    int64_t *v = pv;
+    qemu_get_sbe64s(f, v);
+    return 0;
+}
+
+static void put_int64(QEMUFile *f, const void *pv, size_t size)
+{
+    const int64_t *v = pv;
+    qemu_put_sbe64s(f, v);
+}
+
+const VMStateInfo vmstate_info_int64 = {
+    .name = "int64",
+    .get  = get_int64,
+    .put  = put_int64,
+};
+
+/* 8 bit unsigned int */
+
+static int get_uint8(QEMUFile *f, void *pv, size_t size)
+{
+    uint8_t *v = pv;
+    qemu_get_8s(f, v);
+    return 0;
+}
+
+static void put_uint8(QEMUFile *f, const void *pv, size_t size)
+{
+    const uint8_t *v = pv;
+    qemu_put_8s(f, v);
+}
+
+const VMStateInfo vmstate_info_uint8 = {
+    .name = "uint8",
+    .get  = get_uint8,
+    .put  = put_uint8,
+};
+
+/* 16 bit unsigned int */
+
+static int get_uint16(QEMUFile *f, void *pv, size_t size)
+{
+    uint16_t *v = pv;
+    qemu_get_be16s(f, v);
+    return 0;
+}
+
+static void put_uint16(QEMUFile *f, const void *pv, size_t size)
+{
+    const uint16_t *v = pv;
+    qemu_put_be16s(f, v);
+}
+
+const VMStateInfo vmstate_info_uint16 = {
+    .name = "uint16",
+    .get  = get_uint16,
+    .put  = put_uint16,
+};
+
+/* 32 bit unsigned int */
+
+static int get_uint32(QEMUFile *f, void *pv, size_t size)
+{
+    uint32_t *v = pv;
+    qemu_get_be32s(f, v);
+    return 0;
+}
+
+static void put_uint32(QEMUFile *f, const void *pv, size_t size)
+{
+    const uint32_t *v = pv;
+    qemu_put_be32s(f, v);
+}
+
+const VMStateInfo vmstate_info_uint32 = {
+    .name = "uint32",
+    .get  = get_uint32,
+    .put  = put_uint32,
+};
+
+/* 64 bit unsigned int */
+
+static int get_uint64(QEMUFile *f, void *pv, size_t size)
+{
+    uint64_t *v = pv;
+    qemu_get_be64s(f, v);
+    return 0;
+}
+
+static void put_uint64(QEMUFile *f, const void *pv, size_t size)
+{
+    const uint64_t *v = pv;
+    qemu_put_be64s(f, v);
+}
+
+const VMStateInfo vmstate_info_uint64 = {
+    .name = "uint64",
+    .get  = get_uint64,
+    .put  = put_uint64,
+};
+
 typedef struct SaveStateEntry {
     char idstr[256];
     int instance_id;
@@ -618,11 +786,13 @@ typedef struct SaveStateEntry {
     SaveLiveStateHandler *save_live_state;
     SaveStateHandler *save_state;
     LoadStateHandler *load_state;
+    const 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 +807,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 +817,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,17 +860,110 @@ void unregister_savevm(const char *idstr, void *opaque)
     }
 }

+int vmstate_register(int instance_id, const 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;
+    }
+}
+
+int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
+                       void *opaque, int version_id)
+{
+    VMStateField *field = vmsd->fields;
+
+    if (version_id > vmsd->version_id) {
+        return -EINVAL;
+    }
+    if (version_id < vmsd->minimum_version_id_old) {
+        return -EINVAL;
+    }
+    if  (version_id < vmsd->minimum_version_id) {
+        return vmsd->load_state_old(f, opaque, version_id);
+    }
+    while(field->name) {
+        if (field->version_id <= version_id) {
+            void *addr = opaque + field->offset;
+            int ret;
+
+            ret = field->info->get(f, addr, field->size);
+            if (ret < 0) {
+                return ret;
+            }
+        }
+        field++;
+    }
+    return 0;
+}
+
+void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
+                        const void *opaque)
+{
+    VMStateField *field = vmsd->fields;
+
+    while(field->name) {
+        const void *addr = opaque + field->offset;
+        field->info->put(f, addr, field->size);
+        field++;
+    }
+}
+
 static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
 {
-    return se->load_state(f, se->opaque, version_id);
+    if (!se->vmsd) {         /* Old style */
+        return se->load_state(f, se->opaque, version_id);
+    }
+    return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
 }

 static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
 {
-    se->save_state(f, se->opaque);
+    if (!se->vmsd) {         /* Old style */
+        se->save_state(f, se->opaque);
+        return;
+    }
+    vmstate_save_state(f,se->vmsd, se->opaque);
 }

-
 #define QEMU_VM_FILE_MAGIC           0x5145564d
 #define QEMU_VM_FILE_VERSION_COMPAT  0x00000002
 #define QEMU_VM_FILE_VERSION         0x00000003
@@ -788,7 +1051,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 */
-- 
1.6.2.5

[Qemu-devel] [PATCH 08/23] Add VMState support for pointers

[Juan Quintela]

This patch adds support for saving pointers to values

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |   19 +++++++++++++++++++
 savevm.c |   28 ++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index c58b416..a315f83 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -280,6 +280,7 @@ struct VMStateInfo {

 enum VMStateFlags {
     VMS_SINGLE  = 0x001,
+    VMS_POINTER = 0x002,
 };

 typedef struct {
@@ -310,6 +311,8 @@ extern const VMStateInfo vmstate_info_uint16;
 extern const VMStateInfo vmstate_info_uint32;
 extern const VMStateInfo vmstate_info_uint64;

+extern const VMStateInfo vmstate_info_timer;
+
 #define VMSTATE_SINGLE(_field, _state, _version, _info, _type) {     \
     .name       = (stringify(_field)),                               \
     .version_id = (_version),                                        \
@@ -320,6 +323,16 @@ extern const VMStateInfo vmstate_info_uint64;
             + type_check(_type,typeof_field(_state, _field))         \
 }

+#define VMSTATE_POINTER(_field, _state, _version, _info, _type) {    \
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .info       = &(_info),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_SINGLE|VMS_POINTER,                            \
+    .offset     = offsetof(_state, _field)                           \
+            + type_check(_type,typeof_field(_state, _field))         \
+}
+
 /* _f : field name
    _s : struct state name
    _v : version
@@ -361,6 +374,12 @@ extern const VMStateInfo vmstate_info_uint64;
 #define VMSTATE_UINT64(_f, _s)                                        \
     VMSTATE_UINT64_V(_f, _s, 0)

+#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
+    VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
+
+#define VMSTATE_TIMER(_f, _s)                                         \
+    VMSTATE_TIMER_V(_f, _s, 0)
+
 #define VMSTATE_END_OF_LIST()                                         \
     {}

diff --git a/savevm.c b/savevm.c
index 400d1c0..617b443 100644
--- a/savevm.c
+++ b/savevm.c
@@ -778,6 +778,27 @@ const VMStateInfo vmstate_info_uint64 = {
     .put  = put_uint64,
 };

+/* timers  */
+
+static int get_timer(QEMUFile *f, void *pv, size_t size)
+{
+    QEMUTimer *v = pv;
+    qemu_get_timer(f, v);
+    return 0;
+}
+
+static void put_timer(QEMUFile *f, const void *pv, size_t size)
+{
+    QEMUTimer *v = (void *)pv;
+    qemu_put_timer(f, v);
+}
+
+const VMStateInfo vmstate_info_timer = {
+    .name = "timer",
+    .get  = get_timer,
+    .put  = put_timer,
+};
+
 typedef struct SaveStateEntry {
     char idstr[256];
     int instance_id;
@@ -925,6 +946,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             void *addr = opaque + field->offset;
             int ret;

+            if (field->flags & VMS_POINTER) {
+                addr = *(void **)addr;
+            }
             ret = field->info->get(f, addr, field->size);
             if (ret < 0) {
                 return ret;
@@ -942,6 +966,10 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,

     while(field->name) {
         const void *addr = opaque + field->offset;
+
+        if (field->flags & VMS_POINTER) {
+            addr = *(void **)addr;
+        }
         field->info->put(f, addr, field->size);
         field++;
     }
-- 
1.6.2.5

[Qemu-devel] [PATCH 09/23] Add VMState support for arrays

[Juan Quintela]

This patch adds support for saving arrays inside the struct

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |   28 ++++++++++++++++++++++++++++
 savevm.c |   31 ++++++++++++++++++++++---------
 2 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index a315f83..ebb9c22 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -281,12 +281,14 @@ struct VMStateInfo {
 enum VMStateFlags {
     VMS_SINGLE  = 0x001,
     VMS_POINTER = 0x002,
+    VMS_ARRAY   = 0x004,
 };

 typedef struct {
     const char *name;
     size_t offset;
     size_t size;
+    int num;
     const VMStateInfo *info;
     enum VMStateFlags flags;
     int version_id;
@@ -313,6 +315,8 @@ extern const VMStateInfo vmstate_info_uint64;

 extern const VMStateInfo vmstate_info_timer;

+#define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
+
 #define VMSTATE_SINGLE(_field, _state, _version, _info, _type) {     \
     .name       = (stringify(_field)),                               \
     .version_id = (_version),                                        \
@@ -333,7 +337,19 @@ extern const VMStateInfo vmstate_info_timer;
             + type_check(_type,typeof_field(_state, _field))         \
 }

+#define VMSTATE_ARRAY(_field, _state, _num, _version, _info, _type) {\
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .num        = (_num),                                            \
+    .info       = &(_info),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_ARRAY,                                         \
+    .offset     = offsetof(_state, _field)                           \
+        + type_check_array(_type,typeof_field(_state, _field),_num)  \
+}
+
 /* _f : field name
+   _n : num of elements
    _s : struct state name
    _v : version
 */
@@ -380,6 +396,18 @@ extern const VMStateInfo vmstate_info_timer;
 #define VMSTATE_TIMER(_f, _s)                                         \
     VMSTATE_TIMER_V(_f, _s, 0)

+#define VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v)                        \
+    VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint32, uint32_t)
+
+#define VMSTATE_UINT32_ARRAY(_f, _s, _n)                              \
+    VMSTATE_UINT32_ARRAY_V(_f, _s, _n, 0)
+
+#define VMSTATE_INT32_ARRAY_V(_f, _s, _n, _v)                         \
+    VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_int32, int32_t)
+
+#define VMSTATE_INT32_ARRAY(_f, _s, _n)                               \
+    VMSTATE_INT32_ARRAY_V(_f, _s, _n, 0)
+
 #define VMSTATE_END_OF_LIST()                                         \
     {}

diff --git a/savevm.c b/savevm.c
index 617b443..040748e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -943,15 +943,21 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
     }
     while(field->name) {
         if (field->version_id <= version_id) {
-            void *addr = opaque + field->offset;
-            int ret;
+            void *base_addr = opaque + field->offset;
+            int ret, i, n_elems = 1;

+            if (field->flags & VMS_ARRAY) {
+                n_elems = field->num;
+            }
             if (field->flags & VMS_POINTER) {
-                addr = *(void **)addr;
+                base_addr = *(void **)base_addr;
             }
-            ret = field->info->get(f, addr, field->size);
-            if (ret < 0) {
-                return ret;
+            for (i = 0; i < n_elems; i++) {
+                void *addr = base_addr + field->size * i;
+                ret = field->info->get(f, addr, field->size);
+                if (ret < 0) {
+                    return ret;
+                }
             }
         }
         field++;
@@ -965,12 +971,19 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
     VMStateField *field = vmsd->fields;

     while(field->name) {
-        const void *addr = opaque + field->offset;
+        const void *base_addr = opaque + field->offset;
+        int i, n_elems = 1;

+        if (field->flags & VMS_ARRAY) {
+            n_elems = field->num;
+        }
         if (field->flags & VMS_POINTER) {
-            addr = *(void **)addr;
+            base_addr = *(void **)base_addr;
+        }
+        for (i = 0; i < n_elems; i++) {
+            const void *addr = base_addr + field->size * i;
+            field->info->put(f, addr, field->size);
         }
-        field->info->put(f, addr, field->size);
         field++;
     }
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 10/23] Port apic to new VMState design

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/apic.c |   67 ++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 1927811..141c5f2 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -864,39 +864,8 @@ static void apic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
     }
 }

-static void apic_save(QEMUFile *f, void *opaque)
-{
-    APICState *s = opaque;
-    int i;
-
-    qemu_put_be32s(f, &s->apicbase);
-    qemu_put_8s(f, &s->id);
-    qemu_put_8s(f, &s->arb_id);
-    qemu_put_8s(f, &s->tpr);
-    qemu_put_be32s(f, &s->spurious_vec);
-    qemu_put_8s(f, &s->log_dest);
-    qemu_put_8s(f, &s->dest_mode);
-    for (i = 0; i < 8; i++) {
-        qemu_put_be32s(f, &s->isr[i]);
-        qemu_put_be32s(f, &s->tmr[i]);
-        qemu_put_be32s(f, &s->irr[i]);
-    }
-    for (i = 0; i < APIC_LVT_NB; i++) {
-        qemu_put_be32s(f, &s->lvt[i]);
-    }
-    qemu_put_be32s(f, &s->esr);
-    qemu_put_be32s(f, &s->icr[0]);
-    qemu_put_be32s(f, &s->icr[1]);
-    qemu_put_be32s(f, &s->divide_conf);
-    qemu_put_be32(f, s->count_shift);
-    qemu_put_be32s(f, &s->initial_count);
-    qemu_put_be64(f, s->initial_count_load_time);
-    qemu_put_be64(f, s->next_time);
-
-    qemu_put_timer(f, s->timer);
-}
-
-static int apic_load(QEMUFile *f, void *opaque, int version_id)
+/* This function is only used for old state version 1 and 2 */
+static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
 {
     APICState *s = opaque;
     int i;
@@ -934,6 +903,36 @@ static int apic_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }

+static const VMStateDescription vmstate_apic = {
+    .name = "apic",
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .minimum_version_id_old = 1,
+    .load_state_old = apic_load_old,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT32(apicbase, APICState),
+        VMSTATE_UINT8(id, APICState),
+        VMSTATE_UINT8(arb_id, APICState),
+        VMSTATE_UINT8(tpr, APICState),
+        VMSTATE_UINT32(spurious_vec, APICState),
+        VMSTATE_UINT8(log_dest, APICState),
+        VMSTATE_UINT8(dest_mode, APICState),
+        VMSTATE_UINT32_ARRAY(isr, APICState, 8),
+        VMSTATE_UINT32_ARRAY(tmr, APICState, 8),
+        VMSTATE_UINT32_ARRAY(irr, APICState, 8),
+        VMSTATE_UINT32_ARRAY(lvt, APICState, APIC_LVT_NB),
+        VMSTATE_UINT32(esr, APICState),
+        VMSTATE_UINT32_ARRAY(icr, APICState, 2),
+        VMSTATE_UINT32(divide_conf, APICState),
+        VMSTATE_INT32(count_shift, APICState),
+        VMSTATE_UINT32(initial_count, APICState),
+        VMSTATE_INT64(initial_count_load_time, APICState),
+        VMSTATE_INT64(next_time, APICState),
+        VMSTATE_TIMER(timer, APICState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void apic_reset(void *opaque)
 {
     APICState *s = opaque;
@@ -996,7 +995,7 @@ int apic_init(CPUState *env)
     }
     s->timer = qemu_new_timer(vm_clock, apic_timer, s);

-    register_savevm("apic", s->idx, 2, apic_save, apic_load, s);
+    vmstate_register(s->idx, &vmstate_apic, s);
     qemu_register_reset(apic_reset, s);

     local_apics[s->idx] = s;
-- 
1.6.2.5

[Qemu-devel] [PATCH 11/23] Add VMState support for structs

[Juan Quintela]

This patch adds support for saving one VMStateDescription from other
VMStateDescription.

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

diff --git a/hw/hw.h b/hw/hw.h
index ebb9c22..4f63f73 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -282,6 +282,7 @@ enum VMStateFlags {
     VMS_SINGLE  = 0x001,
     VMS_POINTER = 0x002,
     VMS_ARRAY   = 0x004,
+    VMS_STRUCT  = 0x008,
 };

 typedef struct {
@@ -291,6 +292,7 @@ typedef struct {
     int num;
     const VMStateInfo *info;
     enum VMStateFlags flags;
+    const VMStateDescription *vmsd;
     int version_id;
 } VMStateField;

@@ -348,6 +350,16 @@ extern const VMStateInfo vmstate_info_timer;
         + type_check_array(_type,typeof_field(_state, _field),_num)  \
 }

+#define VMSTATE_STRUCT(_field, _state, _version, _vmsd, _type) {     \
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .vmsd       = &(_vmsd),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = offsetof(_state, _field)                           \
+            + type_check(_type,typeof_field(_state, _field))         \
+}
+
 /* _f : field name
    _n : num of elements
    _s : struct state name
diff --git a/savevm.c b/savevm.c
index 040748e..8320d37 100644
--- a/savevm.c
+++ b/savevm.c
@@ -954,7 +954,13 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             }
             for (i = 0; i < n_elems; i++) {
                 void *addr = base_addr + field->size * i;
-                ret = field->info->get(f, addr, field->size);
+
+                if (field->flags & VMS_STRUCT) {
+                    ret = vmstate_load_state(f, field->vmsd, addr, version_id);
+                } else {
+                    ret = field->info->get(f, addr, field->size);
+
+                }
                 if (ret < 0) {
                     return ret;
                 }
@@ -982,7 +988,12 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
         }
         for (i = 0; i < n_elems; i++) {
             const void *addr = base_addr + field->size * i;
-            field->info->put(f, addr, field->size);
+
+            if (field->flags & VMS_STRUCT) {
+                vmstate_save_state(f, field->vmsd, addr);
+            } else {
+                field->info->put(f, addr, field->size);
+            }
         }
         field++;
     }
-- 
1.6.2.5

[Qemu-devel] [PATCH 12/23] Add VMState support for arrays of structs

[Juan Quintela]

This patch add supports for arrays of structs

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 4f63f73..4b3211e 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -360,6 +360,17 @@ extern const VMStateInfo vmstate_info_timer;
             + type_check(_type,typeof_field(_state, _field))         \
 }

+#define VMSTATE_STRUCT_ARRAY(_field, _state, _num, _version, _vmsd, _type) { \
+    .name       = (stringify(_field)),                               \
+    .num        = (_num),                                            \
+    .version_id = (_version),                                        \
+    .vmsd       = &(_vmsd),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_STRUCT|VMS_ARRAY,                              \
+    .offset     = offsetof(_state, _field)                           \
+        + type_check_array(_type,typeof_field(_state, _field),_num)  \
+}
+
 /* _f : field name
    _n : num of elements
    _s : struct state name
-- 
1.6.2.5

[Qemu-devel] [PATCH 13/23] Port i8254 to new VMState design

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/i8254.c |   66 +++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index 44e4531..6a8cd3e 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -389,35 +389,31 @@ static void pit_irq_timer(void *opaque)
     pit_irq_timer_update(s, s->next_transition_time);
 }

-static void pit_save(QEMUFile *f, void *opaque)
-{
-    PITState *pit = opaque;
-    PITChannelState *s;
-    int i;
-
-    for(i = 0; i < 3; i++) {
-        s = &pit->channels[i];
-        qemu_put_be32(f, s->count);
-        qemu_put_be16s(f, &s->latched_count);
-        qemu_put_8s(f, &s->count_latched);
-        qemu_put_8s(f, &s->status_latched);
-        qemu_put_8s(f, &s->status);
-        qemu_put_8s(f, &s->read_state);
-        qemu_put_8s(f, &s->write_state);
-        qemu_put_8s(f, &s->write_latch);
-        qemu_put_8s(f, &s->rw_mode);
-        qemu_put_8s(f, &s->mode);
-        qemu_put_8s(f, &s->bcd);
-        qemu_put_8s(f, &s->gate);
-        qemu_put_be64(f, s->count_load_time);
-        if (s->irq_timer) {
-            qemu_put_be64(f, s->next_transition_time);
-            qemu_put_timer(f, s->irq_timer);
-        }
+static const VMStateDescription vmstate_pit_channel = {
+    .name = "pit channel",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .fields      = (VMStateField []) {
+        VMSTATE_INT32(count, PITChannelState),
+        VMSTATE_UINT16(latched_count, PITChannelState),
+        VMSTATE_UINT8(count_latched, PITChannelState),
+        VMSTATE_UINT8(status_latched, PITChannelState),
+        VMSTATE_UINT8(status, PITChannelState),
+        VMSTATE_UINT8(read_state, PITChannelState),
+        VMSTATE_UINT8(write_state, PITChannelState),
+        VMSTATE_UINT8(write_latch, PITChannelState),
+        VMSTATE_UINT8(rw_mode, PITChannelState),
+        VMSTATE_UINT8(mode, PITChannelState),
+        VMSTATE_UINT8(bcd, PITChannelState),
+        VMSTATE_UINT8(gate, PITChannelState),
+        VMSTATE_INT64(count_load_time, PITChannelState),
+        VMSTATE_INT64(next_transition_time, PITChannelState),
+        VMSTATE_END_OF_LIST()
     }
-}
+};

-static int pit_load(QEMUFile *f, void *opaque, int version_id)
+static int pit_load_old(QEMUFile *f, void *opaque, int version_id)
 {
     PITState *pit = opaque;
     PITChannelState *s;
@@ -449,6 +445,19 @@ static int pit_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }

+static const VMStateDescription vmstate_pit = {
+    .name = "i8254",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 1,
+    .load_state_old = pit_load_old,
+    .fields      = (VMStateField []) {
+        VMSTATE_STRUCT_ARRAY(channels, PITState, 3, 2, vmstate_pit_channel, PITChannelState),
+        VMSTATE_TIMER(channels[0].irq_timer, PITState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void pit_reset(void *opaque)
 {
     PITState *pit = opaque;
@@ -495,8 +504,7 @@ PITState *pit_init(int base, qemu_irq irq)
     s->irq_timer = qemu_new_timer(vm_clock, pit_irq_timer, s);
     s->irq = irq;

-    register_savevm("i8254", base, 1, pit_save, pit_load, pit);
-
+    vmstate_register(base, &vmstate_pit, pit);
     qemu_register_reset(pit_reset, pit);
     register_ioport_write(base, 4, 1, pit_ioport_write, pit);
     register_ioport_read(base, 3, 1, pit_ioport_read, pit);
-- 
1.6.2.5

[Qemu-devel] [PATCH 14/23] Add VMState support for int32_t check value

[Juan Quintela]

We read the saved value and check that it is the same that the one
is stored in the structure.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |    5 +++++
 savevm.c |   20 ++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 4b3211e..e3e9d5b 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -310,6 +310,8 @@ extern const VMStateInfo vmstate_info_int16;
 extern const VMStateInfo vmstate_info_int32;
 extern const VMStateInfo vmstate_info_int64;

+extern const VMStateInfo vmstate_info_int32_equal;
+
 extern const VMStateInfo vmstate_info_uint8;
 extern const VMStateInfo vmstate_info_uint16;
 extern const VMStateInfo vmstate_info_uint32;
@@ -413,6 +415,9 @@ extern const VMStateInfo vmstate_info_timer;
 #define VMSTATE_UINT64(_f, _s)                                        \
     VMSTATE_UINT64_V(_f, _s, 0)

+#define VMSTATE_INT32_EQUAL(_f, _s)                                   \
+    VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_equal, int32_t)
+
 #define VMSTATE_TIMER_V(_f, _s, _v)                                   \
     VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)

diff --git a/savevm.c b/savevm.c
index 8320d37..e443f00 100644
--- a/savevm.c
+++ b/savevm.c
@@ -673,6 +673,26 @@ const VMStateInfo vmstate_info_int32 = {
     .put  = put_int32,
 };

+/* 32 bit int. See that the received value is the same than the one
+   in the field */
+
+static int get_int32_equal(QEMUFile *f, void *pv, size_t size)
+{
+    int32_t *v = pv;
+    int32_t v2;
+    qemu_get_sbe32s(f, &v2);
+
+    if (*v == v2)
+        return 0;
+    return -EINVAL;
+}
+
+const VMStateInfo vmstate_info_int32_equal = {
+    .name = "int32 equal",
+    .get  = get_int32_equal,
+    .put  = put_int32,
+};
+
 /* 64 bit int */

 static int get_int64(QEMUFile *f, void *pv, size_t size)
-- 
1.6.2.5

[Qemu-devel] [PATCH 15/23] Add VMState support for variable sized arrays

[Juan Quintela]

This patch add supports for variable sized arrays whose size is
another field of the state.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |   22 ++++++++++++++++++++++
 savevm.c |    4 ++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index e3e9d5b..911ab6f 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -283,6 +283,7 @@ enum VMStateFlags {
     VMS_POINTER = 0x002,
     VMS_ARRAY   = 0x004,
     VMS_STRUCT  = 0x008,
+    VMS_VARRAY  = 0x010,  /* Array with size in another field */
 };

 typedef struct {
@@ -290,6 +291,7 @@ typedef struct {
     size_t offset;
     size_t size;
     int num;
+    size_t num_offset;
     const VMStateInfo *info;
     enum VMStateFlags flags;
     const VMStateDescription *vmsd;
@@ -320,6 +322,7 @@ extern const VMStateInfo vmstate_info_uint64;
 extern const VMStateInfo vmstate_info_timer;

 #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
+#define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0)

 #define VMSTATE_SINGLE(_field, _state, _version, _info, _type) {     \
     .name       = (stringify(_field)),                               \
@@ -352,6 +355,18 @@ extern const VMStateInfo vmstate_info_timer;
         + type_check_array(_type,typeof_field(_state, _field),_num)  \
 }

+#define VMSTATE_VARRAY(_field, _state, _field_num, _version, _info, _type) {\
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .num_offset = offsetof(_state, _field_num)                       \
+        + type_check(int32_t,typeof_field(_state, _field_num)),      \
+    .info       = &(_info),                                          \
+    .size       = sizeof(_type),                                     \
+    .flags      = VMS_VARRAY|VMS_POINTER,                            \
+    .offset     = offsetof(_state, _field)                           \
+        + type_check_pointer(_type,typeof_field(_state, _field))     \
+}
+
 #define VMSTATE_STRUCT(_field, _state, _version, _vmsd, _type) {     \
     .name       = (stringify(_field)),                               \
     .version_id = (_version),                                        \
@@ -374,6 +389,7 @@ extern const VMStateInfo vmstate_info_timer;
 }

 /* _f : field name
+   _f_n : num of elements field_name
    _n : num of elements
    _s : struct state name
    _v : version
@@ -436,6 +452,12 @@ extern const VMStateInfo vmstate_info_timer;
 #define VMSTATE_INT32_ARRAY(_f, _s, _n)                               \
     VMSTATE_INT32_ARRAY_V(_f, _s, _n, 0)

+#define VMSTATE_INT32_VARRAY_V(_f, _s, _f_n, _v)                      \
+    VMSTATE_VARRAY(_f, _s, _f_n, _v, vmstate_info_int32, int32_t)
+
+#define VMSTATE_INT32_VARRAY(_f, _s, _f_n)                            \
+    VMSTATE_INT32_VARRAY_V(_f, _s, _f_n, 0)
+
 #define VMSTATE_END_OF_LIST()                                         \
     {}

diff --git a/savevm.c b/savevm.c
index e443f00..721bbf4 100644
--- a/savevm.c
+++ b/savevm.c
@@ -968,6 +968,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,

             if (field->flags & VMS_ARRAY) {
                 n_elems = field->num;
+            } else if (field->flags & VMS_VARRAY) {
+                n_elems = *(size_t *)(opaque+field->num_offset);
             }
             if (field->flags & VMS_POINTER) {
                 base_addr = *(void **)base_addr;
@@ -1002,6 +1004,8 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,

         if (field->flags & VMS_ARRAY) {
             n_elems = field->num;
+        } else if (field->flags & VMS_VARRAY) {
+            n_elems = *(size_t *)(opaque+field->num_offset);
         }
         if (field->flags & VMS_POINTER) {
             base_addr = *(void **)base_addr;
-- 
1.6.2.5

[Qemu-devel] [PATCH 16/23] Port PCI Bus to VMState design

[Juan Quintela]

This uses VARRAY and INT32_EQUAL values

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/pci.c |   41 +++++++++++------------------------------
 1 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 27eac04..681b9d2 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -73,36 +73,17 @@ static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
 static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
 static PCIBus *first_bus;

-static void pcibus_save(QEMUFile *f, void *opaque)
-{
-    PCIBus *bus = (PCIBus *)opaque;
-    int i;
-
-    qemu_put_be32(f, bus->nirq);
-    for (i = 0; i < bus->nirq; i++)
-        qemu_put_be32(f, bus->irq_count[i]);
-}
-
-static int  pcibus_load(QEMUFile *f, void *opaque, int version_id)
-{
-    PCIBus *bus = (PCIBus *)opaque;
-    int i, nirq;
-
-    if (version_id != 1)
-        return -EINVAL;
-
-    nirq = qemu_get_be32(f);
-    if (bus->nirq != nirq) {
-        fprintf(stderr, "pcibus_load: nirq mismatch: src=%d dst=%d\n",
-                nirq, bus->nirq);
-        return -EINVAL;
+static const VMStateDescription vmstate_pcibus = {
+    .name = "PCIBUS",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_INT32_EQUAL(nirq, PCIBus),
+        VMSTATE_INT32_VARRAY(irq_count, PCIBus, nirq),
+        VMSTATE_END_OF_LIST()
     }
-
-    for (i = 0; i < nirq; i++)
-        bus->irq_count[i] = qemu_get_be32(f);
-
-    return 0;
-}
+};

 static void pci_bus_reset(void *opaque)
 {
@@ -135,7 +116,7 @@ PCIBus *pci_register_bus(DeviceState *parent, const char *name,
     bus->irq_count = qemu_mallocz(nirq * sizeof(bus->irq_count[0]));
     bus->next = first_bus;
     first_bus = bus;
-    register_savevm("PCIBUS", nbus++, 1, pcibus_save, pcibus_load, bus);
+    vmstate_register(nbus++, &vmstate_pcibus, bus);
     qemu_register_reset(pci_bus_reset, bus);
     return bus;
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 17/23] Add VMState support for static sized buffers (uint_8)

[Juan Quintela]

This patch adds support for static sized buffer and typecheks that the buffer is right.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |   18 ++++++++++++++++++
 savevm.c |   21 +++++++++++++++++++++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 911ab6f..4713967 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -284,6 +284,7 @@ enum VMStateFlags {
     VMS_ARRAY   = 0x004,
     VMS_STRUCT  = 0x008,
     VMS_VARRAY  = 0x010,  /* Array with size in another field */
+    VMS_BUFFER  = 0x020,  /* static sized buffer */
 };

 typedef struct {
@@ -320,6 +321,7 @@ extern const VMStateInfo vmstate_info_uint32;
 extern const VMStateInfo vmstate_info_uint64;

 extern const VMStateInfo vmstate_info_timer;
+extern const VMStateInfo vmstate_info_buffer;

 #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
 #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0)
@@ -388,6 +390,16 @@ extern const VMStateInfo vmstate_info_timer;
         + type_check_array(_type,typeof_field(_state, _field),_num)  \
 }

+#define VMSTATE_STATIC_BUFFER(_field, _state, _version) {            \
+    .name       = (stringify(_field)),                               \
+    .version_id = (_version),                                        \
+    .size       = sizeof(typeof_field(_state,_field)),               \
+    .info       = &vmstate_info_buffer,                              \
+    .flags      = VMS_BUFFER,                                        \
+    .offset     = offsetof(_state, _field)                           \
+        + type_check_array(uint8_t,typeof_field(_state, _field),sizeof(typeof_field(_state,_field))) \
+}
+
 /* _f : field name
    _f_n : num of elements field_name
    _n : num of elements
@@ -458,6 +470,12 @@ extern const VMStateInfo vmstate_info_timer;
 #define VMSTATE_INT32_VARRAY(_f, _s, _f_n)                            \
     VMSTATE_INT32_VARRAY_V(_f, _s, _f_n, 0)

+#define VMSTATE_BUFFER_V(_f, _s, _v)                                  \
+    VMSTATE_STATIC_BUFFER(_f, _s, _v)
+
+#define VMSTATE_BUFFER(_f, _s)                                        \
+    VMSTATE_STATIC_BUFFER(_f, _s, 0)
+
 #define VMSTATE_END_OF_LIST()                                         \
     {}

diff --git a/savevm.c b/savevm.c
index 721bbf4..1b448b9 100644
--- a/savevm.c
+++ b/savevm.c
@@ -819,6 +819,27 @@ const VMStateInfo vmstate_info_timer = {
     .put  = put_timer,
 };

+/* uint8_t buffers */
+
+static int get_buffer(QEMUFile *f, void *pv, size_t size)
+{
+    uint8_t *v = pv;
+    qemu_get_buffer(f, v, size);
+    return 0;
+}
+
+static void put_buffer(QEMUFile *f, const void *pv, size_t size)
+{
+    uint8_t *v = (void *)pv;
+    qemu_put_buffer(f, v, size);
+}
+
+const VMStateInfo vmstate_info_buffer = {
+    .name = "buffer",
+    .get  = get_buffer,
+    .put  = put_buffer,
+};
+
 typedef struct SaveStateEntry {
     char idstr[256];
     int instance_id;
-- 
1.6.2.5

[Qemu-devel] [PATCH 18/23] Port PS2 devices to VMState design

[Juan Quintela]

This uses STRUCT and BUFFER

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ps2.c |  119 +++++++++++++++++++++++++++-----------------------------------
 1 files changed, 52 insertions(+), 67 deletions(-)

diff --git a/hw/ps2.c b/hw/ps2.c
index 0915b9f..fff4d13 100644
--- a/hw/ps2.c
+++ b/hw/ps2.c
@@ -526,59 +526,29 @@ static void ps2_mouse_reset(void *opaque)
     s->mouse_buttons = 0;
 }

-static void ps2_common_save (QEMUFile *f, PS2State *s)
-{
-    qemu_put_be32 (f, s->write_cmd);
-    qemu_put_be32 (f, s->queue.rptr);
-    qemu_put_be32 (f, s->queue.wptr);
-    qemu_put_be32 (f, s->queue.count);
-    qemu_put_buffer (f, s->queue.data, sizeof (s->queue.data));
-}
-
-static void ps2_common_load (QEMUFile *f, PS2State *s)
-{
-    s->write_cmd=qemu_get_be32 (f);
-    s->queue.rptr=qemu_get_be32 (f);
-    s->queue.wptr=qemu_get_be32 (f);
-    s->queue.count=qemu_get_be32 (f);
-    qemu_get_buffer (f, s->queue.data, sizeof (s->queue.data));
-}
-
-static void ps2_kbd_save(QEMUFile* f, void* opaque)
-{
-    PS2KbdState *s = (PS2KbdState*)opaque;
-
-    ps2_common_save (f, &s->common);
-    qemu_put_be32(f, s->scan_enabled);
-    qemu_put_be32(f, s->translate);
-    qemu_put_be32(f, s->scancode_set);
-}
-
-static void ps2_mouse_save(QEMUFile* f, void* opaque)
-{
-    PS2MouseState *s = (PS2MouseState*)opaque;
-
-    ps2_common_save (f, &s->common);
-    qemu_put_8s(f, &s->mouse_status);
-    qemu_put_8s(f, &s->mouse_resolution);
-    qemu_put_8s(f, &s->mouse_sample_rate);
-    qemu_put_8s(f, &s->mouse_wrap);
-    qemu_put_8s(f, &s->mouse_type);
-    qemu_put_8s(f, &s->mouse_detect_state);
-    qemu_put_be32(f, s->mouse_dx);
-    qemu_put_be32(f, s->mouse_dy);
-    qemu_put_be32(f, s->mouse_dz);
-    qemu_put_8s(f, &s->mouse_buttons);
-}
+static const VMStateDescription vmstate_ps2_common = {
+    .name = "PS2 Common State",
+    .version_id = 3,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .fields      = (VMStateField []) {
+        VMSTATE_INT32(write_cmd, PS2State),
+        VMSTATE_INT32(queue.rptr, PS2State),
+        VMSTATE_INT32(queue.wptr, PS2State),
+        VMSTATE_INT32(queue.count, PS2State),
+        VMSTATE_BUFFER(queue.data, PS2State),
+        VMSTATE_END_OF_LIST()
+    }
+};

-static int ps2_kbd_load(QEMUFile* f, void* opaque, int version_id)
+static int ps2_kbd_load_old(QEMUFile* f, void* opaque, int version_id)
 {
     PS2KbdState *s = (PS2KbdState*)opaque;

     if (version_id != 2 && version_id != 3)
         return -EINVAL;

-    ps2_common_load (f, &s->common);
+    vmstate_load_state(f, &vmstate_ps2_common, &s->common, version_id);
     s->scan_enabled=qemu_get_be32(f);
     s->translate=qemu_get_be32(f);
     if (version_id == 3)
@@ -588,26 +558,41 @@ static int ps2_kbd_load(QEMUFile* f, void* opaque, int version_id)
     return 0;
 }

-static int ps2_mouse_load(QEMUFile* f, void* opaque, int version_id)
-{
-    PS2MouseState *s = (PS2MouseState*)opaque;
-
-    if (version_id != 2)
-        return -EINVAL;
+static const VMStateDescription vmstate_ps2_keyboard = {
+    .name = "ps2kbd",
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .minimum_version_id_old = 2,
+    .load_state_old = ps2_kbd_load_old,
+    .fields      = (VMStateField []) {
+        VMSTATE_STRUCT(common, PS2KbdState, 0, vmstate_ps2_common, PS2State),
+        VMSTATE_INT32(scan_enabled, PS2KbdState),
+        VMSTATE_INT32(translate, PS2KbdState),
+        VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
+        VMSTATE_END_OF_LIST()
+    }
+};

-    ps2_common_load (f, &s->common);
-    qemu_get_8s(f, &s->mouse_status);
-    qemu_get_8s(f, &s->mouse_resolution);
-    qemu_get_8s(f, &s->mouse_sample_rate);
-    qemu_get_8s(f, &s->mouse_wrap);
-    qemu_get_8s(f, &s->mouse_type);
-    qemu_get_8s(f, &s->mouse_detect_state);
-    s->mouse_dx=qemu_get_be32(f);
-    s->mouse_dy=qemu_get_be32(f);
-    s->mouse_dz=qemu_get_be32(f);
-    qemu_get_8s(f, &s->mouse_buttons);
-    return 0;
-}
+static const VMStateDescription vmstate_ps2_mouse = {
+    .name = "ps2mouse",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .fields      = (VMStateField []) {
+        VMSTATE_STRUCT(common, PS2MouseState, 0, vmstate_ps2_common, PS2State),
+        VMSTATE_UINT8(mouse_status, PS2MouseState),
+        VMSTATE_UINT8(mouse_resolution, PS2MouseState),
+        VMSTATE_UINT8(mouse_sample_rate, PS2MouseState),
+        VMSTATE_UINT8(mouse_wrap, PS2MouseState),
+        VMSTATE_UINT8(mouse_type, PS2MouseState),
+        VMSTATE_UINT8(mouse_detect_state, PS2MouseState),
+        VMSTATE_INT32(mouse_dx, PS2MouseState),
+        VMSTATE_INT32(mouse_dy, PS2MouseState),
+        VMSTATE_INT32(mouse_dz, PS2MouseState),
+        VMSTATE_UINT8(mouse_buttons, PS2MouseState),
+        VMSTATE_END_OF_LIST()
+    }
+};

 void *ps2_kbd_init(void (*update_irq)(void *, int), void *update_arg)
 {
@@ -617,7 +602,7 @@ void *ps2_kbd_init(void (*update_irq)(void *, int), void *update_arg)
     s->common.update_arg = update_arg;
     s->scancode_set = 2;
     ps2_kbd_reset(s);
-    register_savevm("ps2kbd", 0, 3, ps2_kbd_save, ps2_kbd_load, s);
+    vmstate_register(0, &vmstate_ps2_keyboard, s);
     qemu_add_kbd_event_handler(ps2_put_keycode, s);
     qemu_register_reset(ps2_kbd_reset, s);
     return s;
@@ -630,7 +615,7 @@ void *ps2_mouse_init(void (*update_irq)(void *, int), void *update_arg)
     s->common.update_irq = update_irq;
     s->common.update_arg = update_arg;
     ps2_mouse_reset(s);
-    register_savevm("ps2mouse", 0, 2, ps2_mouse_save, ps2_mouse_load, s);
+    vmstate_register(0, &vmstate_ps2_mouse, s);
     qemu_add_mouse_event_handler(ps2_mouse_event, s, 0, "QEMU PS/2 Mouse");
     qemu_register_reset(ps2_mouse_reset, s);
     return s;
-- 
1.6.2.5

[Qemu-devel] [PATCH 19/23] Add VMState support for int32_t check value

[Juan Quintela]

We read the saved value and check that it is less or equal than the one
stored in the structure.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |    4 ++++
 savevm.c |   20 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 4713967..c574b47 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -314,6 +314,7 @@ extern const VMStateInfo vmstate_info_int32;
 extern const VMStateInfo vmstate_info_int64;

 extern const VMStateInfo vmstate_info_int32_equal;
+extern const VMStateInfo vmstate_info_int32_le;

 extern const VMStateInfo vmstate_info_uint8;
 extern const VMStateInfo vmstate_info_uint16;
@@ -446,6 +447,9 @@ extern const VMStateInfo vmstate_info_buffer;
 #define VMSTATE_INT32_EQUAL(_f, _s)                                   \
     VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_equal, int32_t)

+#define VMSTATE_INT32_LE(_f, _s)                                   \
+    VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_le, int32_t)
+
 #define VMSTATE_TIMER_V(_f, _s, _v)                                   \
     VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)

diff --git a/savevm.c b/savevm.c
index 1b448b9..4879cf6 100644
--- a/savevm.c
+++ b/savevm.c
@@ -693,6 +693,26 @@ const VMStateInfo vmstate_info_int32_equal = {
     .put  = put_int32,
 };

+/* 32 bit int. See that the received value is the less or the same
+   than the one in the field */
+
+static int get_int32_le(QEMUFile *f, void *pv, size_t size)
+{
+    int32_t *old = pv;
+    int32_t new;
+    qemu_get_sbe32s(f, &new);
+
+    if (*old <= new)
+        return 0;
+    return -EINVAL;
+}
+
+const VMStateInfo vmstate_info_int32_le = {
+    .name = "int32 equal",
+    .get  = get_int32_le,
+    .put  = put_int32,
+};
+
 /* 64 bit int */

 static int get_int64(QEMUFile *f, void *pv, size_t size)
-- 
1.6.2.5

[Qemu-devel] [PATCH 20/23] Add version_id to PCIDevice.

[Juan Quintela]

It is needed for VMState

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/pci.c |    3 ++-
 hw/pci.h |    2 ++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 681b9d2..7644dee 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -144,7 +144,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
 {
     int i;

-    qemu_put_be32(f, 2); /* PCI device version */
+    qemu_put_be32(f, s->version_id); /* PCI device version */
     qemu_put_buffer(f, s->config, 256);
     for (i = 0; i < 4; i++)
         qemu_put_be32(f, s->irq_state[i]);
@@ -319,6 +319,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     pci_dev->config_write = config_write;
     bus->devices[devfn] = pci_dev;
     pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, 4);
+    pci_dev->version_id = 2; /* Current pci device vmstate version */
     return pci_dev;
 }

diff --git a/hw/pci.h b/hw/pci.h
index a2ec16a..32e98d4 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -209,6 +209,8 @@ struct PCIDevice {
     unsigned *msix_entry_used;
     /* Region including the MSI-X table */
     uint32_t msix_bar_size;
+    /* Version id needed for VMState */
+    int32_t version_id;
 };

 PCIDevice *pci_register_device(PCIBus *bus, const char *name,
-- 
1.6.2.5

[Qemu-devel] [PATCH 21/23] Port PCIDevice state to VMState

[Juan Quintela]

This uses a variant of buffer, with extra checks. Also uses the new
support for cheking that a read value is less or equal than a field.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/hw.h  |   12 +++++++++++
 hw/pci.c |   65 +++++++++++++++++++++++++++++++++++++++++---------------------
 2 files changed, 55 insertions(+), 22 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index c574b47..c77b1be 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -401,6 +401,18 @@ extern const VMStateInfo vmstate_info_buffer;
         + type_check_array(uint8_t,typeof_field(_state, _field),sizeof(typeof_field(_state,_field))) \
 }

+extern const VMStateDescription vmstate_pci_device;
+
+#define VMSTATE_PCI_DEVICE(_field, _state) {                         \
+    .name       = (stringify(_field)),                               \
+    .version_id = 2,                                                 \
+    .size       = sizeof(PCIDevice),                                 \
+    .vmsd       = &vmstate_pci_device,                               \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = offsetof(_state, _field)                           \
+            + type_check(PCIDevice,typeof_field(_state, _field))     \
+}
+
 /* _f : field name
    _f_n : num of elements field_name
    _n : num of elements
diff --git a/hw/pci.c b/hw/pci.c
index 7644dee..2266e23 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -140,39 +140,60 @@ int pci_bus_num(PCIBus *s)
     return s->bus_num;
 }

-void pci_device_save(PCIDevice *s, QEMUFile *f)
-{
-    int i;
-
-    qemu_put_be32(f, s->version_id); /* PCI device version */
-    qemu_put_buffer(f, s->config, 256);
-    for (i = 0; i < 4; i++)
-        qemu_put_be32(f, s->irq_state[i]);
-}
-
-int pci_device_load(PCIDevice *s, QEMUFile *f)
+static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
 {
-    uint8_t config[PCI_CONFIG_SPACE_SIZE];
-    uint32_t version_id;
+    PCIDevice *s = container_of(pv, PCIDevice, config);
+    uint8_t config[size];
     int i;

-    version_id = qemu_get_be32(f);
-    if (version_id > 2)
-        return -EINVAL;
-    qemu_get_buffer(f, config, sizeof config);
-    for (i = 0; i < sizeof config; ++i)
+    qemu_get_buffer(f, config, size);
+    for (i = 0; i < size; ++i)
         if ((config[i] ^ s->config[i]) & s->cmask[i] & ~s->wmask[i])
             return -EINVAL;
-    memcpy(s->config, config, sizeof config);
+    memcpy(s->config, config, size);

     pci_update_mappings(s);

-    if (version_id >= 2)
-        for (i = 0; i < 4; i ++)
-            s->irq_state[i] = qemu_get_be32(f);
     return 0;
 }

+/* just put buffer */
+static void put_pci_config_device(QEMUFile *f, const void *pv, size_t size)
+{
+    const uint8_t *v = pv;
+    qemu_put_buffer(f, v, size);
+}
+
+static VMStateInfo vmstate_info_pci_config = {
+    .name = "pci config",
+    .get  = get_pci_config_device,
+    .put  = put_pci_config_device,
+};
+
+const VMStateDescription vmstate_pci_device = {
+    .name = "PCIDevice",
+    .version_id = 2,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_INT32_LE(version_id, PCIDevice),
+        VMSTATE_SINGLE(config, PCIDevice, 0, vmstate_info_pci_config,
+                       typeof_field(PCIDevice,config)),
+        VMSTATE_INT32_ARRAY_V(irq_state, PCIDevice, 4, 2),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+void pci_device_save(PCIDevice *s, QEMUFile *f)
+{
+    vmstate_save_state(f, &vmstate_pci_device, s);
+}
+
+int pci_device_load(PCIDevice *s, QEMUFile *f)
+{
+    return vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
+}
+
 static int pci_set_default_subsystem_id(PCIDevice *pci_dev)
 {
     uint16_t *id;
-- 
1.6.2.5

[Qemu-devel] [PATCH 22/23] Add VMState support to run a function after load

[Juan Quintela]

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

diff --git a/hw/hw.h b/hw/hw.h
index c77b1be..f097c2a 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -305,6 +305,7 @@ struct VMStateDescription {
     int minimum_version_id;
     int minimum_version_id_old;
     LoadStateHandler *load_state_old;
+    int (*run_after_load)(void *opaque);
     VMStateField *fields;
 };

diff --git a/savevm.c b/savevm.c
index 4879cf6..b5a0438 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1031,6 +1031,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
         }
         field++;
     }
+    if (vmsd->run_after_load)
+        return vmsd->run_after_load(opaque);
     return 0;
 }

-- 
1.6.2.5

[Qemu-devel] [PATCH 23/23] Port ACPI to VMState

[Juan Quintela]

This uses a run_after_load() function, and VMSTATE_PCI_DEVICE()
It could be made smaller changing the type of pm_io_space_update()
to return an int.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/acpi.c |   55 +++++++++++++++++++++----------------------------------
 1 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 37a5d98..6eddd0e 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -440,46 +440,33 @@ static void pm_write_config(PCIDevice *d,
         pm_io_space_update((PIIX4PMState *)d);
 }

-static void pm_save(QEMUFile* f,void *opaque)
+static int vmstate_acpi_after_load(void *opaque)
 {
     PIIX4PMState *s = opaque;

-    pci_device_save(&s->dev, f);
-
-    qemu_put_be16s(f, &s->pmsts);
-    qemu_put_be16s(f, &s->pmen);
-    qemu_put_be16s(f, &s->pmcntrl);
-    qemu_put_8s(f, &s->apmc);
-    qemu_put_8s(f, &s->apms);
-    qemu_put_timer(f, s->tmr_timer);
-    qemu_put_be64(f, s->tmr_overflow_time);
-}
-
-static int pm_load(QEMUFile* f,void* opaque,int version_id)
-{
-    PIIX4PMState *s = opaque;
-    int ret;
-
-    if (version_id > 1)
-        return -EINVAL;
-
-    ret = pci_device_load(&s->dev, f);
-    if (ret < 0)
-        return ret;
-
-    qemu_get_be16s(f, &s->pmsts);
-    qemu_get_be16s(f, &s->pmen);
-    qemu_get_be16s(f, &s->pmcntrl);
-    qemu_get_8s(f, &s->apmc);
-    qemu_get_8s(f, &s->apms);
-    qemu_get_timer(f, s->tmr_timer);
-    s->tmr_overflow_time=qemu_get_be64(f);
-
     pm_io_space_update(s);
-
     return 0;
 }

+static const VMStateDescription vmstate_acpi = {
+    .name = "piix4_pm",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .run_after_load = vmstate_acpi_after_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_PCI_DEVICE(dev, PIIX4PMState),
+        VMSTATE_UINT16(pmsts, PIIX4PMState),
+        VMSTATE_UINT16(pmen, PIIX4PMState),
+        VMSTATE_UINT16(pmcntrl, PIIX4PMState),
+        VMSTATE_UINT8(apmc, PIIX4PMState),
+        VMSTATE_UINT8(apms, PIIX4PMState),
+        VMSTATE_TIMER(tmr_timer, PIIX4PMState),
+        VMSTATE_INT64(tmr_overflow_time, PIIX4PMState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void piix4_reset(void *opaque)
 {
     PIIX4PMState *s = opaque;
@@ -561,7 +548,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,

     qemu_system_powerdown = *qemu_allocate_irqs(piix4_powerdown, s, 1);

-    register_savevm("piix4_pm", 0, 1, pm_save, pm_load, s);
+    vmstate_register(0, &vmstate_acpi, s);

     s->smbus = i2c_init_bus(NULL, "i2c");
     s->irq = sci_irq;
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 16/23] Port PCI Bus to VMState design

[Gerd Hoffmann]

On 08/20/09 19:42, Juan Quintela wrote:
> This uses VARRAY and INT32_EQUAL values

This is one place where I would change the code to reduce load/save 
complexity.

Quick grep shows that the max value for bus->nirqs is 32. 
s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation and 
allways save/load 32 values.  INT32_EQUAL + VARRAY isn't needed then.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 21/23] Port PCIDevice state to VMState

[Gerd Hoffmann]

On 08/20/09 19:42, Juan Quintela wrote:
> This uses a variant of buffer, with extra checks. Also uses the new
> support for cheking that a read value is less or equal than a field.

Hmm, why you are hopping through all these loops?

I'd just keep the existing pci_device_{save,load} functions.  load 
anyway for backward compatibility.  save also for devices not yet 
converted to vmstate.  I think alot of the complex stuff you are 
building up here isn't needed then, unless I missed something ...

Beside that we'll have to think about how to handle versioning of 
structs.  Do we want vmstate_pci_device (and all others) have its own 
version?  Probably makes sense.  Handling this should go into the 
generic code then and not be hacked into each structure using 
VMSTATE_INT32_LE().

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 00/23] New VMState table based load/save infrastructure

[Gerd Hoffmann]

On 08/20/09 19:42, Juan Quintela wrote:
> - i440fx_load()
>    Can we put pci_irq_levels[] array in any struct?  This is the only driver
>    that I have found that uses a global variable.  Gerd, can you take a look?
>    It is not possible to have the PCIDevice struct inside a PIIXDevice or similar?

Stuffing a pointer to pci_irq_levels into a struct should work without 
problems and should be good enougth to get vmstate save/load going ;)

Sticking the array itself into a struct would be even better, that might 
need more cleanup work though.

cheers,
   Gerd

[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 08/20/09 19:42, Juan Quintela wrote:
>> This uses a variant of buffer, with extra checks. Also uses the new
>> support for cheking that a read value is less or equal than a field.
>
> Hmm, why you are hopping through all these loops?
>
> I'd just keep the existing pci_device_{save,load} functions.  load
> anyway for backward compatibility.  save also for devices not yet
> converted to vmstate.  I think alot of the complex stuff you are
> building up here isn't needed then, unless I missed something ...

In this specific case, I think that sending:
size + number of things

it is safe.  I agree that the INT32_EQUAL is a bad idea. But that is how
the current code is done.

> Beside that we'll have to think about how to handle versioning of
> structs.  Do we want vmstate_pci_device (and all others) have its own
> version?  Probably makes sense.  Handling this should go into the
> generic code then and not be hacked into each structure using
> VMSTATE_INT32_LE().

Proper solution here is to use subsections.
Each time that you call VMSTATE_PCI_DEVICE() it sends a subsection
there.  Then we get the versioning by free.  This is how I am thinking
it is the right way to do it.

virtio stuff: the more than I think about it, the easier way is to just
get rid of the whole mess and do something that is sensible.

Later, Juan.

> cheers,
>   Gerd

[Qemu-devel] Re: [PATCH 16/23] Port PCI Bus to VMState design

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 08/20/09 19:42, Juan Quintela wrote:
>> This uses VARRAY and INT32_EQUAL values
>
> This is one place where I would change the code to reduce load/save
> complexity.
>
> Quick grep shows that the max value for bus->nirqs is
> 32. s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation
> and allways save/load 32 values.  INT32_EQUAL + VARRAY isn't needed
> then.

I agree to drop INT32_EQUAL, but maintain the VARRAY.
VARRAY is nice if there is another field in the stuct that tolds you how
many are used.

If you change the 32 number, you just need to upgrade the version.
Will do something like that.

The reason that I don't want to send the whole thing is that there are
times when we are sending tables that are empty (devices not being
used).  Having an initial size makes things go away.  And then, I will
create the VMSTATE_VARRAY() that handles the two fields in one go.

Later, Juan.

> cheers,
>   Gerd

[Qemu-devel] Re: [PATCH 00/23] New VMState table based load/save infrastructure

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 08/20/09 19:42, Juan Quintela wrote:
>> - i440fx_load()
>>    Can we put pci_irq_levels[] array in any struct?  This is the only driver
>>    that I have found that uses a global variable.  Gerd, can you take a look?
>>    It is not possible to have the PCIDevice struct inside a PIIXDevice or similar?
>
> Stuffing a pointer to pci_irq_levels into a struct should work without
> problems and should be good enougth to get vmstate save/load going ;)
>
> Sticking the array itself into a struct would be even better, that
> might need more cleanup work though.

Do you mean to do any of the tow approachs?
I promise to study your patch for other places that I need :)

Later, Juan.

[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState

[Gerd Hoffmann]

On 08/21/09 11:01, Juan Quintela wrote:
> I agree that the INT32_EQUAL is a bad idea. But that is how
> the current code is done.

Consider changing the code then ;)

>> Beside that we'll have to think about how to handle versioning of
>> structs.  Do we want vmstate_pci_device (and all others) have its own
>> version?  Probably makes sense.  Handling this should go into the
>> generic code then and not be hacked into each structure using
>> VMSTATE_INT32_LE().
>
> Proper solution here is to use subsections.
> Each time that you call VMSTATE_PCI_DEVICE() it sends a subsection
> there.  Then we get the versioning by free.  This is how I am thinking
> it is the right way to do it.

Yep, something like this.

> virtio stuff: the more than I think about it, the easier way is to just
> get rid of the whole mess and do something that is sensible.

You are talking about VirtIOPCIProxy I guess?
Yea, that is messy ...

cheers,
   Gerd

[Qemu-devel] Re: [PATCH 16/23] Port PCI Bus to VMState design

[Gerd Hoffmann]

On 08/21/09 11:04, Juan Quintela wrote:
> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> On 08/20/09 19:42, Juan Quintela wrote:
>>> This uses VARRAY and INT32_EQUAL values
>> This is one place where I would change the code to reduce load/save
>> complexity.
>>
>> Quick grep shows that the max value for bus->nirqs is
>> 32. s/*irq_count/irq_count[32]/ for PCIBus, drop dynamic allocation
>> and allways save/load 32 values.  INT32_EQUAL + VARRAY isn't needed
>> then.
>
> I agree to drop INT32_EQUAL, but maintain the VARRAY.
> VARRAY is nice if there is another field in the stuct that tolds you how
> many are used.

If you save all of them anyway you don't need that.
(but you still must save the bus->nirqs field obviously).

> If you change the 32 number, you just need to upgrade the version.
> Will do something like that.
>
> The reason that I don't want to send the whole thing is that there are
> times when we are sending tables that are empty (devices not being
> used).

I still would save it.  If you find a NULL pointer save a bunch of zeros 
instead.  Device state isn't that big, and I think it is good to have 
fixed-size sections.

cheers
   Gerd

[Qemu-devel] Re: [PATCH 00/23] New VMState table based load/save infrastructure

[Gerd Hoffmann]

On 08/21/09 11:12, Juan Quintela wrote:
> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> Stuffing a pointer to pci_irq_levels into a struct should work without
>> problems and should be good enougth to get vmstate save/load going ;)
>>
>> Sticking the array itself into a struct would be even better, that
>> might need more cleanup work though.
>
> Do you mean to do any of the tow approachs?
> I promise to study your patch for other places that I need :)
>
> Later, Juan.

I can have a look, probably next week though.

cheers,
   Gerd

[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 08/21/09 11:01, Juan Quintela wrote:
>> I agree that the INT32_EQUAL is a bad idea. But that is how
>> the current code is done.
>
> Consider changing the code then ;)
>
>>> Beside that we'll have to think about how to handle versioning of
>>> structs.  Do we want vmstate_pci_device (and all others) have its own
>>> version?  Probably makes sense.  Handling this should go into the
>>> generic code then and not be hacked into each structure using
>>> VMSTATE_INT32_LE().
>>
>> Proper solution here is to use subsections.
>> Each time that you call VMSTATE_PCI_DEVICE() it sends a subsection
>> there.  Then we get the versioning by free.  This is how I am thinking
>> it is the right way to do it.
>
> Yep, something like this.
>
>> virtio stuff: the more than I think about it, the easier way is to just
>> get rid of the whole mess and do something that is sensible.
>
> You are talking about VirtIOPCIProxy I guess?
> Yea, that is messy ...

I am talking about virtio_load() actually. How do you translate this to
a table?

- If at the beggining (and that load_config() reads the config from the
  savevm
- and another if in the middle of a for

int virtio_load(VirtIODevice *vdev, QEMUFile *f)
{
    int num, i, ret;

    if (vdev->binding->load_config) {
        ret = vdev->binding->load_config(vdev->binding_opaque, f);
        if (ret)
            return ret;
    }

    qemu_get_8s(f, &vdev->status);
    qemu_get_8s(f, &vdev->isr);
    qemu_get_be16s(f, &vdev->queue_sel);
    qemu_get_be32s(f, &vdev->features);
    vdev->config_len = qemu_get_be32(f);
    qemu_get_buffer(f, vdev->config, vdev->config_len);

    num = qemu_get_be32(f);

    for (i = 0; i < num; i++) {
        vdev->vq[i].vring.num = qemu_get_be32(f);
        vdev->vq[i].pa = qemu_get_be64(f);
        qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);

        if (vdev->vq[i].pa) {
            virtqueue_init(&vdev->vq[i]);
        }
        if (vdev->binding->load_queue) {
            ret = vdev->binding->load_queue(vdev->binding_opaque, i, f);
            if (ret)
                return ret;
        }
    }

    virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
    return 0;
}

Can we have something saner like:

int virtio_load(VirtIODevice *vdev, QEMUFile *f)
{
    int num, i, ret;

    qemu_get_be32s(f, &vdev->config_load_size);
    qemu_get_buffer(f, &vdev->config_load_value, &vdev->config_load_size);

    qemu_get_8s(f, &vdev->status);
    qemu_get_8s(f, &vdev->isr);
    qemu_get_be16s(f, &vdev->queue_sel);
    qemu_get_be32s(f, &vdev->features);
    vdev->config_len = qemu_get_be32(f);
    qemu_get_buffer(f, vdev->config, vdev->config_len);

    num = qemu_get_be32(f);

    for (i = 0; i < num; i++) {
        vdev->vq[i].vring.num = qemu_get_be32(f);
        vdev->vq[i].pa = qemu_get_be64(f);
        qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);

        qemu_get_be32s(f, &vdev->queue_size[i]);
        qemu_get_buffer(f, &vdev->queue_value, &vdev->queue_size[i]);
    }

    /* load of state has ended, time to start configuring things */

    if (vdev->binding->load_config) {
        ret = vdev->binding->load_config(vdev->binding_opaque, f);
        if (ret)
            return ret;
    }

    for (i = 0; i < num; i++) {
        if (vdev->vq[i].pa) {
            virtqueue_init(&vdev->vq[i]);
        if (vdev->binding->load_queue) {
            ret = vdev->binding->load_queue(vdev->binding_opaque, i, f);
            if (ret)
                return ret;
        }
        }
    }

    virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
    return 0;
}

I.e. we just reserved the space to load the stuff (0 is ok), and after
loading, We initialize things at virtio maintainers pace.  Obviously the
load_queue/config stuff has to be changed to work from a local variable
instead of the savevm stream.

Later, Juan.

[Qemu-devel] Re: [PATCH 00/23] New VMState table based load/save infrastructure

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 08/21/09 11:12, Juan Quintela wrote:
>> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>> Stuffing a pointer to pci_irq_levels into a struct should work without
>>> problems and should be good enougth to get vmstate save/load going ;)
>>>
>>> Sticking the array itself into a struct would be even better, that
>>> might need more cleanup work though.
>>
>> Do you mean to do any of the tow approachs?
>> I promise to study your patch for other places that I need :)
>>
>> Later, Juan.
>
> I can have a look, probably next week though.

I have just saw Markus Watchdog conversion.  Perhaps can get enough
"inspiration" from there to try to do it myself.

Later, Juan.

[Qemu-devel] Re: [PATCH 21/23] Port PCIDevice state to VMState

[Gerd Hoffmann]

On 08/21/09 11:30, Juan Quintela wrote:
> I am talking about virtio_load() actually. How do you translate this to
> a table?

Oh yea, that is nasty.

> int virtio_load(VirtIODevice *vdev, QEMUFile *f)
> {
>      int num, i, ret;
>
>      if (vdev->binding->load_config) {
>          ret = vdev->binding->load_config(vdev->binding_opaque, f);
>          if (ret)
>              return ret;
>      }

Hmm.  Move register_savevm from virtio.c to virtio_pci.c?

Then have virtio_load_pci() which basically calls pci_device_save() + 
virtio_common_save() and kill this binding indirection.

Note: There are non-pci virtio bindings somewhere in qemu.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 07/23] New VMstate save/load infrastructure

[Michael S. Tsirkin]

On Thu, Aug 20, 2009 at 07:42:25PM +0200, Juan Quintela wrote:
> This patch introduces VMState infrastructure, to convert the save/load
> functions of devices to a table approach.  This new approach has the
> following 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.
> - Uses old load_state() function for loading old state.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---

...

> +#define VMSTATE_SINGLE(_field, _state, _version, _info, _type) {     \
> +    .name       = (stringify(_field)),                               \
> +    .version_id = (_version),                                        \
> +    .size       = sizeof(_type),                                     \
> +    .info       = &(_info),                                          \
> +    .flags      = VMS_SINGLE,                                        \
> +    .offset     = offsetof(_state, _field)                           \
> +            + type_check(_type,typeof_field(_state, _field))         \
> +}


As I said in a different thread, I think the versioning approach
taken here is too limited. In particular, it does not easily support
optional features such as msix. We can use other mechanisms for this,
such as encoding options in the device name, but I think it's too
limiting.

What I think we should have instead of a version is an identifier for
each field. This way it is easy to add or skip each field separately,
depending on command line options, and we can also verify, at load time,
that all required fields are present. This will also reduce the amount
of duplicated code required each time we extend migration format.

As a bonus, we'll be able to write a utility parsing the state
and checking that it's sane.

-- 
MST