qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] virtio-pci: Fix cross-version migration with older machines
@ 2016-12-14 16:19 Maxime Coquelin
  2016-12-14 16:24 ` Cornelia Huck
  0 siblings, 1 reply; 3+ messages in thread
From: Maxime Coquelin @ 2016-12-14 16:19 UTC (permalink / raw)
  To: mdroth, stefanha, qemu-devel, mst, cornelia.huck, marcel
  Cc: Maxime Coquelin, Dr . David Alan Gilbert

This patch fixes a cross-version migration regression introduced
by commit d1b4259f ("virtio-bus: Plug devices after features are
negotiated").

The problem is encountered when host's vhost backend does not support
VIRTIO_F_VERSION_1, and migration is initiated from a v2.7 or prior
QEMU version with virtio-pci modern capabilities enabled to a v2.8
QEMU version.

In this case, modern capabilities get exposed to the guest by the source,
whereas the target will detect version 1 is not supported so will only
expose legacy capabilities.

The problem is fixed by introducing a new "x-modern-broken" property,
which is set in v2.7 and prior compatibility modes. Doing this, v2.7
machine keeps its broken behaviour (enabling modern while version is
not supported), and newer machines will behave correctly.

Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---

Thanks for the reviews and tests.
This version changes the naming as proposed by Michael T.
and Cornelia, and fixies commit message.

 hw/virtio/virtio-pci.c | 5 ++++-
 hw/virtio/virtio-pci.h | 1 +
 include/hw/compat.h    | 4 ++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 521ba0b..21c2b9d 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1580,7 +1580,8 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
      * Virtio capabilities present without
      * VIRTIO_F_VERSION_1 confuses guests
      */
-    if (!virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {
+    if (!proxy->ignore_backend_features &&
+            !virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {
         virtio_pci_disable_modern(proxy);
 
         if (!legacy) {
@@ -1852,6 +1853,8 @@ static Property virtio_pci_properties[] = {
                     VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
     DEFINE_PROP_BIT("page-per-vq", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT, false),
+    DEFINE_PROP_BOOL("x-ignore-backend-features", VirtIOPCIProxy,
+                     ignore_backend_features, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index b2a996f..5e07886 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -153,6 +153,7 @@ struct VirtIOPCIProxy {
     int config_cap;
     uint32_t flags;
     bool disable_modern;
+    bool ignore_backend_features;
     OnOffAuto disable_legacy;
     uint32_t class_code;
     uint32_t nvectors;
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 0f06e11..8dfc7a3 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -18,6 +18,10 @@
         .driver   = "intel-iommu",\
         .property = "x-buggy-eim",\
         .value    = "true",\
+    },{\
+        .driver   = "virtio-pci",\
+        .property = "x-ignore-backend-features",\
+        .value    = "on",\
     },
 
 #define HW_COMPAT_2_6 \
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] virtio-pci: Fix cross-version migration with older machines
  2016-12-14 16:19 [Qemu-devel] [PATCH v2] virtio-pci: Fix cross-version migration with older machines Maxime Coquelin
@ 2016-12-14 16:24 ` Cornelia Huck
  2016-12-14 16:27   ` Maxime Coquelin
  0 siblings, 1 reply; 3+ messages in thread
From: Cornelia Huck @ 2016-12-14 16:24 UTC (permalink / raw)
  To: Maxime Coquelin
  Cc: mdroth, stefanha, qemu-devel, mst, marcel,
	Dr . David Alan Gilbert

On Wed, 14 Dec 2016 17:19:58 +0100
Maxime Coquelin <maxime.coquelin@redhat.com> wrote:

> This patch fixes a cross-version migration regression introduced
> by commit d1b4259f ("virtio-bus: Plug devices after features are
> negotiated").
> 
> The problem is encountered when host's vhost backend does not support
> VIRTIO_F_VERSION_1, and migration is initiated from a v2.7 or prior
> QEMU version with virtio-pci modern capabilities enabled to a v2.8
> QEMU version.
> 
> In this case, modern capabilities get exposed to the guest by the source,
> whereas the target will detect version 1 is not supported so will only
> expose legacy capabilities.
> 
> The problem is fixed by introducing a new "x-modern-broken" property,

wrong property name...

> which is set in v2.7 and prior compatibility modes. Doing this, v2.7
> machine keeps its broken behaviour (enabling modern while version is
> not supported), and newer machines will behave correctly.
> 
> Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Tested-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> 
> Thanks for the reviews and tests.
> This version changes the naming as proposed by Michael T.
> and Cornelia, and fixies commit message.
> 
>  hw/virtio/virtio-pci.c | 5 ++++-
>  hw/virtio/virtio-pci.h | 1 +
>  include/hw/compat.h    | 4 ++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 521ba0b..21c2b9d 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1580,7 +1580,8 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp)
>       * Virtio capabilities present without
>       * VIRTIO_F_VERSION_1 confuses guests
>       */
> -    if (!virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {
> +    if (!proxy->ignore_backend_features &&
> +            !virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) {
>          virtio_pci_disable_modern(proxy);
> 
>          if (!legacy) {
> @@ -1852,6 +1853,8 @@ static Property virtio_pci_properties[] = {
>                      VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
>      DEFINE_PROP_BIT("page-per-vq", VirtIOPCIProxy, flags,
>                      VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT, false),
> +    DEFINE_PROP_BOOL("x-ignore-backend-features", VirtIOPCIProxy,
> +                     ignore_backend_features, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index b2a996f..5e07886 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -153,6 +153,7 @@ struct VirtIOPCIProxy {
>      int config_cap;
>      uint32_t flags;
>      bool disable_modern;
> +    bool ignore_backend_features;
>      OnOffAuto disable_legacy;
>      uint32_t class_code;
>      uint32_t nvectors;
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 0f06e11..8dfc7a3 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -18,6 +18,10 @@
>          .driver   = "intel-iommu",\
>          .property = "x-buggy-eim",\
>          .value    = "true",\
> +    },{\
> +        .driver   = "virtio-pci",\
> +        .property = "x-ignore-backend-features",\
> +        .value    = "on",\
>      },
> 
>  #define HW_COMPAT_2_6 \

Not 100% sure that this was the property name we agreed on, but it's
fine with me.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] virtio-pci: Fix cross-version migration with older machines
  2016-12-14 16:24 ` Cornelia Huck
