Patchwork [RFC,08/16] Visitor: Output path

login
register
mail settings
Submitter Dave Gilbert
Date March 25, 2014, 8:17 p.m.
Message ID <1395778647-30925-9-git-send-email-dgilbert@redhat.com>
Download mbox | patch
Permalink /patch/333707/
State New
Headers show

Comments

Dave Gilbert - March 25, 2014, 8:17 p.m.
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Replace QEMUFile by Visitor in most of the output path.
There are still a few places that want a QEMUFile* and
those are TODOs at the moment.

Hacks in pci.c, spapr, spapr_vscsi.c for now where new
visitor/qemufile boundaries have been added.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 arch_init.c                 | 147 ++++++++++++++++++++++---------
 block-migration.c           |  13 ++-
 hw/pci/pci.c                |   2 +-
 hw/ppc/spapr.c              |   9 +-
 hw/scsi/spapr_vscsi.c       |   3 +-
 include/migration/vmstate.h |   6 +-
 savevm.c                    | 208 +++++++++++++++++++++++++++++++-------------
 vmstate.c                   | 102 ++++++++++++++++++----
 8 files changed, 365 insertions(+), 125 deletions(-)

Patch

diff --git a/arch_init.c b/arch_init.c
index 73b9303..02bf78a 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -31,6 +31,7 @@ 
 #include "config.h"
 #include "monitor/monitor.h"
 #include "sysemu/sysemu.h"
+#include "qapi/visitor.h"
 #include "qemu/bitops.h"
 #include "qemu/bitmap.h"
 #include "sysemu/arch_init.h"
@@ -60,6 +61,13 @@ 
     do { } while (0)
 #endif
 
+#define LOCAL_ERR_REPORT(fallout) \
+    if (local_err) { \
+        error_report("%s:%d %s", __func__, __LINE__, \
+                     error_get_pretty(local_err));   \
+        fallout \
+    }
+
 #ifdef TARGET_SPARC
 int graphic_width = 1024;
 int graphic_height = 768;
@@ -70,7 +78,6 @@  int graphic_height = 600;
 int graphic_depth = 32;
 #endif
 
-
 #if defined(TARGET_ALPHA)
 #define QEMU_ARCH QEMU_ARCH_ALPHA
 #elif defined(TARGET_ARM)
@@ -281,20 +288,22 @@  uint64_t xbzrle_mig_pages_overflow(void)
     return acct_info.xbzrle_overflows;
 }
 
-static size_t save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
+static size_t save_block_hdr(Visitor *v, RAMBlock *block, ram_addr_t offset,
                              int cont, int flag)
 {
-    size_t size;
-
-    qemu_put_be64(f, offset | cont | flag);
-    size = 8;
+    size_t size = 8; /* Note bogus with visitor */
+    Error *local_err = NULL;
+    ramsecentry_header rse_hdr;
 
+    rse_hdr.addr = offset;
+    rse_hdr.flags = cont | flag;
     if (!cont) {
-        qemu_put_byte(f, strlen(block->idstr));
-        qemu_put_buffer(f, (uint8_t *)block->idstr,
-                        strlen(block->idstr));
+        strcpy(rse_hdr.idstr, block->idstr);
         size += 1 + strlen(block->idstr);
     }
+    visit_start_sequence_compat(v, "ramsecentry", VISIT_SEQ_COMPAT_RAMSECENTRY,
+                                &rse_hdr, &local_err);
+
     return size;
 }
 
@@ -328,10 +337,13 @@  static void xbzrle_cache_zero_page(ram_addr_t current_addr)
 
 #define ENCODING_FLAG_XBZRLE 0x1
 
-static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
+static int save_xbzrle_page(Visitor *v, uint8_t *current_data,
                             ram_addr_t current_addr, RAMBlock *block,
                             ram_addr_t offset, int cont, bool last_stage)
 {
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
+    Error *local_err = NULL;
+
     int encoded_len = 0, bytes_sent = -1;
     uint8_t *prev_cached_page;
 
@@ -371,7 +383,7 @@  static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
     }
 
     /* Send XBZRLE based compressed page */
-    bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_XBZRLE);
+    bytes_sent = save_block_hdr(v, block, offset, cont, RAM_SAVE_FLAG_XBZRLE);
     qemu_put_byte(f, ENCODING_FLAG_XBZRLE);
     qemu_put_be16(f, encoded_len);
     qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
@@ -379,6 +391,9 @@  static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
     acct_info.xbzrle_pages++;
     acct_info.xbzrle_bytes += bytes_sent;
 
+    visit_end_sequence_compat(v, "ramsecentry", VISIT_SEQ_COMPAT_RAMSECENTRY,
+                              &local_err);
+    LOCAL_ERR_REPORT(return -1;);
     return bytes_sent;
 }
 
