Re: [PATCH RFC] virtio-fs: force virtio 1.x usage

[Cornelia Huck <cohuck@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

On Tue, 30 Jun 2020 13:10:37 +0100
Stefan Hajnoczi <stefanha@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>
> > ---  
> 
> I thought that the following already forced VIRTIO 1.0 because it
> doesn't advertize Legacy or Transitional devices:
> 
>   static const VirtioPCIDeviceTypeInfo vhost_user_fs_pci_info = {
>       .base_name             = TYPE_VHOST_USER_FS_PCI,
>       .non_transitional_name = "vhost-user-fs-pci",
>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>       .instance_size = sizeof(VHostUserFSPCI),
>       .instance_init = vhost_user_fs_pci_instance_init,
>       .class_init    = vhost_user_fs_pci_class_init,
>   };

This indeed makes vhost-user-fs-pci modern-only, I had not spotted that
when I wrote the patch. Other modern-only devices do not go down this
route and use the virtio_pci_force_virtio_1() approach.

> 
> Do you have a guest that sees this VIRTIO 1.0 device and still fails to
> negotiate the VERSION_1 feature bit?
> 
> > 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);  
> 
> Can this be moved to virtio_pci_types_register() so that it
> automatically happens for .non_transitional_name devices?

There are several existing modern-only devices that don't use that kind
of naming scheme...

What bothers me most is that you need to explicitly request a device to
be modern-only, while that should be the default for any newly added
device. Hence the approach with the centralized list of device types
mentioned in a parallel thread. The main problem with that is that the
proxy device starts getting realized before the virtio device with its
id is present... I failed to find a solution so far. But I'd really
like an approach that can work for all transports.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]