@ 2016-12-14 16:27   ` Maxime Coquelin
  0 siblings, 0 replies; 3+ messages in thread
From: Maxime Coquelin @ 2016-12-14 16:27 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: mdroth, stefanha, qemu-devel, mst, marcel,
	Dr . David Alan Gilbert



On 12/14/2016 05:24 PM, Cornelia Huck wrote:
> On Wed, 14 Dec 2016 17:19:58 +0100
> Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
>
>> > This patch fixes a cross-version migration regression introduced
>> > by commit d1b4259f ("virtio-bus: Plug devices after features are
>> > negotiated").
>> >
>> > The problem is encountered when host's vhost backend does not support
>> > VIRTIO_F_VERSION_1, and migration is initiated from a v2.7 or prior
>> > QEMU version with virtio-pci modern capabilities enabled to a v2.8
>> > QEMU version.
>> >
>> > In this case, modern capabilities get exposed to the guest by the source,
>> > whereas the target will detect version 1 is not supported so will only
>> > expose legacy capabilities.
>> >
>> > The problem is fixed by introducing a new "x-modern-broken" property,
> wrong property name...
>

Whoops... v3 on its way.

Thanks,
Maxime

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-12-14 16:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-14 16:19 [Qemu-devel] [PATCH v2] virtio-pci: Fix cross-version migration with older machines Maxime Coquelin
2016-12-14 16:24 ` Cornelia Huck
2016-12-14 16:27   ` Maxime Coquelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).