Re: [PATCH] virtio-net: not enable vq reset feature unconditionally

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, May 08, 2023 at 11:09:46AM +0200, Eugenio Perez Martin wrote:
> On Sat, May 6, 2023 at 4:25 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Thu,  4 May 2023 12:14:47 +0200, =?utf-8?q?Eugenio_P=C3=A9rez?= <eperezma@redhat.com> wrote:
> > > The commit 93a97dc5200a ("virtio-net: enable vq reset feature") enables
> > > unconditionally vq reset feature as long as the device is emulated.
> > > This makes impossible to actually disable the feature, and it causes
> > > migration problems from qemu version previous than 7.2.
> > >
> > > The entire final commit is unneeded as device system already enable or
> > > disable the feature properly.
> > >
> > > This reverts commit 93a97dc5200a95e63b99cb625f20b7ae802ba413.
> > > Fixes: 93a97dc5200a ("virtio-net: enable vq reset feature")
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > >
> > > ---
> > > Tested by checking feature bit at  /sys/devices/pci.../virtio0/features
> > > enabling and disabling queue_reset virtio-net feature and vhost=on/off
> > > on net device backend.
> >
> > Do you mean that this feature cannot be closed?
> >
> > I tried to close in the guest, it was successful.
> >
> 
> I'm not sure what you mean with close. If the device dataplane is
> emulated in qemu (vhost=off), I'm not able to make the device not
> offer it.
> 
> > In addition, in this case, could you try to repair the problem instead of
> > directly revert.
> >
> 
> I'm not following this. The revert is not to always disable the feature.
> 
> By default, the feature is enabled. If cmdline states queue_reset=on,
> the feature is enabled. That is true both before and after applying
> this patch.
> 
> However, in qemu master, queue_reset=off keeps enabling this feature
> on the device. It happens that there is a commit explicitly doing
> that, so I'm reverting it.
> 
> Let me know if that makes sense to you.
> 
> Thanks!


question is this:

    DEFINE_PROP_BIT64("queue_reset", _state, _field, \
                      VIRTIO_F_RING_RESET, true)



don't we need compat for 7.2 and back for this property?

-- 
MST