[Qemu-devel] [PATCH v3 0/4] Fix subsection ambiguity in the migration format

[Qemu-devel] [PATCH v3 0/4] Fix subsection ambiguity in the 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 with half-loaded state.

There is no way out of this except defining an incompatible migration
protocol; a sentinel at the end of embedded structs is enough to remove
the ambiguity, but is already incompatible of course.  So this can be
restricted to new machine models, and this is what the patch series does.
(Only patch 3 is specific to the short-term solution, everything else
is entirely generic).

v2->v3
        resolve conflicts, move savevm.o to libhw

v1->v2
        coding style

Paolo Bonzini (4):
  add support for machine models to specify their migration format
  add pc-0.14/pc-0.15 machine
  savevm: define new unambiguous migration format
  Partially revert "savevm: fix corruption in vmstate_subsection_load()."

 Makefile.objs |    4 ++--
 hw/boards.h   |    4 ++++
 hw/pc_piix.c  |   16 +++++++++++++++-
 savevm.c      |   44 ++++++++++++++++++++++++++++++--------------
 4 files changed, 51 insertions(+), 17 deletions(-)

-- 
1.7.6

[Qemu-devel] [PATCH v3 1/4] add support for machine models to specify their migration format

[Paolo Bonzini]

We need to provide a new migration format, and not break migration
in old machine models.  So add a migration_format field to QEMUMachine.

This adds a dependency on ram_addr_t to savevm.c (through hw/boards.h),
whose type definition depends on the word size.  So move savevm.o to
libhw.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs |    4 ++--
 hw/boards.h   |    1 +
 savevm.c      |   16 ++++++++++++++--
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 89ca361..039d1fd 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -108,7 +108,7 @@ common-obj-$(CONFIG_SD) += sd.o
 common-obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o usb-bt.o
 common-obj-y += bt-hci-csr.o
 common-obj-y += buffered_file.o migration.o migration-tcp.o
-common-obj-y += qemu-char.o savevm.o #aio.o
+common-obj-y += qemu-char.o #aio.o
 common-obj-y += msmouse.o ps2.o
 common-obj-y += qdev.o qdev-properties.o
 common-obj-y += block-migration.o iohandler.o
@@ -181,7 +181,7 @@ user-obj-y += cutils.o cache-utils.o
 # libhw
 
 hw-obj-y =
-hw-obj-y += vl.o loader.o
+hw-obj-y += vl.o loader.o savevm.o
 hw-obj-$(CONFIG_VIRTIO) += virtio-console.o
 hw-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 hw-obj-y += fw_cfg.o
diff --git a/hw/boards.h b/hw/boards.h
index 716fd7b..560dbaf 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -19,6 +19,7 @@ typedef struct QEMUMachine {
     QEMUMachineInitFunc *init;
     int use_scsi;
     int max_cpus;
+    unsigned migration_format;
     unsigned int no_serial:1,
         no_parallel:1,
         use_virtcon:1,
diff --git a/savevm.c b/savevm.c
index 9f4a3af..87f2b71 100644
--- a/savevm.c
+++ b/savevm.c
@@ -72,6 +72,7 @@
 #include "qemu-common.h"
 #include "hw/hw.h"
 #include "hw/qdev.h"
+#include "hw/boards.h"
 #include "net.h"
 #include "monitor.h"
 #include "sysemu.h"
@@ -1215,6 +1216,15 @@ void register_device_unmigratable(DeviceState *dev, const char *idstr,
     }
 }
 
+static inline int qemu_current_migration_format(void)
+{
+    if (current_machine->migration_format)
+        return current_machine->migration_format;
+
+    /* No format specified, default to the latest.  */
+    return QEMU_VM_FILE_VERSION;
+}
+
 int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
                                    const VMStateDescription *vmsd,
                                    void *opaque, int alias_id,
@@ -1484,7 +1494,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
     }
     
     qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
-    qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+    qemu_put_be32(f, qemu_current_migration_format());
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         int len;
@@ -1761,8 +1771,10 @@ int qemu_loadvm_state(QEMUFile *f)
         fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n");
         return -ENOTSUP;
     }
-    if (v != QEMU_VM_FILE_VERSION)
+    if (v != qemu_current_migration_format()) {
+        fprintf(stderr, "Mismatching SaveVM format v%d\n", v);
         return -ENOTSUP;
+    }
 
     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
         uint32_t instance_id, version_id, section_id;
-- 
1.7.6

[Qemu-devel] [PATCH v3 2/4] add pc-0.14/pc-0.15 machine

[Paolo Bonzini]

