[Qemu-devel] [PATCH v2 0.15 3/4] savevm: define new unambiguous migration format

[Paolo Bonzini]

With the current migration format, VMS_STRUCTs with subsections
are ambiguous.  The protocol cannot tell whether a 0x5 byte after
the VMS_STRUCT is a subsection or part of the parent data stream.
In the past QEMU assumed it was always a part of a subsection; after
commit eb60260 (savevm: fix corruption in vmstate_subsection_load().,
2011-02-03) the choice depends on whether the VMS_STRUCT has subsections
defined.

Unfortunately, this means that if a destination has no subsections
defined for the struct, it will happily read subsection data into
its own fields.  And if you are "lucky" enough to stumble on a
zero byte at the right time, it will be interpreted as QEMU_VM_EOF
and migration will be interrupted.

There is no way out of this except defining an incompatible
migration protocol with a sentinel at the end of embedded structs.
Of course, this is restricted to new machine models.

Signed-off-by: Paolo Bonzini <address@hidden>
---
 hw/boards.h  |    3 +++
 hw/pc_piix.c |    5 +++++
 savevm.c     |   26 ++++++++++++++++++--------
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/hw/boards.h b/hw/boards.h
index 560dbaf..1cca3ce 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -5,6 +5,9 @@
 
 #include "qdev.h"
 
+#define QEMU_VM_FILE_VERSION_0_14    0x00000003
+#define QEMU_VM_FILE_VERSION         0x00000004
+
 typedef void QEMUMachineInitFunc(ram_addr_t ram_size,
                                  const char *boot_device,
                                  const char *kernel_filename,
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 61f8cbb..38818de 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -271,6 +271,7 @@ static QEMUMachine pc_machine_v0_14 = {
     .desc = "Standard PC",
     .init = pc_init_pci,
     .max_cpus = 255,
+    .migration_format = QEMU_VM_FILE_VERSION_0_14,
 };
 
 static QEMUMachine pc_machine_v0_13 = {
@@ -278,6 +279,7 @@ static QEMUMachine pc_machine_v0_13 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .migration_format = QEMU_VM_FILE_VERSION_0_14,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-9p-pci",
@@ -317,6 +319,7 @@ static QEMUMachine pc_machine_v0_12 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .migration_format = QEMU_VM_FILE_VERSION_0_14,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-serial-pci",
@@ -360,6 +363,7 @@ static QEMUMachine pc_machine_v0_11 = {
     .desc = "Standard PC, qemu 0.11",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .migration_format = QEMU_VM_FILE_VERSION_0_14,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-blk-pci",
@@ -411,6 +415,7 @@ static QEMUMachine pc_machine_v0_10 = {
     .desc = "Standard PC, qemu 0.10",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .migration_format = QEMU_VM_FILE_VERSION_0_14,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-blk-pci",
diff --git a/savevm.c b/savevm.c
index 3049aa1..197af4b 100644
--- a/savevm.c
+++ b/savevm.c
@@ -158,6 +158,14 @@ void qemu_announce_self(void)
 
 #define IO_BUF_SIZE 32768
 
+#define QEMU_VM_EOF                  0x00
+#define QEMU_VM_SECTION_START        0x01
+#define QEMU_VM_SECTION_PART         0x02
+#define QEMU_VM_SECTION_END          0x03
+#define QEMU_VM_SECTION_FULL         0x04
+#define QEMU_VM_SUBSECTION           0x05
+#define QEMU_VM_SUBSECTIONS_END      0x06
+
 struct QEMUFile {
     QEMUFilePutBufferFunc *put_buffer;
     QEMUFileGetBufferFunc *get_buffer;
@@ -1348,6 +1356,12 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
                 }
                 if (field->flags & VMS_STRUCT) {
                     ret = vmstate_load_state(f, field->vmsd, addr, 
field->vmsd->version_id);
+                    if (!current_machine->migration_format ||
+                        current_machine->migration_format >= 4) {
+                        if (qemu_get_byte(f) != QEMU_VM_SUBSECTIONS_END) {
+                            return -EINVAL;
+                        }
+                    }
                 } else {
                     ret = field->info->get(f, addr, size);
 
@@ -1410,6 +1424,10 @@ void vmstate_save_state(QEMUFile *f, const 
VMStateDescription *vmsd,
                 }
                 if (field->flags & VMS_STRUCT) {
                     vmstate_save_state(f, field->vmsd, addr);
+                    if (!current_machine->migration_format ||
+                        current_machine->migration_format >= 4) {
+                        qemu_put_byte(f, QEMU_VM_SUBSECTIONS_END);
+                    }
                 } else {
                     field->info->put(f, addr, size);
                 }
@@ -1439,14 +1457,6 @@ static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
 
 #define QEMU_VM_FILE_MAGIC           0x5145564d
 #define QEMU_VM_FILE_VERSION_COMPAT  0x00000002
-#define QEMU_VM_FILE_VERSION         0x00000003
-
-#define QEMU_VM_EOF                  0x00
-#define QEMU_VM_SECTION_START        0x01
-#define QEMU_VM_SECTION_PART         0x02
-#define QEMU_VM_SECTION_END          0x03
-#define QEMU_VM_SECTION_FULL         0x04
-#define QEMU_VM_SUBSECTION           0x05
 
 bool qemu_savevm_state_blocked(Monitor *mon)
 {
-- 
1.7.6