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
<dgilb...@redhat.com>:
>
> Copying in Stefan, Jason and Michael who know the virtio details
>
> Dave
>
> * Mikhail Sennikovsky (mikhail.sennikovs...@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.sennikovs...@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 / dgilb...@redhat.com / Manchester, UK
