[Qemu-devel] [PATCH v2 0/3] migration: Fixups for VMDESC submission

[Qemu-devel] [PATCH v2 0/3] migration: Fixups for VMDESC submission

[Alexander Graf]

After the vmdesc self-describing JSON blob landed upstream, some people
noticed their self-written scripts around migration to fail.

This patch set aims at making the transition to the new VMDESC containing
migration format as smooth as possible for everyone involved.

Alexander Graf (3):
  migration: Read JSON VM description on incoming migration
  migration: Allow to suppress vmdesc submission
  pc: Disable vmdesc submission for old machines

 hw/core/machine.c   | 20 ++++++++++++++++++++
 hw/i386/pc_piix.c   |  1 +
 hw/i386/pc_q35.c    |  1 +
 include/hw/boards.h |  1 +
 qemu-options.hx     |  3 ++-
 savevm.c            | 36 ++++++++++++++++++++++++++++++++----
 6 files changed, 57 insertions(+), 5 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH v2 1/3] migration: Read JSON VM description on incoming migration

[Alexander Graf]

One of the really nice things about the VM description format is that it goes
over the wire when live migration is happening. Unfortunately QEMU today closes
any socket once it sees VM_EOF coming, so we never give the VMDESC the chance to
actually land on the wire.

This patch makes QEMU read the description as well. This way we ensure that
anything wire tapping us in between will get the chance to also interpret the
stream.

Along the way we also fix virt tests that assume that number_bytes_sent on the
sender side is equal to number_bytes_read which was true before the VMDESC
patches and is true again with this patch.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - read in small chunks
---
 savevm.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/savevm.c b/savevm.c
index 8040766..1b78654 100644
--- a/savevm.c
+++ b/savevm.c
@@ -929,6 +929,7 @@ int qemu_loadvm_state(QEMUFile *f)
     uint8_t section_type;
     unsigned int v;
     int ret;
+    int file_error_after_eof = -1;
 
     if (qemu_savevm_state_blocked(&local_err)) {
         error_report("%s", error_get_pretty(local_err));
@@ -1034,6 +1035,24 @@ int qemu_loadvm_state(QEMUFile *f)
         }
     }
 
+    file_error_after_eof = qemu_file_get_error(f);
+
+    /*
+     * Try to read in the VMDESC section as well, so that dumping tools that
+     * intercept our migration stream have the chance to see it.
+     */
+    if (qemu_get_byte(f) == QEMU_VM_VMDESCRIPTION) {
+        uint32_t size = qemu_get_be32(f);
+        uint8_t *buf = g_malloc(0x1000);
+
+        while (size > 0) {
+            uint32_t read_chunk = MIN(size, 0x1000);
+            qemu_get_buffer(f, buf, read_chunk);
+            size -= read_chunk;
+        }
+        g_free(buf);
+    }
+
     cpu_synchronize_all_post_init();
 
     ret = 0;
@@ -1045,7 +1064,8 @@ out:
     }
 
     if (ret == 0) {
-        ret = qemu_file_get_error(f);
+        /* We may not have a VMDESC section, so ignore relative errors */
+        ret = file_error_after_eof;
     }
 
     return ret;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v2 1/3] migration: Read JSON VM description on incoming migration

[Dr. David Alan Gilbert]

* Alexander Graf (agraf@suse.de) wrote:
> One of the really nice things about the VM description format is that it goes
> over the wire when live migration is happening. Unfortunately QEMU today closes
> any socket once it sees VM_EOF coming, so we never give the VMDESC the chance to
> actually land on the wire.
> 
> This patch makes QEMU read the description as well. This way we ensure that
> anything wire tapping us in between will get the chance to also interpret the
> stream.
> 
> Along the way we also fix virt tests that assume that number_bytes_sent on the
> sender side is equal to number_bytes_read which was true before the VMDESC
> patches and is true again with this patch.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

and I've tested the set of 3 and it works, so:

Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