@@ -523,8 +538,11 @@  static void migration_bitmap_sync(void)
  *           0 means no dirty pages
  */
 
-static int ram_save_block(QEMUFile *f, bool last_stage)
+static int ram_save_block(Visitor *v, bool last_stage)
 {
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
+    Error *local_err = NULL;
+
     RAMBlock *block = last_seen_block;
     ram_addr_t offset = last_offset;
     bool complete_round = false;
@@ -576,17 +594,23 @@  static int ram_save_block(QEMUFile *f, bool last_stage)
                     }
                 }
             } else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
+                uint8_t tmpbyte;
                 acct_info.dup_pages++;
-                bytes_sent = save_block_hdr(f, block, offset, cont,
+                bytes_sent = save_block_hdr(v, block, offset, cont,
                                             RAM_SAVE_FLAG_COMPRESS);
-                qemu_put_byte(f, 0);
+                tmpbyte = 0;
+                visit_type_uint8(v, &tmpbyte, "pagefill", &local_err);
+                visit_end_sequence_compat(v, "ramsecentry",
+                                          VISIT_SEQ_COMPAT_RAMSECENTRY,
+                                          &local_err);
+                LOCAL_ERR_REPORT(return -1;);
                 bytes_sent++;
                 /* Must let xbzrle know, otherwise a previous (now 0'd) cached
                  * page would be stale
                  */
                 xbzrle_cache_zero_page(current_addr);
             } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
-                bytes_sent = save_xbzrle_page(f, p, current_addr, block,
+                bytes_sent = save_xbzrle_page(v, p, current_addr, block,
                                               offset, cont, last_stage);
                 if (!last_stage) {
                     /* We must send exactly what's in the xbzrle cache
@@ -604,12 +628,14 @@  static int ram_save_block(QEMUFile *f, bool last_stage)
 
             /* XBZRLE overflow or normal page */
             if (bytes_sent == -1) {
-                bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE);
-                if (send_async) {
-                    qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
-                } else {
-                    qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
-                }
+                bytes_sent = save_block_hdr(v, block, offset, cont,
+                                            RAM_SAVE_FLAG_PAGE);
+                visit_type_buffer(v, p, TARGET_PAGE_SIZE, send_async, "page",
+                                            &local_err);
+                visit_end_sequence_compat(v, "ramsecentry",
+                                          VISIT_SEQ_COMPAT_RAMSECENTRY,
+                                          &local_err);
+                LOCAL_ERR_REPORT(return -1;);
                 bytes_sent += TARGET_PAGE_SIZE;
                 acct_info.norm_pages++;
             }
@@ -711,10 +737,12 @@  static void reset_ram_globals(void)
 
 #define MAX_WAIT 50 /* ms, half buffered_file limit */
 
-static int ram_save_setup(QEMUFile *f, void *opaque)
+static int ram_save_setup(Visitor *v, void *opaque)
 {
     RAMBlock *block;
     int64_t ram_pages = last_ram_offset() >> TARGET_PAGE_BITS;
+    Error *local_err = NULL;
+    ramsecentry_header rse_hdr;
 
     migration_bitmap = bitmap_new(ram_pages);
     bitmap_set(migration_bitmap, 0, ram_pages);
@@ -762,31 +790,65 @@  static int ram_save_setup(QEMUFile *f, void *opaque)
     migration_bitmap_sync();
     qemu_mutex_unlock_iothread();
 
-    qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
+    visit_start_sequence_compat(v, "ramseclist", VISIT_SEQ_COMPAT_RAMSECLIST,
+                                NULL, &local_err);
+    rse_hdr.addr = ram_bytes_total();
+    rse_hdr.flags = RAM_SAVE_FLAG_MEM_SIZE;
+    visit_start_sequence_compat(v, "ramsecentry", VISIT_SEQ_COMPAT_RAMSECENTRY,
+                                &rse_hdr, &local_err);
+    LOCAL_ERR_REPORT(
+        qemu_mutex_unlock_ramlist();
+        return -1;
+    );
 
+    /* Note in the binary file the list is ended when it hits ram_bytes_total
+     * in length rather than any terminator
+     */
+    visit_start_list(v, "blocklist", &local_err);
     QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-        qemu_put_byte(f, strlen(block->idstr));
-        qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
-        qemu_put_be64(f, block->length);
+        visit_next_list(v, NULL, &local_err);
+
+        visit_start_struct(v, NULL, "blockid", "blockid", 0, &local_err);
+        visit_type_str256(v, block->idstr, "blockname", &local_err);
+        visit_type_uint64(v, &block->length, "blocklen", &local_err);
+        LOCAL_ERR_REPORT(
+            qemu_mutex_unlock_ramlist();
+            return -1;
+        );
+        visit_end_struct(v, &local_err);
     }
+    LOCAL_ERR_REPORT(
+        qemu_mutex_unlock_ramlist();
+        return -1;
+    );
+
+    visit_end_list(v, &local_err);
 
     qemu_mutex_unlock_ramlist();
 
-    ram_control_before_iterate(f, RAM_CONTROL_SETUP);
-    ram_control_after_iterate(f, RAM_CONTROL_SETUP);
+    visit_end_sequence_compat(v, "ramsecentry", VISIT_SEQ_COMPAT_RAMSECENTRY,
+                              &local_err);
+    LOCAL_ERR_REPORT(return -1;);
+    ram_control_before_iterate(visitor_get_qemufile(v), RAM_CONTROL_SETUP);
+    ram_control_after_iterate(visitor_get_qemufile(v), RAM_CONTROL_SETUP);
 
-    qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+    visit_end_sequence_compat(v, "ramseclist", VISIT_SEQ_COMPAT_RAMSECLIST,
+                              &local_err);
 
     return 0;
 }
 
