[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC] virtio-fs: force virtio 1.x usage
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH RFC] virtio-fs: force virtio 1.x usage |
Date: |
Mon, 29 Jun 2020 11:45:35 -0400 |
On Mon, Jun 29, 2020 at 05:39:33PM +0200, Cornelia Huck wrote:
> On Mon, 29 Jun 2020 10:53:23 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Mon, Jun 29, 2020 at 12:27:58PM +0200, Cornelia Huck wrote:
> > > virtio-fs devices are only specified for virtio-1, so it is unclear
> > > how a legacy or transitional device should behave.
> > >
> > > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > > ---
> > >
> > > Forcing off legacy now (after the virtio-fs device has already been
> > > available) may have unintended consequences, therefore RFC.
> > >
> > > By default, a virtio-pci device uses 'AUTO' for disable_legacy, which
> > > will resolve to different values based upon which bus the device has
> > > been plugged. Therefore, forcing disable_legacy may result in the same
> > > device or a quite different one.
> > >
> > > Even though pre-virtio-1 behaviour of virtio-fs devices is simply not
> > > specified, toggling disable_legacy will have implications for the BAR
> > > layout, IIRC, and therefore a guest might end up getting a different
> > > device, even if it always used it with virtio-1 anyway.
> > >
> > > Not sure what the best way to solve this problem is. Adding a compat
> > > property for disable_legacy=AUTO may be the right thing to do, but I'm
> > > not quite clear if there are any further implications here.
> >
> > Well I notice that this device is not migrateable.
> > So I think that we can just switch it over and be done with it.
>
> Oh, that makes things easier. (I'm wondering if libvirt already
> configures this correctly?)
>
> >
> >
> > > Whatever we do here, we should make sure that the ccw incarnation of
> > > this device indeed forces virtio-1.
> >
> > I agree. I notice that the API virtio_pci_force_virtio_1 turned out
> > to be too fragile. I propose that instead we have a whitelist of
> > devices which can be legacy or transitional. Force rest to modern.
>
> Also, there are further complications because the mechanism is per
> transport, and therefore easy to miss.
>
> bool virtio_legacy_allowed(VirtIODevice *vdev)
> {
> switch (vdev->device_id) {
> case <...>:
> <list of legacy-capable devices>
> return true;
> default:
> return false;
> }
>
> Seems straightforward enough.
Agreed. virtio spec has the list.
> >
> >
> > > ---
> > > hw/virtio/vhost-user-fs-pci.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> > > index e11c889d82b3..244205edf765 100644
> > > --- a/hw/virtio/vhost-user-fs-pci.c
> > > +++ b/hw/virtio/vhost-user-fs-pci.c
> > > @@ -44,6 +44,7 @@ static void vhost_user_fs_pci_realize(VirtIOPCIProxy
> > > *vpci_dev, Error **errp)
> > > vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 2;
> > > }
> > >
> > > + virtio_pci_force_virtio_1(vpci_dev);
> > > qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> > > }
> > >
> > > --
> > > 2.25.4
> >
Re: [PATCH RFC] virtio-fs: force virtio 1.x usage, Dr. David Alan Gilbert, 2020/06/29
Re: [PATCH RFC] virtio-fs: force virtio 1.x usage, Stefan Hajnoczi, 2020/06/30