On Thu, Aug 13, 2020 at 11:28:20AM +0200, Cornelia Huck wrote:
> On Thu, 13 Aug 2020 11:16:56 +0200
> Stefano Garzarella <sgarz...@redhat.com> wrote:
>
> > Hi,
> >
> > Qinghua discovered that virtio-vsock-pci requires 'disable-legacy=on' in
> > QEMU 5.1:
> > $ ./qemu-system-x86_64 ... -device vhost-vsock-pci,guest-cid=5
> > qemu-system-x86_64: -device vhost-vsock-pci,guest-cid=5:
> > device is modern-only, use disable-legacy=on
> >
> > Bisecting I found that this behaviour starts from this commit:
> > 9b3a35ec82 ("virtio: verify that legacy support is not accidentally on")
>
> Oh, I had heard that from others already, was still trying to figure
> out what to do.
>
> >
> > IIUC virtio-vsock is modern-only, so I tried this patch and it works:
> >
> > diff --git a/hw/virtio/vhost-user-vsock-pci.c
> > b/hw/virtio/vhost-user-vsock-pci.c
> > index f4cf95873d..6e4cc874cd 100644
> > --- a/hw/virtio/vhost-user-vsock-pci.c
> > +++ b/hw/virtio/vhost-user-vsock-pci.c
> > @@ -40,6 +40,7 @@ static void vhost_user_vsock_pci_realize(VirtIOPCIProxy
> > *vpci_dev, Error **errp)
> > VHostUserVSockPCI *dev = VHOST_USER_VSOCK_PCI(vpci_dev);
> > DeviceState *vdev = DEVICE(&dev->vdev);
> >
> > + virtio_pci_force_virtio_1(vpci_dev);
> > qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> > }
> >
> > diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c
> > index a815278e69..f641b974e9 100644
> > --- a/hw/virtio/vhost-vsock-pci.c
> > +++ b/hw/virtio/vhost-vsock-pci.c
> > @@ -44,6 +44,7 @@ static void vhost_vsock_pci_realize(VirtIOPCIProxy
> > *vpci_dev, Error **errp)
> > VHostVSockPCI *dev = VHOST_VSOCK_PCI(vpci_dev);
> > DeviceState *vdev = DEVICE(&dev->vdev);
> >
> > + virtio_pci_force_virtio_1(vpci_dev);
> > qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
> > }
> >
> >
> > Do you think this is the right approach or is there a better way to
> > solve this issue?
>
> We basically have three possible ways to deal with this:
>
> - Force it to modern (i.e., what you have been doing; would need the
> equivalent changes in ccw as well.)
Oo, thanks for pointing out ccw!
I don't know ccw well, in this case should we set dev->max_rev to 1 or 2
to force to modern?
> Pro: looks like the cleanest approach.
> Con: not sure if we would need backwards compatibility support,
> which looks hairy.
Not sure too.
> - Add vsock to the list of devices with legacy support.
> Pro: Existing setups continue to work.
> Con: If vsock is really virtio-1-only, we still carry around
> possibly broken legacy support.
I'm not sure it is virtio-1-only, but virtio-vsock was introduced in
2016, so I supposed it is modern-only.
How can I verify that? Maybe forcing legacy mode and run some tests.
> - Do nothing, have users force legacy off. Bad idea, as ccw has no way
> to do that on the command line.
>
> The first option is probably best.
>
Yeah, I agree with you!
Thanks,
Stefano