-static int ram_save_iterate(QEMUFile *f, void *opaque)
+static int ram_save_iterate(Visitor *v, void *opaque)
 {
+    QEMUFile *f = visitor_get_qemufile(v);
+    Error *local_err = NULL;
     int ret;
     int i;
     int64_t t0;
     int total_sent = 0;
 
+    visit_start_sequence_compat(v, "ramseclist", VISIT_SEQ_COMPAT_RAMSECLIST,
+                                NULL, &local_err);
     qemu_mutex_lock_ramlist();
 
     if (ram_list.version != last_version) {
@@ -800,7 +862,7 @@  static int ram_save_iterate(QEMUFile *f, void *opaque)
     while ((ret = qemu_file_rate_limit(f)) == 0) {
         int bytes_sent;
 
-        bytes_sent = ram_save_block(f, false);
+        bytes_sent = ram_save_block(v, false);
         /* no more blocks to sent */
         if (bytes_sent == 0) {
             break;
@@ -838,7 +900,8 @@  static int ram_save_iterate(QEMUFile *f, void *opaque)
      * Do not count these 8 bytes into total_sent, so that we can
      * return 0 if no page had been dirtied.
      */
-    qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+    visit_end_sequence_compat(v, "ramseclist", VISIT_SEQ_COMPAT_RAMSECLIST,
+                              &local_err);
     bytes_transferred += 8;
 
     ret = qemu_file_get_error(f);
@@ -846,15 +909,19 @@  static int ram_save_iterate(QEMUFile *f, void *opaque)
         return ret;
     }
 
+    LOCAL_ERR_REPORT(return -EINVAL;);
     return total_sent;
 }
 
-static int ram_save_complete(QEMUFile *f, void *opaque)
+static int ram_save_complete(Visitor *v, void *opaque)
 {
+    Error *local_err = NULL;
     qemu_mutex_lock_ramlist();
     migration_bitmap_sync();
 
-    ram_control_before_iterate(f, RAM_CONTROL_FINISH);
+    visit_start_sequence_compat(v, "ramseclist", VISIT_SEQ_COMPAT_RAMSECLIST,
+                                NULL, &local_err);
+    ram_control_before_iterate(visitor_get_qemufile(v), RAM_CONTROL_FINISH);
 
     /* try transferring iterative blocks of memory */
 
@@ -862,24 +929,26 @@  static int ram_save_complete(QEMUFile *f, void *opaque)
     while (true) {
         int bytes_sent;
 
-        bytes_sent = ram_save_block(f, true);
-        /* no more blocks to sent */
+        bytes_sent = ram_save_block(v, true);
+        /* no more blocks to send */
         if (bytes_sent == 0) {
             break;
         }
         bytes_transferred += bytes_sent;
     }
 
-    ram_control_after_iterate(f, RAM_CONTROL_FINISH);
+    ram_control_after_iterate(visitor_get_qemufile(v), RAM_CONTROL_FINISH);
+    visit_end_sequence_compat(v, "ramseclist", VISIT_SEQ_COMPAT_RAMSECLIST,
+                              &local_err);
     migration_end();
 
     qemu_mutex_unlock_ramlist();
-    qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+    LOCAL_ERR_REPORT(return -EINVAL;);
 
     return 0;
 }
 
-static uint64_t ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size)
+static uint64_t ram_save_pending(void *opaque, uint64_t max_size)
 {
     uint64_t remaining_size;
 
diff --git a/block-migration.c b/block-migration.c
index 897fdba..3475d76 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -16,6 +16,7 @@ 
 #include "qemu-common.h"
 #include "block/block_int.h"
 #include "hw/hw.h"
+#include "qapi/visitor.h"
 #include "qemu/queue.h"
 #include "qemu/timer.h"
 #include "migration/block.h"
@@ -603,9 +604,10 @@  static void block_migration_cancel(void *opaque)
     blk_mig_cleanup();
 }
 
-static int block_save_setup(QEMUFile *f, void *opaque)
+static int block_save_setup(Visitor *v, void *opaque)
 {
     int ret;
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
 
     DPRINTF("Enter save live setup submitted %d transferred %d\n",
             block_mig_state.submitted, block_mig_state.transferred);
@@ -624,8 +626,10 @@  static int block_save_setup(QEMUFile *f, void *opaque)
     return ret;
 }
 
-static int block_save_iterate(QEMUFile *f, void *opaque)
+static int block_save_iterate(Visitor *v, void *opaque)
 {
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
+
     int ret;
     int64_t last_ftell = qemu_ftell(f);
 
@@ -682,8 +686,9 @@  static int block_save_iterate(QEMUFile *f, void *opaque)
 
 /* Called with iothread lock taken.  */
 
-static int block_save_complete(QEMUFile *f, void *opaque)
+static int block_save_complete(Visitor *v, void *opaque)
 {
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
     int ret;
 
     DPRINTF("Enter save live complete submitted %d transferred %d\n",
@@ -720,7 +725,7 @@  static int block_save_complete(QEMUFile *f, void *opaque)
     return 0;
 }
 
-static uint64_t block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size)
+static uint64_t block_save_pending(void *opaque, uint64_t max_size)
 {
     /* Estimate pending number of bytes to send */
     uint64_t pending;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 8f722dd..e91f729 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -515,7 +515,7 @@  void pci_device_save(PCIDevice *s, QEMUFile *f)
      * This makes us compatible with old devices
      * which never set or clear this bit. */
     s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
-    vmstate_save_state(f, pci_get_vmstate(s), s);
+    vmstate_save_state(qemu_file_get_tmp_visitor(f), pci_get_vmstate(s), s);
     /* Restore the interrupt status bit. */
     pci_update_irq_status(s);
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a11e121..34443c1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -835,9 +835,10 @@  static const VMStateDescription vmstate_spapr = {
 #define HPTE_DIRTY(_hpte)  (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY)
 #define CLEAN_HPTE(_hpte)  ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY))
 
-static int htab_save_setup(QEMUFile *f, void *opaque)
+static int htab_save_setup(Visitor *v, void *opaque)
 {
     sPAPREnvironment *spapr = opaque;
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
 
     /* "Iteration" header */
     qemu_put_be32(f, spapr->htab_shift);
@@ -990,8 +991,9 @@  static int htab_save_later_pass(QEMUFile *f, sPAPREnvironment *spapr,
 #define MAX_ITERATION_NS    5000000 /* 5 ms */
 #define MAX_KVM_BUF_SIZE    2048
 
-static int htab_save_iterate(QEMUFile *f, void *opaque)
+static int htab_save_iterate(Visitor *v, void *opaque)
 {
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
     sPAPREnvironment *spapr = opaque;
     int rc = 0;
 
@@ -1020,8 +1022,9 @@  static int htab_save_iterate(QEMUFile *f, void *opaque)
     return rc;
 }
 
-static int htab_save_complete(QEMUFile *f, void *opaque)
+static int htab_save_complete(Visitor *v, void *opaque)
 {
+    QEMUFile *f = visitor_get_qemufile(v); // TODO
     sPAPREnvironment *spapr = opaque;
 
     /* Iteration header */
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 34478f0..8e799e2 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -624,7 +624,8 @@  static void vscsi_save_request(QEMUFile *f, SCSIRequest *sreq)
     vscsi_req *req = sreq->hba_private;
     assert(req->active);
 
-    vmstate_save_state(f, &vmstate_spapr_vscsi_req, req);
+    vmstate_save_state(qemu_file_get_tmp_visitor(f), &vmstate_spapr_vscsi_req,
+                       req);
 
     DPRINTF("VSCSI: saving tag=%u, current desc#%d, offset=%x\n",
             req->qtag, req->cur_desc_num, req->cur_desc_offset);
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index a5e4b0b..b66342b 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -40,7 +40,7 @@  typedef struct SaveVMHandlers {
     SaveStateHandler *save_state;
 
     void (*cancel)(void *opaque);
-    int (*save_live_complete)(QEMUFile *f, void *opaque);
+    int (*save_live_complete)(Visitor *v, void *opaque);
 
     /* This runs both outside and inside the iothread lock.  */
     bool (*is_active)(void *opaque);
@@ -98,6 +98,8 @@  struct VMStateInfo {
     const char *name;
     int (*get)(QEMUFile *f, void *pv, size_t size);
     void (*put)(QEMUFile *f, void *pv, size_t size);
+    int (*visit)(Visitor *v, void *pdata, const char* name, size_t size,
+                 Error **errp);
 };
 
 enum VMStateFlags {
@@ -759,7 +761,7 @@  extern const VMStateInfo vmstate_info_bitmap;
 
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id);
-void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
+void vmstate_save_state(Visitor *v, const VMStateDescription *vmsd,
                         void *opaque);
 
 int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
diff --git a/savevm.c b/savevm.c
index d094fbb..51efd4a 100644
--- a/savevm.c
+++ b/savevm.c
@@ -24,6 +24,8 @@ 
 
 #include "config-host.h"
 #include "qemu-common.h"
+#include "qapi/qemu-file-binary-input-visitor.h"
+#include "qapi/qemu-file-binary-output-visitor.h"
 #include "hw/hw.h"
 #include "hw/qdev.h"
 #include "net/net.h"
@@ -51,6 +53,13 @@ 
 #define ARP_PTYPE_IP 0x0800
 #define ARP_OP_REQUEST_REV 0x3
 
+#define LOCAL_ERR_REPORT(fallout) \
+    if (local_err) { \
+        error_report("%s:%d %s", __func__, __LINE__, \
+                     error_get_pretty(local_err)); \
+        fallout \
+    }
+
 static int announce_self_create(uint8_t *buf,
                                 uint8_t *mac_addr)
 {
@@ -435,13 +444,42 @@  static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id)
     return vmstate_load_state(f, se->vmsd, se->opaque, version_id);
 }
 
-static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
+static int vmstate_save(Visitor *v, SaveStateEntry *se, int version_id)
 {
+    int ret = 0;
+    Error *local_err = NULL;
+
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        return -1;
+    }
+
     if (!se->vmsd) {         /* Old style */
-        se->ops->save_state(f, se->opaque);
-        return;
+        QEMUFile *wrapperf;
+
+        /* Some visitors put old format things down separate QEMUFile's */
+        visit_start_sequence_compat(v, "oldstate", VISIT_SEQ_COMPAT_BLOB,
+                                    &wrapperf, &local_err);
+        if (local_err) {
+            goto out_lerr;
+        }
+
+        se->ops->save_state(wrapperf, se->opaque);
+
+        visit_end_sequence_compat(v, "oldstate", VISIT_SEQ_COMPAT_BLOB,
+                                  &local_err);
+        if (local_err) {
+            goto out_lerr;
+        }
+        return ret;
     }
-    vmstate_save_state(f, se->vmsd, se->opaque);
+    vmstate_save_state(v, se->vmsd, se->opaque);
+
+    return ret;
+
+out_lerr:
+    error_report("%s", error_get_pretty(local_err));
+    return -EINVAL;
 }
 
 bool qemu_savevm_state_blocked(Error **errp)
@@ -462,6 +500,12 @@  void qemu_savevm_state_begin(QEMUFile *f,
 {
     SaveStateEntry *se;
     int ret;
+    Error *local_err = NULL;
+    SectionHeader sh;
+
+    QemuFileBinOutputVisitor *qfbov = qemu_file_bin_output_visitor_new(f);
+    Visitor *v = qemu_file_bin_output_get_visitor(qfbov);
+    qemu_file_set_tmp_visitor(f, v);
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if (!se->ops || !se->ops->set_params) {
@@ -470,11 +514,14 @@  void qemu_savevm_state_begin(QEMUFile *f,
         se->ops->set_params(params, se->opaque);
     }
 
-    qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
-    qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+    /* Maybe we should squash these two together? */
+    visit_start_sequence_compat(v, "file", VISIT_SEQ_COMPAT_FILE, NULL,
+                                &local_err);
+    visit_start_sequence_compat(v, "top", VISIT_SEQ_COMPAT_BYTE0TERM, NULL,
+                                &local_err);
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-        int len;
+        int32_t section_type;
 
         if (!se->ops || !se->ops->save_live_setup) {
             continue;
@@ -485,18 +532,19 @@  void qemu_savevm_state_begin(QEMUFile *f,
             }
         }
         /* Section type */
-        qemu_put_byte(f, QEMU_VM_SECTION_START);
-        qemu_put_be32(f, se->section_id);
-
-        /* ID string */
-        len = strlen(se->idstr);
-        qemu_put_byte(f, len);
-        qemu_put_buffer(f, (uint8_t *)se->idstr, len);
-
-        qemu_put_be32(f, se->instance_id);
-        qemu_put_be32(f, se->version_id);
-
-        ret = se->ops->save_live_setup(f, se->opaque);
+        section_type = QEMU_VM_SECTION_START;
+        visit_get_next_type(v, &section_type, NULL, "secstart", &local_err);
+
+        sh.section_id = se->section_id;
+        strcpy(sh.idstr, se->idstr);
+        sh.instance_id = se->instance_id;
+        sh.version_id = se->version_id;
+        visit_start_sequence_compat(v, "secstart",
+                        VISIT_SEQ_COMPAT_SECTION_HEADER, &sh, &local_err);
+
+        ret = se->ops->save_live_setup(v, se->opaque);
+        visit_end_sequence_compat(v, "secstart",
+                                  VISIT_SEQ_COMPAT_SECTION_HEADER, &local_err);
         if (ret < 0) {
             qemu_file_set_error(f, ret);
             break;
@@ -512,8 +560,12 @@  void qemu_savevm_state_begin(QEMUFile *f,
  */
 int qemu_savevm_state_iterate(QEMUFile *f)
 {
+    Visitor *v = qemu_file_get_tmp_visitor(f);
     SaveStateEntry *se;
     int ret = 1;
+    Error *local_err = NULL;
+    SectionHeader sh;
+    int32_t section_type;
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if (!se->ops || !se->ops->save_live_iterate) {
@@ -529,12 +581,21 @@  int qemu_savevm_state_iterate(QEMUFile *f)
         }
         trace_savevm_section_start(se->idstr, se->section_id);
         /* Section type */
-        qemu_put_byte(f, QEMU_VM_SECTION_PART);
-        qemu_put_be32(f, se->section_id);
+        section_type = QEMU_VM_SECTION_PART;
+        visit_get_next_type(v, &section_type, NULL, "secpart", &local_err);
+
+        sh.section_id = se->section_id;
+        visit_start_sequence_compat(v, "secpart", VISIT_SEQ_COMPAT_SECTION_MIN,
+                                    &sh, &local_err);
+
+        ret = se->ops->save_live_iterate(v, se->opaque);
+        visit_end_sequence_compat(v, "secpart", VISIT_SEQ_COMPAT_SECTION_MIN,
+                                  &local_err);
 
-        ret = se->ops->save_live_iterate(f, se->opaque);
         trace_savevm_section_end(se->idstr, se->section_id);
 
+        LOCAL_ERR_REPORT(return -EINVAL;);
+
         if (ret < 0) {
             qemu_file_set_error(f, ret);
         }
@@ -552,10 +613,18 @@  int qemu_savevm_state_iterate(QEMUFile *f)
 void qemu_savevm_state_complete(QEMUFile *f)
 {
     SaveStateEntry *se;
+    SectionHeader sh;
     int ret;
 
     cpu_synchronize_all_states();
 
+    Visitor *v = qemu_file_get_tmp_visitor(f);
+    Error *local_err = NULL;
+    int32_t section_type;
+
+    /* The visitor is in the middle of the 'top' sequence_compat structure
+     * started in begin.
+     */
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         if (!se->ops || !se->ops->save_live_complete) {
             continue;
@@ -567,41 +636,54 @@  void qemu_savevm_state_complete(QEMUFile *f)
         }
         trace_savevm_section_start(se->idstr, se->section_id);
         /* Section type */
-        qemu_put_byte(f, QEMU_VM_SECTION_END);
-        qemu_put_be32(f, se->section_id);
-
-        ret = se->ops->save_live_complete(f, se->opaque);
+        section_type = QEMU_VM_SECTION_END;
+        visit_get_next_type(v, &section_type, NULL, "secend", &local_err);
+        LOCAL_ERR_REPORT()
+        sh.section_id = se->section_id;
+        visit_start_sequence_compat(v, "secend", VISIT_SEQ_COMPAT_SECTION_MIN,
+                                    &sh, &local_err);
+        LOCAL_ERR_REPORT()
+
+        ret = se->ops->save_live_complete(v, se->opaque);
         trace_savevm_section_end(se->idstr, se->section_id);
-        if (ret < 0) {
+        visit_end_sequence_compat(v, "secend", VISIT_SEQ_COMPAT_SECTION_MIN,
+                                  &local_err);
+        if ((ret < 0) || local_err) {
+            LOCAL_ERR_REPORT()
             qemu_file_set_error(f, ret);
+            visit_destroy(v, &local_err);
             return;
         }
     }
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-        int len;
-
         if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
             continue;
         }
         trace_savevm_section_start(se->idstr, se->section_id);
         /* Section type */
-        qemu_put_byte(f, QEMU_VM_SECTION_FULL);
-        qemu_put_be32(f, se->section_id);
-
-        /* ID string */
-        len = strlen(se->idstr);
-        qemu_put_byte(f, len);
-        qemu_put_buffer(f, (uint8_t *)se->idstr, len);
-
-        qemu_put_be32(f, se->instance_id);
-        qemu_put_be32(f, se->version_id);
-
-        vmstate_save(f, se);
+        /* Note: the get_next_type plants the section type in compat mode */
+        section_type = QEMU_VM_SECTION_FULL;
+        visit_get_next_type(v, &section_type, NULL, "secfull", &local_err);
+
+        sh.section_id = se->section_id;
+        strcpy(sh.idstr, se->idstr);
+        sh.instance_id = se->instance_id;
+        sh.version_id = se->version_id;
+        visit_start_sequence_compat(v, "secfull",
+                        VISIT_SEQ_COMPAT_SECTION_HEADER, &sh, &local_err);
+
+        vmstate_save(v, se, se->version_id);
+        visit_end_sequence_compat(v, "secfull",
+                                  VISIT_SEQ_COMPAT_SECTION_HEADER, &local_err);
         trace_savevm_section_end(se->idstr, se->section_id);
     }
+    visit_end_sequence_compat(v, "top", VISIT_SEQ_COMPAT_BYTE0TERM, &local_err);
+    visit_end_sequence_compat(v, "file", VISIT_SEQ_COMPAT_FILE, &local_err);
+    visit_destroy(v, &local_err);
+
+    LOCAL_ERR_REPORT(ret = -EINVAL;)
 
-    qemu_put_byte(f, QEMU_VM_EOF);
     qemu_fflush(f);
 }
 
@@ -619,7 +701,7 @@  uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size)
                 continue;
             }
         }
-        ret += se->ops->save_live_pending(f, se->opaque, max_size);
+        ret += se->ops->save_live_pending(se->opaque, max_size);
     }
     return ret;
 }
@@ -671,38 +753,44 @@  static int qemu_savevm_state(QEMUFile *f)
 static int qemu_save_device_state(QEMUFile *f)
 {
     SaveStateEntry *se;
+    Error *local_err;
+    SectionHeader sh;
 
-    qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
-    qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+    QemuFileBinOutputVisitor *qfbov = qemu_file_bin_output_visitor_new(f);
+    qemu_file_set_tmp_visitor(f, qemu_file_bin_output_get_visitor(qfbov));
+    Visitor *v = qemu_file_bin_output_get_visitor(qfbov);
+    int32_t section_type;
 
     cpu_synchronize_all_states();
-
+    visit_start_sequence_compat(v, "file", VISIT_SEQ_COMPAT_FILE, NULL,
+                                &local_err);
+    visit_start_sequence_compat(v, "top", VISIT_SEQ_COMPAT_BYTE0TERM,
+                                NULL, &local_err);
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-        int len;
-
         if (se->is_ram) {
             continue;
         }
-        if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
+        if ((!se->ops || (!se->ops->save_state)) && !se->vmsd) {
             continue;
         }
 
         /* Section type */
-        qemu_put_byte(f, QEMU_VM_SECTION_FULL);
-        qemu_put_be32(f, se->section_id);
-
-        /* ID string */
-        len = strlen(se->idstr);
-        qemu_put_byte(f, len);
-        qemu_put_buffer(f, (uint8_t *)se->idstr, len);
+        section_type = QEMU_VM_SECTION_FULL;
+        visit_get_next_type(v, &section_type, NULL, "secfull", &local_err);
 
-        qemu_put_be32(f, se->instance_id);
-        qemu_put_be32(f, se->version_id);
+        sh.section_id = se->section_id;
+        strcpy(sh.idstr, se->idstr);
+        sh.instance_id = se->instance_id;
+        sh.version_id = se->version_id;
+        visit_start_sequence_compat(v, "secfull",
+                        VISIT_SEQ_COMPAT_SECTION_HEADER, &sh, &local_err);
 
-        vmstate_save(f, se);
+        vmstate_save(v, se, se->version_id);
     }
 
-    qemu_put_byte(f, QEMU_VM_EOF);
+    visit_end_sequence_compat(v, "top", VISIT_SEQ_COMPAT_BYTE0TERM, &local_err);
+    visit_end_sequence_compat(v, "file", VISIT_SEQ_COMPAT_FILE, &local_err);
+    visit_destroy(v, &local_err);
 
     return qemu_file_get_error(f);
 }
diff --git a/vmstate.c b/vmstate.c
index d1f5eb0..46bf7b9 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -3,12 +3,22 @@ 
 #include "migration/qemu-file.h"
 #include "migration/vmstate.h"
 #include "qemu/bitops.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qapi/visitor.h"
 
-static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
+static void vmstate_subsection_save(Visitor *v, const VMStateDescription *vmsd,
                                     void *opaque);
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
                                    void *opaque);
 
+#define LOCAL_ERR_REPORT(fallout) \
+    if (local_err) { \
+        error_report("%s:%d %s", __func__, __LINE__,  \
+                     error_get_pretty(local_err)); \
+        fallout \
+    }
+
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id)
 {
@@ -93,6 +103,14 @@  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                         void *opaque)
 {
     VMStateField *field = vmsd->fields;
+    Error *local_err = NULL;
+    uint32_t tmp32;
+
+    /* BER type comes from the vmsd if it's set */
+    tmp32 = vmsd->ber_tag;
+    visit_start_sequence_compat(v, vmsd->name, VISIT_SEQ_COMPAT_VMSTATE,
+                                &tmp32, &local_err);
+    LOCAL_ERR_REPORT(return;);
 
     if (vmsd->pre_save) {
         vmsd->pre_save(opaque);
@@ -124,6 +142,11 @@  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
             if (field->flags & VMS_POINTER) {
                 base_addr = *(void **)base_addr + field->start;
             }
+
+            if (n_elems > 1) {
+                visit_start_array(v, NULL, field->name, n_elems, 0, &local_err);
+            }
+
             for (i = 0; i < n_elems; i++) {
                 void *addr = base_addr + size * i;
 
@@ -131,19 +154,50 @@  void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                     addr = *(void **)addr;
                 }
                 if (field->flags & VMS_STRUCT) {
-                    vmstate_save_state(f, field->vmsd, addr);
+                    vmstate_save_state(v, field->vmsd, addr);
                 } else {
-                    field->info->put(f, addr, size);
+                    if (field->info->visit) {
+                        field->info->visit(v, addr, field->name, size,
+                                           &local_err);
+                    } else {
+                        QEMUFile *wrapperf;
+
+                        /*
+                         * Some visitors put old format things down separate
+                         * QEMUFile's
+                         */
+                        visit_start_sequence_compat(v, field->name,
+                                                    VISIT_SEQ_COMPAT_BLOB,
+                                                    &wrapperf, &local_err);
+                        LOCAL_ERR_REPORT(return;);
+
+                        field->info->put(wrapperf, addr, size);
+
+                        visit_end_sequence_compat(v, field->name,
+                                                  VISIT_SEQ_COMPAT_BLOB,
+                                                  &local_err);
+                        LOCAL_ERR_REPORT(return;);
+                    }
+                }
+                if ((i+1) != n_elems) {
+                    visit_next_array(v, &local_err);
                 }
             }
+            if (n_elems > 1) {
+                visit_end_array(v, &local_err);
+            }
         }
         field++;
     }
-    vmstate_subsection_save(f, vmsd, opaque);
+    vmstate_subsection_save(v, vmsd, opaque);
+
+    visit_end_sequence_compat(v, vmsd->name, VISIT_SEQ_COMPAT_VMSTATE,
+                              &local_err);
+    LOCAL_ERR_REPORT(return;);
 }
 
 static const VMStateDescription *