> 
> ---
> 
> v1 -> v2:
> 
>   - read in small chunks
> ---
>  savevm.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/savevm.c b/savevm.c
> index 8040766..1b78654 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -929,6 +929,7 @@ int qemu_loadvm_state(QEMUFile *f)
>      uint8_t section_type;
>      unsigned int v;
>      int ret;
> +    int file_error_after_eof = -1;
>  
>      if (qemu_savevm_state_blocked(&local_err)) {
>          error_report("%s", error_get_pretty(local_err));
> @@ -1034,6 +1035,24 @@ int qemu_loadvm_state(QEMUFile *f)
>          }
>      }
>  
> +    file_error_after_eof = qemu_file_get_error(f);
> +
> +    /*
> +     * Try to read in the VMDESC section as well, so that dumping tools that
> +     * intercept our migration stream have the chance to see it.
> +     */
> +    if (qemu_get_byte(f) == QEMU_VM_VMDESCRIPTION) {
> +        uint32_t size = qemu_get_be32(f);
> +        uint8_t *buf = g_malloc(0x1000);
> +
> +        while (size > 0) {
> +            uint32_t read_chunk = MIN(size, 0x1000);
> +            qemu_get_buffer(f, buf, read_chunk);
> +            size -= read_chunk;
> +        }
> +        g_free(buf);
> +    }
> +
>      cpu_synchronize_all_post_init();
>  
>      ret = 0;
> @@ -1045,7 +1064,8 @@ out:
>      }
>  
>      if (ret == 0) {
> -        ret = qemu_file_get_error(f);
> +        /* We may not have a VMDESC section, so ignore relative errors */
> +        ret = file_error_after_eof;
>      }
>  
>      return ret;
> -- 
> 1.7.12.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v2 2/3] migration: Allow to suppress vmdesc submission

[Alexander Graf]

We now always send a JSON blob describing the migration file format as part
of the migration stream. However, some tools built around QEMU have proven
to stumble over this.

This patch gives the user the chance to disable said self-describing part of
the migration stream. To disable vmdesc submission, just add

  -machine suppress-vmdesc=on

to your QEMU command line.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/core/machine.c   | 20 ++++++++++++++++++++
 include/hw/boards.h |  1 +
 qemu-options.hx     |  3 ++-
 savevm.c            | 14 +++++++++++---
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index fbd91be..53409b2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -260,6 +260,20 @@ static void machine_set_iommu(Object *obj, bool value, Error **errp)
     ms->iommu = value;
 }
 
