diff mbox

[RFC,3/4] savevm: define new unambiguous migration format

Message ID 1309448777-1447-4-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini June 30, 2011, 3:46 p.m. UTC 
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 <pbonzini@redhat.com>
---
 hw/pc_piix.c |    6 ++++++
 savevm.c     |   27 +++++++++++++++++++--------
 2 files changed, 25 insertions(+), 8 deletions(-)

Comments

Anthony Liguori July 29, 2011, 1:12 p.m. UTC | #1 
On 06/30/2011 10:46 AM, Paolo Bonzini wrote:
> 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<pbonzini@redhat.com>
> ---
>   hw/pc_piix.c |    6 ++++++
>   savevm.c     |   27 +++++++++++++++++++--------
>   2 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 18cc942..d8d629c 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 = 3,
>   };

Please introduce a macro so this code is readable.

We have other machines that support migration in other archs too.

Regards,

Anthony Liguori
Paolo Bonzini July 29, 2011, 2:35 p.m. UTC | #2 
On 07/29/2011 03:12 PM, Anthony Liguori wrote:
> Please introduce a macro so this code is readable.

Ok.

> We have other machines that support migration in other archs too.

Those machine types are not versioned, so they will automatically switch 
to the newest version.

Paolo
diff mbox

Patch

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 18cc942..d8d629c 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 = 3,
 };
 
 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 = 3,
     .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 = 3,
     .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 = 3,
     .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 = 3,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-blk-pci",
@@ -474,6 +479,7 @@  static QEMUMachine isapc_machine = {
     .desc = "ISA-only PC",
     .init = pc_init_isa,
     .max_cpus = 1,
+    .migration_format = 3,
 };
 
 #ifdef CONFIG_XEN
diff --git a/savevm.c b/savevm.c
index 74e6e99..654770a 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,7 @@  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
+#define QEMU_VM_FILE_VERSION         0x00000004
 
 bool qemu_savevm_state_blocked(Monitor *mon)
 {