-    vmstate_get_subsection(const VMStateSubsection *sub, char *idstr)
+    vmstate_subsection_name_lookup(const VMStateSubsection *sub, char *idstr)
 {
     while (sub && sub->needed) {
         if (strcmp(idstr, sub->vmsd->name) == 0) {
@@ -195,25 +249,43 @@  static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
     return 0;
 }
 
-static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
+static void vmstate_subsection_save(Visitor *v, const VMStateDescription *vmsd,
                                     void *opaque)
 {
     const VMStateSubsection *sub = vmsd->subsections;
+    Error *local_err = NULL;
+
+    if (!sub) {
+        /*
+         * If the type has no subsection defined at all then skip completely
+         * Note that this means if we have conditional subsections we might
+         * plant an empty 'subseclist' if none of them are in use
+         */
+        return;
+    }
 
+    visit_start_sequence_compat(v, "subseclist", VISIT_SEQ_COMPAT_SUBSECLIST,
+                                (char *)vmsd->name, &local_err);
     while (sub && sub->needed) {
         if (sub->needed(opaque)) {
-            const VMStateDescription *vmsd = sub->vmsd;
-            uint8_t len;
-
-            qemu_put_byte(f, QEMU_VM_SUBSECTION);
-            len = strlen(vmsd->name);
-            qemu_put_byte(f, len);
-            qemu_put_buffer(f, (uint8_t *)vmsd->name, len);
-            qemu_put_be32(f, vmsd->version_id);
-            vmstate_save_state(f, vmsd, opaque);
+            const VMStateDescription *sub_vmsd = sub->vmsd;
+            SectionHeader sh;
+
+            sh.version_id = sub_vmsd->version_id;
+            strcpy(sh.idstr, sub_vmsd->name);
+            visit_start_sequence_compat(v, "subsection",
+                                        VISIT_SEQ_COMPAT_SUBSECTION,
+                                        &sh, &local_err);
+            vmstate_save_state(v, sub_vmsd, opaque);
+
+            visit_end_sequence_compat(v, "subsection",
+                                      VISIT_SEQ_COMPAT_SUBSECTION, &local_err);
         }
         sub++;
     }
+    visit_end_sequence_compat(v, "subseclist", VISIT_SEQ_COMPAT_SUBSECLIST,
+                              &local_err);
+    LOCAL_ERR_REPORT(return;);
 }
 
 /* bool */