[PATCH v1] hw/i386: disable smbus migration for xenpv

Olaf Hering posted 1 patch 1 week ago
Test docker-mingw@fedora passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200113174521.3336-1-olaf@aepfle.de
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <rth@twiddle.net>, "Michael S. Tsirkin" <mst@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>
hw/i386/pc_piix.c | 1 +
1 file changed, 1 insertion(+)

[PATCH v1] hw/i386: disable smbus migration for xenpv

Posted by Olaf Hering 1 week ago
With commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea a new member
smbus_no_migration_support was added, and enabled in two places.
With commit 4ab2f2a8aabfea95cc53c64e13b3f67960b27fdf the vmstate_acpi
got new elements, which are conditionally filled. As a result, an
incoming migration expected smbus related data unless smbus migration
was disabled for a given MachineClass.

Since commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea forgot to handle
xenpv, live migration to receiving hosts using qemu-4.0 and later is broken.
Therefore this patch must be applied to stable-4.x as well.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 hw/i386/pc_piix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fa12203079..e19716d0d3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -952,6 +952,7 @@ static void xenfv_machine_options(MachineClass *m)
     m->desc = "Xen Fully-virtualized PC";
     m->max_cpus = HVM_MAX_VCPUS;
     m->default_machine_opts = "accel=xen";
+    m->smbus_no_migration_support = true;
 }
 
 DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,

Re: [PATCH v1] hw/i386: disable smbus migration for xenpv

Posted by Michael S. Tsirkin 1 week ago
On Mon, Jan 13, 2020 at 06:45:21PM +0100, Olaf Hering wrote:
> With commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea a new member
> smbus_no_migration_support was added, and enabled in two places.
> With commit 4ab2f2a8aabfea95cc53c64e13b3f67960b27fdf the vmstate_acpi
> got new elements, which are conditionally filled. As a result, an
> incoming migration expected smbus related data unless smbus migration
> was disabled for a given MachineClass.
> 
> Since commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea forgot to handle
> xenpv, live migration to receiving hosts using qemu-4.0 and later is broken.
> Therefore this patch must be applied to stable-4.x as well.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Thanks!

I think you need to copy Xen maintainers to ack this.
Suggest reposting with a fixed commit message and
cc appropriate maintainers.


> ---
>  hw/i386/pc_piix.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index fa12203079..e19716d0d3 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -952,6 +952,7 @@ static void xenfv_machine_options(MachineClass *m)
>      m->desc = "Xen Fully-virtualized PC";
>      m->max_cpus = HVM_MAX_VCPUS;
>      m->default_machine_opts = "accel=xen";
> +    m->smbus_no_migration_support = true;
>  }
>  
>  DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,


Re: [PATCH v1] hw/i386: disable smbus migration for xenfv

Posted by Olaf Hering 1 week ago
Am Mon, 13 Jan 2020 18:45:21 +0100
schrieb Olaf Hering <olaf@aepfle.de>:

> commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea forgot to handle xenpv

Actually it is xenFv, but you get the idea.

Olaf

[PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Olaf Hering 1 week ago
With commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea a new member
smbus_no_migration_support was added, and enabled in two places.
With commit 4ab2f2a8aabfea95cc53c64e13b3f67960b27fdf the vmstate_acpi
got new elements, which are conditionally filled. As a result, an
incoming migration expected smbus related data unless smbus migration
was disabled for a given MachineClass.

Since commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea forgot to handle
xenfv, live migration to receiving hosts using qemu-4.0 and later is broken.

Therefore this patch must be applied to stable-4.x as well.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fa12203079..e19716d0d3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -952,6 +952,7 @@ static void xenfv_machine_options(MachineClass *m)
     m->desc = "Xen Fully-virtualized PC";
     m->max_cpus = HVM_MAX_VCPUS;
     m->default_machine_opts = "accel=xen";
+    m->smbus_no_migration_support = true;
 }
 
 DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,

Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Paolo Bonzini 1 week ago
On 16/01/20 19:03, Olaf Hering wrote:
> With commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea a new member
> smbus_no_migration_support was added, and enabled in two places.
> With commit 4ab2f2a8aabfea95cc53c64e13b3f67960b27fdf the vmstate_acpi
> got new elements, which are conditionally filled. As a result, an
> incoming migration expected smbus related data unless smbus migration
> was disabled for a given MachineClass.
> 
> Since commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea forgot to handle
> xenfv, live migration to receiving hosts using qemu-4.0 and later is broken.
> 
> Therefore this patch must be applied to stable-4.x as well.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index fa12203079..e19716d0d3 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -952,6 +952,7 @@ static void xenfv_machine_options(MachineClass *m)
>      m->desc = "Xen Fully-virtualized PC";
>      m->max_cpus = HVM_MAX_VCPUS;
>      m->default_machine_opts = "accel=xen";
> +    m->smbus_no_migration_support = true;
>  }
>  
>  DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,
> 

