Re: [PULL v4 30/83] virtio: core: vq reset feature negotation support

[Stefano Garzarella]

On Mon, Nov 21, 2022 at 02:07:41AM -0500, Michael S. Tsirkin wrote:
> On Mon, Nov 21, 2022 at 02:17:02PM +0800, Jason Wang wrote:
> > On Sun, Nov 20, 2022 at 1:19 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Fri, Nov 18, 2022 at 03:32:56PM +0100, Stefano Garzarella wrote:
> > > > Hi,
> > > > starting from this commit 69e1c14aa2 ("virtio: core: vq reset feature
> > > > negotation support"), vhost-user-vsock and vhost-vsock fails while
> > > > setting the device features, because VIRTIO_F_RING_RESET is not masked.
> > > >
> > > > I'm not sure vsock is the only one affected.
> > > >
> > > > We could fix in two ways:
> > > >
> > > > 1) Masking VIRTIO_F_RING_RESET when we call vhost_get_features:
> > > >
> > > > diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
> > > > index 29b9ab4f72..e671cc695f 100644
> > > > --- a/hw/virtio/vhost-vsock-common.c
> > > > +++ b/hw/virtio/vhost-vsock-common.c
> > > > @@ -21,6 +21,7 @@
> > > >
> > > >  const int feature_bits[] = {
> > > >      VIRTIO_VSOCK_F_SEQPACKET,
> > > > +    VIRTIO_F_RING_RESET,
> > > >      VHOST_INVALID_FEATURE_BIT
> > > >  };
> > > >
> > >
> > > Let's do this, we need to be conservative.
> >
> > Ack.

Okay, thanks for the feedbacks!

> > Thanks
>
>
> Patch pls? Stefano?

Yep, I'll send it.

Should we do the same in the other vhost and vhost-user devices?
IIUC now we only filter it in vhost-net.

Then I'll try to see if we can rework how we handle features to make 
sure we avoid this in the future. Maybe by creating an array of core 
features always filtered, because for example this one is not device 
specific.

Thanks,
Stefano