+static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    ms->suppress_vmdesc = value;
+}
+
+static bool machine_get_suppress_vmdesc(Object *obj, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    return ms->suppress_vmdesc;
+}
+
 static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque)
 {
     error_report("Option '-device %s' cannot be handled by this machine",
@@ -378,6 +392,12 @@ static void machine_initfn(Object *obj)
     object_property_set_description(obj, "iommu",
                                     "Set on/off to enable/disable Intel IOMMU (VT-d)",
                                     NULL);
+    object_property_add_bool(obj, "suppress-vmdesc",
+                             machine_get_suppress_vmdesc,
+                             machine_set_suppress_vmdesc, NULL);
+    object_property_set_description(obj, "suppress-vmdesc",
+                                    "Set on to disable self-describing migration",
+                                    NULL);
 
     /* Register notifier when init is done for sysbus sanity checks */
     ms->sysbus_notifier.notify = machine_init_notify;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3ddc449..bf87a29 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -135,6 +135,7 @@ struct MachineState {
     bool usb;
     char *firmware;
     bool iommu;
+    bool suppress_vmdesc;
 
     ram_addr_t ram_size;
     ram_addr_t maxram_size;
diff --git a/qemu-options.hx b/qemu-options.hx
index 85ca3ad..86be4f4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -37,7 +37,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                kvm_shadow_mem=size of KVM shadow MMU\n"
     "                dump-guest-core=on|off include guest memory in a core dump (default=on)\n"
     "                mem-merge=on|off controls memory merge support (default: on)\n"
-    "                iommu=on|off controls emulated Intel IOMMU (VT-d) support (default=off)\n",
+    "                iommu=on|off controls emulated Intel IOMMU (VT-d) support (default=off)\n"
+    "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
diff --git a/savevm.c b/savevm.c
index 1b78654..8a0ecf2 100644
--- a/savevm.c
+++ b/savevm.c
@@ -710,6 +710,12 @@ int qemu_savevm_state_iterate(QEMUFile *f)
     return ret;
 }
 
+static bool should_send_vmdesc(void)
+{
+    MachineState *machine = MACHINE(qdev_get_machine());
+    return !machine->suppress_vmdesc;
+}
+
 void qemu_savevm_state_complete(QEMUFile *f)
 {
     QJSON *vmdesc;
@@ -782,9 +788,11 @@ void qemu_savevm_state_complete(QEMUFile *f)
     qjson_finish(vmdesc);
     vmdesc_len = strlen(qjson_get_str(vmdesc));
 
-    qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
-    qemu_put_be32(f, vmdesc_len);
-    qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
+    if (should_send_vmdesc()) {
+        qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
+        qemu_put_be32(f, vmdesc_len);
+        qemu_put_buffer(f, (uint8_t *)qjson_get_str(vmdesc), vmdesc_len);
+    }
     object_unref(OBJECT(vmdesc));
 
     qemu_fflush(f);
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 3/3] pc: Disable vmdesc submission for old machines

[Alexander Graf]

Older PC machine types might by accident be backwards live migration compatible,
but with the new vmdesc self-describing blob in our live migration stream we
would break that compatibility.

Also users wouldn't expect massive behaviorial differences when updating to a
new version of QEMU while retaining their old machine type, especially not
potential breakage in tooling around live migration.

So disable vmdesc submission for old PC machine types.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 38b42b0..35bc377 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -332,6 +332,7 @@ static void pc_compat_2_2(MachineState *machine)
                                 CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
     x86_cpu_compat_set_features("Broadwell", FEAT_7_0_EBX,
                                 CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
+    machine->suppress_vmdesc = true;
 }
 
 static void pc_compat_2_1(MachineState *machine)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 63027ee..1305eb0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -311,6 +311,7 @@ static void pc_compat_2_2(MachineState *machine)
                                 CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
     x86_cpu_compat_set_features("Broadwell", FEAT_7_0_EBX,
                                 CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
+    machine->suppress_vmdesc = true;
 }
 
 static void pc_compat_2_1(MachineState *machine)
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v2 3/3] pc: Disable vmdesc submission for old machines

[Dr. David Alan Gilbert]

* Alexander Graf (agraf@suse.de) wrote:
> Older PC machine types might by accident be backwards live migration compatible,
> but with the new vmdesc self-describing blob in our live migration stream we
> would break that compatibility.
> 
> Also users wouldn't expect massive behaviorial differences when updating to a
> new version of QEMU while retaining their old machine type, especially not
> potential breakage in tooling around live migration.
> 
> So disable vmdesc submission for old PC machine types.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

(I'm leaving 2/3 for someone more familiar with -machine)

Dave

> ---
>  hw/i386/pc_piix.c | 1 +
>  hw/i386/pc_q35.c  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 38b42b0..35bc377 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -332,6 +332,7 @@ static void pc_compat_2_2(MachineState *machine)
>                                  CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
>      x86_cpu_compat_set_features("Broadwell", FEAT_7_0_EBX,
>                                  CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
> +    machine->suppress_vmdesc = true;
>  }
>  
>  static void pc_compat_2_1(MachineState *machine)
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 63027ee..1305eb0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -311,6 +311,7 @@ static void pc_compat_2_2(MachineState *machine)
>                                  CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
>      x86_cpu_compat_set_features("Broadwell", FEAT_7_0_EBX,
>                                  CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_RTM, 0);
> +    machine->suppress_vmdesc = true;
>  }
>  
>  static void pc_compat_2_1(MachineState *machine)
> -- 
> 1.7.12.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 0/3] migration: Fixups for VMDESC submission

[Lucas Meneghel Rodrigues]

Just for the record, I've tested it using the virt-test reproducer and 
indeed the problem is fixed with this patchset. I'm sorry for the delay!

Thanks :)

On Mon, Feb 23, 2015 at 9:56 AM, Alexander Graf <agraf@suse.de> wrote:
> After the vmdesc self-describing JSON blob landed upstream, some 
> people
> noticed their self-written scripts around migration to fail.
> 
> This patch set aims at making the transition to the new VMDESC 
> containing
> migration format as smooth as possible for everyone involved.
> 
> Alexander Graf (3):
>   migration: Read JSON VM description on incoming migration
>   migration: Allow to suppress vmdesc submission
>   pc: Disable vmdesc submission for old machines
> 
>  hw/core/machine.c   | 20 ++++++++++++++++++++
>  hw/i386/pc_piix.c   |  1 +
>  hw/i386/pc_q35.c    |  1 +
>  include/hw/boards.h |  1 +
>  qemu-options.hx     |  3 ++-
>  savevm.c            | 36 ++++++++++++++++++++++++++++++++----
>  6 files changed, 57 insertions(+), 5 deletions(-)
> 
> --
> 1.7.12.4
>