This patch is wrong; xenfv does not support cross-version migration
compatibility.  Even if the migration stream does not change, the
hardware exposed to the guest will.

My understanding is that Xen is able to use "-M
pc-i440fx-VERSION,accel=xen".  The presence of the version in the
machine type guarantees that the migration stream is compatible and that
the hardware exposed to the guest is the same on the source and destination.

Is this incorrect?

Paolo


Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Olaf Hering 1 week ago
Am Thu, 16 Jan 2020 19:26:39 +0100
schrieb Paolo Bonzini <pbonzini@redhat.com>:

> My understanding is that Xen is able to use "-M
> pc-i440fx-VERSION,accel=xen".  The presence of the version in the
> machine type guarantees that the migration stream is compatible and that
> the hardware exposed to the guest is the same on the source and destination.

It seems 'xenfv' is not a 'pc-i440fx-X.X', even with accel=xen.
The "xen-platform" will not be created and as a result no PV driver can be used.

I do not know what a 'xenfv' is supposed to be, and how it is supposed to
behave compatible for all existing and future guests.


Olaf

Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Paolo Bonzini 1 week ago
On 17/01/20 10:22, Olaf Hering wrote:
> Am Thu, 16 Jan 2020 19:26:39 +0100
> schrieb Paolo Bonzini <pbonzini@redhat.com>:
> 
>> My understanding is that Xen is able to use "-M
>> pc-i440fx-VERSION,accel=xen".  The presence of the version in the
>> machine type guarantees that the migration stream is compatible and that
>> the hardware exposed to the guest is the same on the source and destination.
> 
> It seems 'xenfv' is not a 'pc-i440fx-X.X', even with accel=xen.
> The "xen-platform" will not be created and as a result no PV driver can be used.

Just add "-device xen-platform" to the command line.

> I do not know what a 'xenfv' is supposed to be, and how it is supposed to
> behave compatible for all existing and future guests.

It's nothing more than a synonym for "-machine pc -device xen-platform
-accel xen" (plus some magic to support igd passthrough, which we could
and should move to the pc machine type as well).  It doesn't even try to
be compatible for all existing and future guests.

Paolo

Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Olaf Hering 1 week ago
Am Fri, 17 Jan 2020 11:27:59 +0100
schrieb Paolo Bonzini <pbonzini@redhat.com>:

> It doesn't even try to be compatible for all existing and future guests.

This looks like the underlying bug.

What would future domUs lose if 'xenfv' would be locked to 'pc-i440fx-3.0'?


Olaf

Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Paul Durrant 5 days ago
On Fri, 17 Jan 2020 at 13:06, Olaf Hering <olaf@aepfle.de> wrote:
>
> Am Fri, 17 Jan 2020 11:27:59 +0100
> schrieb Paolo Bonzini <pbonzini@redhat.com>:
>
> > It doesn't even try to be compatible for all existing and future guests.
>
> This looks like the underlying bug.
>
> What would future domUs lose if 'xenfv' would be locked to 'pc-i440fx-3.0'?
>

I guess eventually that pc type would be removed and then we'd have a
compat issue. Ideally I think libxl should simply not use xenfv and
then it can be deprecated and removed, and then such issues can be
dealt with directly in xl/libxl.

  Paul

>
> Olaf

Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Olaf Hering 1 week ago
Am Thu, 16 Jan 2020 19:26:39 +0100
schrieb Paolo Bonzini <pbonzini@redhat.com>:

> xenfv does not support cross-version migration compatibility.

Wait, what does that mean?
So far live migration of a running domU must be possible from Xen N to Xen N+1.
It would be more than unexpected if the target host running "Xen N+1" must have the very same qemu version as "Xen N".

Olaf

Re: [PATCH v2] hw/i386: disable smbus migration for xenfv

Posted by Paolo Bonzini 1 week ago
Il gio 16 gen 2020, 19:33 Olaf Hering <olaf@aepfle.de> ha scritto:

> Am Thu, 16 Jan 2020 19:26:39 +0100
> schrieb Paolo Bonzini <pbonzini@redhat.com>:
>
> > xenfv does not support cross-version migration compatibility.
>
> Wait, what does that mean?
> So far live migration of a running domU must be possible from Xen N to Xen
> N+1.
>

The  Xen N+1 must know that the source is running Xen N, and use a machine
type corresponding to the QEMU version that was shipped with Xen N. For new
virtual machines Xen must also use pc-i440fx-M.N and not just PC.

This is not new, versioned machine types were introduced in QEMU 0.10 or
something like that for exactly this purpose.

Paolo

It would be more than unexpected if the target host running "Xen N+1" must
> have the very same qemu version as "Xen N".
>
> Olaf
>