IMO, this does not depend on features of vhost as soon as we're not able to
provide parameters to it.
On Tue, Mar 10, 2020 at 8:17 AM Michael S. Tsirkin <m...@redhat.com> wrote:
> On Tue, Mar 10, 2020 at 11:12:05AM +0800, Jason Wang wrote:
> >
> > On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> > > Block migration for reference implementation of
> > > RSS feature in QEMU. When we add support for RSS
> > > on backend side, we'll implement migration of
> > > current RSS settings.
> > >
> > > Signed-off-by: Yuri Benditovich <yuri.benditov...@daynix.com>
> > > ---
> > > hw/net/virtio-net.c | 18 ++++++++++++++++++
> > > include/hw/virtio/virtio-net.h | 1 +
> > > 2 files changed, 19 insertions(+)
> > >
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index abc41fdb16..943d1931a2 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -37,6 +37,7 @@
> > > #include "qapi/qapi-events-migration.h"
> > > #include "hw/virtio/virtio-access.h"
> > > #include "migration/misc.h"
> > > +#include "migration/blocker.h"
> > > #include "standard-headers/linux/ethtool.h"
> > > #include "sysemu/sysemu.h"
> > > #include "trace.h"
> > > @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev)
> > > n->announce_timer.round = 0;
> > > n->status &= ~VIRTIO_NET_S_ANNOUNCE;
> > > + if (n->migration_blocker) {
> > > + migrate_del_blocker(n->migration_blocker);
> > > + error_free(n->migration_blocker);
> > > + n->migration_blocker = NULL;
> > > + }
> > > +
> > > /* Flush any MAC and VLAN filter table state */
> > > n->mac_table.in_use = 0;
> > > n->mac_table.first_multi = 0;
> > > @@ -1003,6 +1010,17 @@ static void
> virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> > > vhost_net_ack_features(get_vhost_net(nc->peer), features);
> > > }
> > > + if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
> > > + if (!n->migration_blocker) {
> > > + error_setg(&n->migration_blocker, "virtio-net: RSS
> negotiated");
> > > + migrate_add_blocker(n->migration_blocker, &err);
> > > + if (err) {
> > > + error_report_err(err);
> > > + err = NULL;
> > > + }
> > > + }
> > > + }
> >
> >
> > It looks to me we should add the blocker unconditionally based on
> > virtio_host_has_feature() instead of depending the negotiated feature
> here.
> >
> > Thanks
>
> I agree, this is a stopgap measure anyway. If this is merged in its
> current form, there should also be a big TODO here that we must fix this
> ASAP, and maybe a warning produced.
>
>
> >
> > > +
> > > if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
> > > memset(n->vlans, 0, MAX_VLAN >> 3);
> > > } else {
> > > diff --git a/include/hw/virtio/virtio-net.h
> b/include/hw/virtio/virtio-net.h
> > > index 45670dd054..fba768ba9b 100644
> > > --- a/include/hw/virtio/virtio-net.h
> > > +++ b/include/hw/virtio/virtio-net.h
> > > @@ -180,6 +180,7 @@ struct VirtIONet {
> > > virtio_net_conf net_conf;
> > > NICConf nic_conf;
> > > DeviceState *qdev;
> > > + Error *migration_blocker;
> > > int multiqueue;
> > > uint16_t max_queues;
> > > uint16_t curr_queues;
>
>
