[PATCH] virtio-net: prevent offloads reset on migration

Mikhail Sennikovsky posted 1 patch 4 years, 5 months ago
Test checkpatch failed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu failed
Test asan passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1569932308-30478-2-git-send-email-mikhail.sennikovskii@cloud.ionos.com
Maintainers: Jason Wang <jasowang@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
hw/display/virtio-gpu-base.c |  3 ++-
hw/net/virtio-net.c          |  5 +++--
hw/virtio/virtio.c           | 10 +++++-----
include/hw/virtio/virtio.h   |  2 +-
4 files changed, 11 insertions(+), 9 deletions(-)
[PATCH] virtio-net: prevent offloads reset on migration
Posted by Mikhail Sennikovsky 4 years, 5 months ago
Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
command are not preserved on VM migration.
Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES)
get enabled.
What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads
are getting set correctly:

 #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474
 #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
 #2  virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334
 #3  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11)
     at migration/vmstate.c:168
 #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197
 #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
 #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
 #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
 #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
 #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
 #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
 #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449

However later on the features are getting restored, and offloads get reset to
everything supported by features:

 #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474
 #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
 #2  virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773
 #3  virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052
 #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220
 #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
 #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
 #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
 #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
 #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
 #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
 #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449

This patch fixes this by adding an extra argument to the set_features callback,
specifying whether the offloads are to be reset, and setting it to false
for the migration case.

Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
---
 hw/display/virtio-gpu-base.c |  3 ++-
 hw/net/virtio-net.c          |  5 +++--
 hw/virtio/virtio.c           | 10 +++++-----
 include/hw/virtio/virtio.h   |  2 +-
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 55e0799..04d8a23 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
 }
 
 static void
-virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
+virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features,
+                               bool reset_offloads)
 {
     static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL);
     VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev);
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b9e1cd7..5d108e5 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
     return virtio_net_guest_offloads_by_features(vdev->guest_features);
 }
 
-static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
+static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features,
+                                        bool reset_offloads)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
     int i;
@@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
     n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) &&
         virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6);
 