The new pc-1.0 machine will have a different migration format, so
define the compatibility one right now.  Alias it to pc-0.14, since
they are the same.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pc_piix.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index c0a2abe..88a5f3b 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -270,7 +270,7 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
 #endif
 
 static QEMUMachine pc_machine = {
-    .name = "pc-0.14",
+    .name = "pc-1.0",
     .alias = "pc",
     .desc = "Standard PC",
     .init = pc_init_pci,
@@ -278,6 +278,14 @@ static QEMUMachine pc_machine = {
     .is_default = 1,
 };
 
+static QEMUMachine pc_machine_v0_15 = {
+    .name = "pc-0.15",
+    .alias = "pc-0.14",
+    .desc = "Standard PC",
+    .init = pc_init_pci,
+    .max_cpus = 255,
+};
+
 static QEMUMachine pc_machine_v0_13 = {
     .name = "pc-0.13",
     .desc = "Standard PC",
@@ -494,6 +502,7 @@ static QEMUMachine xenfv_machine = {
 static void pc_machine_init(void)
 {
     qemu_register_machine(&pc_machine);
+    qemu_register_machine(&pc_machine_v0_15);
     qemu_register_machine(&pc_machine_v0_13);
     qemu_register_machine(&pc_machine_v0_12);
     qemu_register_machine(&pc_machine_v0_11);
-- 
1.7.6

[Qemu-devel] [PATCH v3 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 <pbonzini@redhat.com>
---
 hw/boards.h  |    3 +++
 hw/pc_piix.c |    5 +++++
 savevm.c     |   24 ++++++++++++++++--------
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/hw/boards.h b/hw/boards.h
index 560dbaf..f20d58e 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -5,6 +5,9 @@
 
 #include "qdev.h"
 
+#define QEMU_VM_FILE_VERSION_0_15    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 88a5f3b..8af1f6f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -284,6 +284,7 @@ static QEMUMachine pc_machine_v0_15 = {
     .desc = "Standard PC",
     .init = pc_init_pci,
     .max_cpus = 255,
+    .migration_format = QEMU_VM_FILE_VERSION_0_15,
 };
 
 static QEMUMachine pc_machine_v0_13 = {
@@ -291,6 +292,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_15,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-9p-pci",
@@ -330,6 +332,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_15,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-serial-pci",
@@ -373,6 +376,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_15,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-blk-pci",
@@ -424,6 +428,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_15,
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "virtio-blk-pci",
diff --git a/savevm.c b/savevm.c
index 87f2b71..a362ad7 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;
@@ -1358,6 +1366,11 @@ 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 (qemu_current_migration_format() >= 4) {
+                        if (qemu_get_byte(f) != QEMU_VM_SUBSECTIONS_END) {
+                            return -EINVAL;
+                        }
+                    }
                 } else {
                     ret = field->info->get(f, addr, size);
 
@@ -1429,6 +1442,9 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                 }
                 if (field->flags & VMS_STRUCT) {
                     vmstate_save_state(f, field->vmsd, addr);
+                    if (qemu_current_migration_format() >= 4) {
+                        qemu_put_byte(f, QEMU_VM_SUBSECTIONS_END);
+                    }
                 } else {
                     field->info->put(f, addr, size);
                 }
@@ -1458,14 +1474,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

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

[Anthony Liguori]

On 08/12/2011 04:59 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>

Because of the change we made for 0.15, this is no longer strictly 
needed.  It only matters if we add a subsection to a structure, right?

Regards,

Anthony Liguori

> ---
>   hw/boards.h  |    3 +++
>   hw/pc_piix.c |    5 +++++
>   savevm.c     |   24 ++++++++++++++++--------
>   3 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/hw/boards.h b/hw/boards.h
> index 560dbaf..f20d58e 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -5,6 +5,9 @@
>
>   #include "qdev.h"
>
> +#define QEMU_VM_FILE_VERSION_0_15    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 88a5f3b..8af1f6f 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -284,6 +284,7 @@ static QEMUMachine pc_machine_v0_15 = {
>       .desc = "Standard PC",
>       .init = pc_init_pci,
>       .max_cpus = 255,
> +    .migration_format = QEMU_VM_FILE_VERSION_0_15,
>   };
>
>   static QEMUMachine pc_machine_v0_13 = {
> @@ -291,6 +292,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_15,
>       .compat_props = (GlobalProperty[]) {
>           {
>               .driver   = "virtio-9p-pci",
> @@ -330,6 +332,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_15,
>       .compat_props = (GlobalProperty[]) {
>           {
>               .driver   = "virtio-serial-pci",
> @@ -373,6 +376,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_15,
>       .compat_props = (GlobalProperty[]) {
>           {
>               .driver   = "virtio-blk-pci",
> @@ -424,6 +428,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_15,
>       .compat_props = (GlobalProperty[]) {
>           {
>               .driver   = "virtio-blk-pci",
> diff --git a/savevm.c b/savevm.c
> index 87f2b71..a362ad7 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;
> @@ -1358,6 +1366,11 @@ 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 (qemu_current_migration_format()>= 4) {
> +                        if (qemu_get_byte(f) != QEMU_VM_SUBSECTIONS_END) {
> +                            return -EINVAL;
> +                        }
> +                    }
>                   } else {
>                       ret = field->info->get(f, addr, size);
>
> @@ -1429,6 +1442,9 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
>                   }
>                   if (field->flags&  VMS_STRUCT) {
>                       vmstate_save_state(f, field->vmsd, addr);
> +                    if (qemu_current_migration_format()>= 4) {
> +                        qemu_put_byte(f, QEMU_VM_SUBSECTIONS_END);
> +                    }
>                   } else {
>                       field->info->put(f, addr, size);
>                   }
> @@ -1458,14 +1474,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)
>   {

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

[Anthony Liguori]

On 08/12/2011 10:11 AM, Anthony Liguori wrote:
> On 08/12/2011 04:59 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>
>
> Because of the change we made for 0.15, this is no longer strictly
> needed. It only matters if we add a subsection to a structure, right?

I'm definitely not saying we shouldn't do this BTW.  But trying to 
understand whether we can position this as a strictly optional feature 
(iow, a capability) or whether this requires a hard break in the 
migration protocol.

Regards,

Anthony Liguori

>
> Regards,
>
> Anthony Liguori
>
>> ---
>> hw/boards.h | 3 +++
>> hw/pc_piix.c | 5 +++++
>> savevm.c | 24 ++++++++++++++++--------
>> 3 files changed, 24 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/boards.h b/hw/boards.h
>> index 560dbaf..f20d58e 100644
>> --- a/hw/boards.h
>> +++ b/hw/boards.h
>> @@ -5,6 +5,9 @@
>>
>> #include "qdev.h"
>>
>> +#define QEMU_VM_FILE_VERSION_0_15 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 88a5f3b..8af1f6f 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -284,6 +284,7 @@ static QEMUMachine pc_machine_v0_15 = {
>> .desc = "Standard PC",
>> .init = pc_init_pci,
>> .max_cpus = 255,
>> + .migration_format = QEMU_VM_FILE_VERSION_0_15,
>> };
>>
>> static QEMUMachine pc_machine_v0_13 = {
>> @@ -291,6 +292,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_15,
>> .compat_props = (GlobalProperty[]) {
>> {
>> .driver = "virtio-9p-pci",
>> @@ -330,6 +332,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_15,
>> .compat_props = (GlobalProperty[]) {
>> {
>> .driver = "virtio-serial-pci",
>> @@ -373,6 +376,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_15,
>> .compat_props = (GlobalProperty[]) {
>> {
>> .driver = "virtio-blk-pci",
>> @@ -424,6 +428,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_15,
>> .compat_props = (GlobalProperty[]) {
>> {
>> .driver = "virtio-blk-pci",
>> diff --git a/savevm.c b/savevm.c
>> index 87f2b71..a362ad7 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;
>> @@ -1358,6 +1366,11 @@ 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 (qemu_current_migration_format()>= 4) {
>> + if (qemu_get_byte(f) != QEMU_VM_SUBSECTIONS_END) {
>> + return -EINVAL;
>> + }
>> + }
>> } else {
>> ret = field->info->get(f, addr, size);
>>
>> @@ -1429,6 +1442,9 @@ void vmstate_save_state(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> }
>> if (field->flags& VMS_STRUCT) {
>> vmstate_save_state(f, field->vmsd, addr);
>> + if (qemu_current_migration_format()>= 4) {
>> + qemu_put_byte(f, QEMU_VM_SUBSECTIONS_END);
>> + }
>> } else {
>> field->info->put(f, addr, size);
>> }
>> @@ -1458,14 +1474,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)
>> {
>
>

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

[Paolo Bonzini]

On 08/12/2011 05:16 PM, Anthony Liguori wrote:
>>
>> Because of the change we made for 0.15, this is no longer strictly
>> needed. It only matters if we add a subsection to a structure, right?

Yes.  We can also check the current migration format in the "needed" 
function for such subsections, if necessary.  This is what I plan to do 
for the floppy subsection, skipping it for old machine types that do not 
use the new migration format.

> I'm definitely not saying we shouldn't do this BTW.  But trying to
> understand whether we can position this as a strictly optional feature
> (iow, a capability) or whether this requires a hard break in the
> migration protocol.

This what? :)  Adding a subsection to a structure, or using the new 
migration format?  If you mean using the new format only when you have 
subsections in a structure, then you'll need quite a bit of new code to 
try all VMState handlers and see if they'll need a subsection.  I don't 
think it's worth it, also because we all hope this is a temporary 
solution until we have a serious format (e.g. ASN-1).

Paolo

[Qemu-devel] [PATCH v3 4/4] Partially revert "savevm: fix corruption in vmstate_subsection_load()."

[Paolo Bonzini]

This reverts the additional check in commit eb60260d (but not the
assertions).

The new format does not require the check, and with the old format
it traded one kind of bogus failure for a different kind of silent
failure.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 savevm.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/savevm.c b/savevm.c
index a362ad7..2164edc 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1699,10 +1699,6 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
 {
     const VMStateSubsection *sub = vmsd->subsections;
 
-    if (!sub || !sub->needed) {
-        return 0;
-    }
-
     while (qemu_peek_byte(f) == QEMU_VM_SUBSECTION) {
         char idstr[256];
         int ret;
-- 
1.7.6