-    if (n->has_vnet_hdr) {
+    if (reset_offloads && n->has_vnet_hdr) {
         n->curr_guest_offloads =
             virtio_net_guest_offloads_by_features(features);
         virtio_net_apply_guest_offloads(n);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index a94ea18..b89f7b0 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2042,14 +2042,14 @@ const VMStateInfo  virtio_vmstate_info = {
     .put = virtio_device_put,
 };
 
-static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
+static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     bool bad = (val & ~(vdev->host_features)) != 0;
 
     val &= vdev->host_features;
     if (k->set_features) {
-        k->set_features(vdev, val);
+        k->set_features(vdev, val, reset_offloads);
     }
     vdev->guest_features = val;
     return bad ? -1 : 0;
@@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
     if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) {
         return -EINVAL;
     }
-    ret = virtio_set_features_nocheck(vdev, val);
+    ret = virtio_set_features_nocheck(vdev, val, true);
     if (!ret) {
         if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
             /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
@@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
          * host_features.
          */
         uint64_t features64 = vdev->guest_features;
-        if (virtio_set_features_nocheck(vdev, features64) < 0) {
+        if (virtio_set_features_nocheck(vdev, features64, false) < 0) {
             error_report("Features 0x%" PRIx64 " unsupported. "
                          "Allowed features: 0x%" PRIx64,
                          features64, vdev->host_features);
             return -1;
         }
     } else {
-        if (virtio_set_features_nocheck(vdev, features) < 0) {
+        if (virtio_set_features_nocheck(vdev, features, false) < 0) {
             error_report("Features 0x%x unsupported. "
                          "Allowed features: 0x%" PRIx64,
                          features, vdev->host_features);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b189788..fd8cac5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass {
                              uint64_t requested_features,
                              Error **errp);
     uint64_t (*bad_features)(VirtIODevice *vdev);
-    void (*set_features)(VirtIODevice *vdev, uint64_t val);
+    void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads);
     int (*validate_features)(VirtIODevice *vdev);
     void (*get_config)(VirtIODevice *vdev, uint8_t *config);
     void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
-- 
2.7.4


Re: [PATCH] virtio-net: prevent offloads reset on migration
Posted by no-reply@patchew.org 4 years, 5 months ago
Patchew URL: https://patchew.org/QEMU/1569932308-30478-2-git-send-email-mikhail.sennikovskii@cloud.ionos.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1569932308-30478-2-git-send-email-mikhail.sennikovskii@cloud.ionos.com
Subject: [PATCH] virtio-net: prevent offloads reset on migration

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
9d2774e virtio-net: prevent offloads reset on migration

=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#97: FILE: hw/virtio/virtio.c:2049:
+static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads)

total: 1 errors, 0 warnings, 74 lines checked

Commit 9d2774ea7eb1 (virtio-net: prevent offloads reset on migration) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1569932308-30478-2-git-send-email-mikhail.sennikovskii@cloud.ionos.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH] virtio-net: prevent offloads reset on migration
Posted by Dr. David Alan Gilbert 4 years, 5 months ago
Copying in Stefan, Jason and Michael who know the virtio details 

Dave

* Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote:
> Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
> command are not preserved on VM migration.
> Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES)
> get enabled.
> What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads
> are getting set correctly:
> 
>  #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474
>  #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
>  #2  virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334
>  #3  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11)
>      at migration/vmstate.c:168
>  #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197
>  #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
>  #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
>  #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
>  #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
>  #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
>  #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
>  #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
> 
> However later on the features are getting restored, and offloads get reset to
> everything supported by features:
> 
>  #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474
>  #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
>  #2  virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773
>  #3  virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052
>  #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220
>  #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
>  #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
>  #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
>  #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
>  #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
>  #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
>  #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
> 
> This patch fixes this by adding an extra argument to the set_features callback,
> specifying whether the offloads are to be reset, and setting it to false
> for the migration case.
> 
> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
> ---
>  hw/display/virtio-gpu-base.c |  3 ++-
>  hw/net/virtio-net.c          |  5 +++--
>  hw/virtio/virtio.c           | 10 +++++-----
>  include/hw/virtio/virtio.h   |  2 +-
>  4 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
> index 55e0799..04d8a23 100644
> --- a/hw/display/virtio-gpu-base.c
> +++ b/hw/display/virtio-gpu-base.c
> @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
>  }
>  
>  static void
> -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
> +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features,
> +                               bool reset_offloads)
>  {
>      static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL);
>      VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev);
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index b9e1cd7..5d108e5 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
>      return virtio_net_guest_offloads_by_features(vdev->guest_features);
>  }
>  
> -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features,
> +                                        bool reset_offloads)
>  {
>      VirtIONet *n = VIRTIO_NET(vdev);
>      int i;
> @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>      n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) &&
>          virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6);
>  
> -    if (n->has_vnet_hdr) {
> +    if (reset_offloads && n->has_vnet_hdr) {
>          n->curr_guest_offloads =
>              virtio_net_guest_offloads_by_features(features);
>          virtio_net_apply_guest_offloads(n);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index a94ea18..b89f7b0 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2042,14 +2042,14 @@ const VMStateInfo  virtio_vmstate_info = {
>      .put = virtio_device_put,
>  };
>  
> -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
> +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads)
>  {
>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>      bool bad = (val & ~(vdev->host_features)) != 0;
>  
>      val &= vdev->host_features;
>      if (k->set_features) {
> -        k->set_features(vdev, val);
> +        k->set_features(vdev, val, reset_offloads);
>      }
>      vdev->guest_features = val;
>      return bad ? -1 : 0;
> @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
>      if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) {
>          return -EINVAL;
>      }
> -    ret = virtio_set_features_nocheck(vdev, val);
> +    ret = virtio_set_features_nocheck(vdev, val, true);
>      if (!ret) {
>          if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
>              /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
> @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>           * host_features.
>           */
>          uint64_t features64 = vdev->guest_features;
> -        if (virtio_set_features_nocheck(vdev, features64) < 0) {
> +        if (virtio_set_features_nocheck(vdev, features64, false) < 0) {
>              error_report("Features 0x%" PRIx64 " unsupported. "
>                           "Allowed features: 0x%" PRIx64,
>                           features64, vdev->host_features);
>              return -1;
>          }
>      } else {
> -        if (virtio_set_features_nocheck(vdev, features) < 0) {
> +        if (virtio_set_features_nocheck(vdev, features, false) < 0) {
>              error_report("Features 0x%x unsupported. "
>                           "Allowed features: 0x%" PRIx64,
>                           features, vdev->host_features);
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b189788..fd8cac5 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass {
>                               uint64_t requested_features,
>                               Error **errp);
>      uint64_t (*bad_features)(VirtIODevice *vdev);
> -    void (*set_features)(VirtIODevice *vdev, uint64_t val);
> +    void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads);
>      int (*validate_features)(VirtIODevice *vdev);
>      void (*get_config)(VirtIODevice *vdev, uint8_t *config);
>      void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
> -- 
> 2.7.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] virtio-net: prevent offloads reset on migration
Posted by Jason Wang 4 years, 5 months ago
On 2019/10/2 下午5:55, Dr. David Alan Gilbert wrote:
> Copying in Stefan, Jason and Michael who know the virtio details
>
> Dave
>
> * Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote:
>> Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
>> command are not preserved on VM migration.
>> Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES)
>> get enabled.
>> What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads
>> are getting set correctly:
>>
>>   #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474
>>   #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
>>   #2  virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334
>>   #3  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11)
>>       at migration/vmstate.c:168
>>   #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197
>>   #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
>>   #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
>>   #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
>>   #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
>>   #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
>>   #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
>>   #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
>>
>> However later on the features are getting restored, and offloads get reset to
>> everything supported by features:
>>
>>   #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474
>>   #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
>>   #2  virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773
>>   #3  virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052
>>   #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220
>>   #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
>>   #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
>>   #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
>>   #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
>>   #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
>>   #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
>>   #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
>>
>> This patch fixes this by adding an extra argument to the set_features callback,
>> specifying whether the offloads are to be reset, and setting it to false
>> for the migration case.
>>
>> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
>> ---
>>   hw/display/virtio-gpu-base.c |  3 ++-
>>   hw/net/virtio-net.c          |  5 +++--
>>   hw/virtio/virtio.c           | 10 +++++-----
>>   include/hw/virtio/virtio.h   |  2 +-
>>   4 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
>> index 55e0799..04d8a23 100644
>> --- a/hw/display/virtio-gpu-base.c
>> +++ b/hw/display/virtio-gpu-base.c
>> @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
>>   }
>>   
>>   static void
>> -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
>> +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features,
>> +                               bool reset_offloads)


It's not good for e.g gpu to know anything about net.

How about checking runstate and do not call apply_guest_offload() in 
virtio_net_set_features() when in the state of migration?

Thanks


>>   {
>>       static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL);
>>       VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev);
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index b9e1cd7..5d108e5 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
>>       return virtio_net_guest_offloads_by_features(vdev->guest_features);
>>   }
>>   
>> -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>> +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features,
>> +                                        bool reset_offloads)
>>   {
>>       VirtIONet *n = VIRTIO_NET(vdev);
>>       int i;
>> @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>>       n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) &&
>>           virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6);
>>   
>> -    if (n->has_vnet_hdr) {
>> +    if (reset_offloads && n->has_vnet_hdr) {
>>           n->curr_guest_offloads =
>>               virtio_net_guest_offloads_by_features(features);
>>           virtio_net_apply_guest_offloads(n);
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index a94ea18..b89f7b0 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -2042,14 +2042,14 @@ const VMStateInfo  virtio_vmstate_info = {
>>       .put = virtio_device_put,
>>   };
>>   
>> -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
>> +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads)
>>   {
>>       VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>>       bool bad = (val & ~(vdev->host_features)) != 0;
>>   
>>       val &= vdev->host_features;
>>       if (k->set_features) {
>> -        k->set_features(vdev, val);
>> +        k->set_features(vdev, val, reset_offloads);
>>       }
>>       vdev->guest_features = val;
>>       return bad ? -1 : 0;
>> @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
>>       if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) {
>>           return -EINVAL;
>>       }
>> -    ret = virtio_set_features_nocheck(vdev, val);
>> +    ret = virtio_set_features_nocheck(vdev, val, true);
>>       if (!ret) {
>>           if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
>>               /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
>> @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>>            * host_features.
>>            */
>>           uint64_t features64 = vdev->guest_features;
>> -        if (virtio_set_features_nocheck(vdev, features64) < 0) {
>> +        if (virtio_set_features_nocheck(vdev, features64, false) < 0) {
>>               error_report("Features 0x%" PRIx64 " unsupported. "
>>                            "Allowed features: 0x%" PRIx64,
>>                            features64, vdev->host_features);
>>               return -1;
>>           }
>>       } else {
>> -        if (virtio_set_features_nocheck(vdev, features) < 0) {
>> +        if (virtio_set_features_nocheck(vdev, features, false) < 0) {
>>               error_report("Features 0x%x unsupported. "
>>                            "Allowed features: 0x%" PRIx64,
>>                            features, vdev->host_features);
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index b189788..fd8cac5 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass {
>>                                uint64_t requested_features,
>>                                Error **errp);
>>       uint64_t (*bad_features)(VirtIODevice *vdev);
>> -    void (*set_features)(VirtIODevice *vdev, uint64_t val);
>> +    void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads);
>>       int (*validate_features)(VirtIODevice *vdev);
>>       void (*get_config)(VirtIODevice *vdev, uint8_t *config);
>>       void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
>> -- 
>> 2.7.4
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] virtio-net: prevent offloads reset on migration
Posted by Mikhail Sennikovsky 4 years, 5 months ago
Hi Jason,

Thank you for your reply.
I've submitted the third version of the patch which does the runstate
check as you propose.

Thanks,
Mikhail

Am Do., 10. Okt. 2019 um 07:33 Uhr schrieb Jason Wang <jasowang@redhat.com>:
>
>
> On 2019/10/2 下午5:55, Dr. David Alan Gilbert wrote:
> > Copying in Stefan, Jason and Michael who know the virtio details
> >
> > Dave
> >
> > * Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote:
> >> Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
> >> command are not preserved on VM migration.
> >> Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES)
> >> get enabled.
> >> What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads
> >> are getting set correctly:
> >>
> >>   #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474
> >>   #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
> >>   #2  virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334
> >>   #3  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11)
> >>       at migration/vmstate.c:168
> >>   #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197
> >>   #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
> >>   #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
> >>   #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
> >>   #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
> >>   #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
> >>   #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
> >>   #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
> >>
> >> However later on the features are getting restored, and offloads get reset to
> >> everything supported by features:
> >>
> >>   #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474
> >>   #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
> >>   #2  virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773
> >>   #3  virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052
> >>   #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220
> >>   #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
> >>   #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
> >>   #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
> >>   #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
> >>   #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
> >>   #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
> >>   #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
> >>
> >> This patch fixes this by adding an extra argument to the set_features callback,
> >> specifying whether the offloads are to be reset, and setting it to false
> >> for the migration case.
> >>
> >> Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
> >> ---
> >>   hw/display/virtio-gpu-base.c |  3 ++-
> >>   hw/net/virtio-net.c          |  5 +++--
> >>   hw/virtio/virtio.c           | 10 +++++-----
> >>   include/hw/virtio/virtio.h   |  2 +-
> >>   4 files changed, 11 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
> >> index 55e0799..04d8a23 100644
> >> --- a/hw/display/virtio-gpu-base.c
> >> +++ b/hw/display/virtio-gpu-base.c
> >> @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
> >>   }
> >>
> >>   static void
> >> -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
> >> +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features,
> >> +                               bool reset_offloads)
>
>
> It's not good for e.g gpu to know anything about net.
>
> How about checking runstate and do not call apply_guest_offload() in
> virtio_net_set_features() when in the state of migration?
>
> Thanks
>
>
> >>   {
> >>       static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL);
> >>       VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev);
> >> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >> index b9e1cd7..5d108e5 100644
> >> --- a/hw/net/virtio-net.c
> >> +++ b/hw/net/virtio-net.c
> >> @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
> >>       return virtio_net_guest_offloads_by_features(vdev->guest_features);
> >>   }
> >>
> >> -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> >> +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features,
> >> +                                        bool reset_offloads)
> >>   {
> >>       VirtIONet *n = VIRTIO_NET(vdev);
> >>       int i;
> >> @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> >>       n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) &&
> >>           virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6);
> >>
> >> -    if (n->has_vnet_hdr) {
> >> +    if (reset_offloads && n->has_vnet_hdr) {
> >>           n->curr_guest_offloads =
> >>               virtio_net_guest_offloads_by_features(features);
> >>           virtio_net_apply_guest_offloads(n);
> >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >> index a94ea18..b89f7b0 100644
> >> --- a/hw/virtio/virtio.c
> >> +++ b/hw/virtio/virtio.c
> >> @@ -2042,14 +2042,14 @@ const VMStateInfo  virtio_vmstate_info = {
> >>       .put = virtio_device_put,
> >>   };
> >>
> >> -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
> >> +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads)
> >>   {
> >>       VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> >>       bool bad = (val & ~(vdev->host_features)) != 0;
> >>
> >>       val &= vdev->host_features;
> >>       if (k->set_features) {
> >> -        k->set_features(vdev, val);
> >> +        k->set_features(vdev, val, reset_offloads);
> >>       }
> >>       vdev->guest_features = val;
> >>       return bad ? -1 : 0;
> >> @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> >>       if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) {
> >>           return -EINVAL;
> >>       }
> >> -    ret = virtio_set_features_nocheck(vdev, val);
> >> +    ret = virtio_set_features_nocheck(vdev, val, true);
> >>       if (!ret) {
> >>           if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
> >>               /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
> >> @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> >>            * host_features.
> >>            */
> >>           uint64_t features64 = vdev->guest_features;
> >> -        if (virtio_set_features_nocheck(vdev, features64) < 0) {
> >> +        if (virtio_set_features_nocheck(vdev, features64, false) < 0) {
> >>               error_report("Features 0x%" PRIx64 " unsupported. "
> >>                            "Allowed features: 0x%" PRIx64,
> >>                            features64, vdev->host_features);
> >>               return -1;
> >>           }
> >>       } else {
> >> -        if (virtio_set_features_nocheck(vdev, features) < 0) {
> >> +        if (virtio_set_features_nocheck(vdev, features, false) < 0) {
> >>               error_report("Features 0x%x unsupported. "
> >>                            "Allowed features: 0x%" PRIx64,
> >>                            features, vdev->host_features);
> >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >> index b189788..fd8cac5 100644
> >> --- a/include/hw/virtio/virtio.h
> >> +++ b/include/hw/virtio/virtio.h
> >> @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass {
> >>                                uint64_t requested_features,
> >>                                Error **errp);
> >>       uint64_t (*bad_features)(VirtIODevice *vdev);
> >> -    void (*set_features)(VirtIODevice *vdev, uint64_t val);
> >> +    void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads);
> >>       int (*validate_features)(VirtIODevice *vdev);
> >>       void (*get_config)(VirtIODevice *vdev, uint8_t *config);
> >>       void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
> >> --
> >> 2.7.4
> >>
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] virtio-net: prevent offloads reset on migration
Posted by Mikhail Sennikovsky 4 years, 5 months ago
Guys, any update on this? Note that I have also sent a second version
of the patch here, which has the
coding style issue fixed.

Thanks,
Mikhail

Am Mi., 2. Okt. 2019 um 11:55 Uhr schrieb Dr. David Alan Gilbert
<dgilbert@redhat.com>:
>
> Copying in Stefan, Jason and Michael who know the virtio details
>
> Dave
>
> * Mikhail Sennikovsky (mikhail.sennikovskii@cloud.ionos.com) wrote:
> > Currently offloads disabled by guest via the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
> > command are not preserved on VM migration.
> > Instead all offloads reported by guest features (via VIRTIO_PCI_GUEST_FEATURES)
> > get enabled.
> > What happens is: first the VirtIONet::curr_guest_offloads gets restored and offloads
> > are getting set correctly:
> >
> >  #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=0, tso6=0, ecn=0, ufo=0) at net/net.c:474
> >  #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
> >  #2  virtio_net_post_load_device (opaque=0x555557701ca0, version_id=11) at hw/net/virtio-net.c:2334
> >  #3  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577c80 <vmstate_virtio_net_device>, opaque=0x555557701ca0, version_id=11)
> >      at migration/vmstate.c:168
> >  #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2197
> >  #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
> >  #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
> >  #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
> >  #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
> >  #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
> >  #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
> >  #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
> >
> > However later on the features are getting restored, and offloads get reset to
> > everything supported by features:
> >
> >  #0  qemu_set_offload (nc=0x555556a11400, csum=1, tso4=1, tso6=1, ecn=0, ufo=0) at net/net.c:474
> >  #1  virtio_net_apply_guest_offloads (n=0x555557701ca0) at hw/net/virtio-net.c:720
> >  #2  virtio_net_set_features (vdev=0x555557701ca0, features=5104441767) at hw/net/virtio-net.c:773
> >  #3  virtio_set_features_nocheck (vdev=0x555557701ca0, val=5104441767) at hw/virtio/virtio.c:2052
> >  #4  virtio_load (vdev=0x555557701ca0, f=0x5555569dc010, version_id=11) at hw/virtio/virtio.c:2220
> >  #5  virtio_device_get (f=0x5555569dc010, opaque=0x555557701ca0, size=0, field=0x55555668cd00 <__compound_literal.5>) at hw/virtio/virtio.c:2036
> >  #6  vmstate_load_state (f=0x5555569dc010, vmsd=0x555556577ce0 <vmstate_virtio_net>, opaque=0x555557701ca0, version_id=11) at migration/vmstate.c:143
> >  #7  vmstate_load (f=0x5555569dc010, se=0x5555578189e0) at migration/savevm.c:829
> >  #8  qemu_loadvm_section_start_full (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2211
> >  #9  qemu_loadvm_state_main (f=0x5555569dc010, mis=0x5555569eee20) at migration/savevm.c:2395
> >  #10 qemu_loadvm_state (f=0x5555569dc010) at migration/savevm.c:2467
> >  #11 process_incoming_migration_co (opaque=0x0) at migration/migration.c:449
> >
> > This patch fixes this by adding an extra argument to the set_features callback,
> > specifying whether the offloads are to be reset, and setting it to false
> > for the migration case.
> >
> > Signed-off-by: Mikhail Sennikovsky <mikhail.sennikovskii@cloud.ionos.com>
> > ---
> >  hw/display/virtio-gpu-base.c |  3 ++-
> >  hw/net/virtio-net.c          |  5 +++--
> >  hw/virtio/virtio.c           | 10 +++++-----
> >  include/hw/virtio/virtio.h   |  2 +-
> >  4 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
> > index 55e0799..04d8a23 100644
> > --- a/hw/display/virtio-gpu-base.c
> > +++ b/hw/display/virtio-gpu-base.c
> > @@ -193,7 +193,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
> >  }
> >
> >  static void
> > -virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
> > +virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features,
> > +                               bool reset_offloads)
> >  {
> >      static const uint32_t virgl = (1 << VIRTIO_GPU_F_VIRGL);
> >      VirtIOGPUBase *g = VIRTIO_GPU_BASE(vdev);
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index b9e1cd7..5d108e5 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -743,7 +743,8 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
> >      return virtio_net_guest_offloads_by_features(vdev->guest_features);
> >  }
> >
> > -static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> > +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features,
> > +                                        bool reset_offloads)
> >  {
> >      VirtIONet *n = VIRTIO_NET(vdev);
> >      int i;
> > @@ -767,7 +768,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> >      n->rsc6_enabled = virtio_has_feature(features, VIRTIO_NET_F_RSC_EXT) &&
> >          virtio_has_feature(features, VIRTIO_NET_F_GUEST_TSO6);
> >
> > -    if (n->has_vnet_hdr) {
> > +    if (reset_offloads && n->has_vnet_hdr) {
> >          n->curr_guest_offloads =
> >              virtio_net_guest_offloads_by_features(features);
> >          virtio_net_apply_guest_offloads(n);
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index a94ea18..b89f7b0 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -2042,14 +2042,14 @@ const VMStateInfo  virtio_vmstate_info = {
> >      .put = virtio_device_put,
> >  };
> >
> > -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
> > +static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val, bool reset_offloads)
> >  {
> >      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> >      bool bad = (val & ~(vdev->host_features)) != 0;
> >
> >      val &= vdev->host_features;
> >      if (k->set_features) {
> > -        k->set_features(vdev, val);
> > +        k->set_features(vdev, val, reset_offloads);
> >      }
> >      vdev->guest_features = val;
> >      return bad ? -1 : 0;
> > @@ -2065,7 +2065,7 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> >      if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) {
> >          return -EINVAL;
> >      }
> > -    ret = virtio_set_features_nocheck(vdev, val);
> > +    ret = virtio_set_features_nocheck(vdev, val, true);
> >      if (!ret) {
> >          if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
> >              /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
> > @@ -2217,14 +2217,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> >           * host_features.
> >           */
> >          uint64_t features64 = vdev->guest_features;
> > -        if (virtio_set_features_nocheck(vdev, features64) < 0) {
> > +        if (virtio_set_features_nocheck(vdev, features64, false) < 0) {
> >              error_report("Features 0x%" PRIx64 " unsupported. "
> >                           "Allowed features: 0x%" PRIx64,
> >                           features64, vdev->host_features);
> >              return -1;
> >          }
> >      } else {
> > -        if (virtio_set_features_nocheck(vdev, features) < 0) {
> > +        if (virtio_set_features_nocheck(vdev, features, false) < 0) {
> >              error_report("Features 0x%x unsupported. "
> >                           "Allowed features: 0x%" PRIx64,
> >                           features, vdev->host_features);
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index b189788..fd8cac5 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -128,7 +128,7 @@ typedef struct VirtioDeviceClass {
> >                               uint64_t requested_features,
> >                               Error **errp);
> >      uint64_t (*bad_features)(VirtIODevice *vdev);
> > -    void (*set_features)(VirtIODevice *vdev, uint64_t val);
> > +    void (*set_features)(VirtIODevice *vdev, uint64_t val, bool reset_offloads);
> >      int (*validate_features)(VirtIODevice *vdev);
> >      void (*get_config)(VirtIODevice *vdev, uint8_t *config);
> >      void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
> > --
> > 2.7.4
> >
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK