Re: [PATCH v2 4/5] pci-ids: drop PCI_DEVICE_ID_VIRTIO_VSOCK

[Cornelia Huck]

On Tue, Oct 04 2022, Gerd Hoffmann <kraxel@redhat.com> wrote:

>> > diff --git a/hw/virtio/vhost-vsock-pci.c b/hw/virtio/vhost-vsock-pci.c
>> > index 9f34414d3814..170a806b6765 100644
>> > --- a/hw/virtio/vhost-vsock-pci.c
>> > +++ b/hw/virtio/vhost-vsock-pci.c
>> > @@ -65,8 +65,6 @@ static void vhost_vsock_pci_class_init(ObjectClass 
>> > *klass, void *data)
>> >     k->realize = vhost_vsock_pci_realize;
>> >     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>> >     device_class_set_props(dc, vhost_vsock_pci_properties);
>> > -    pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
>> > -    pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_VSOCK;
>> >     pcidev_k->revision = 0x00;
>> >     pcidev_k->class_id = PCI_CLASS_COMMUNICATION_OTHER;
>> > }
>> 
>> Could we have migration issues with this change?
>> 
>> This reminded me that we've had issues already with vsock being incorrectly
>> exported as legacy, that we discovered when we added commit 9b3a35ec82
>> ("virtio: verify that legacy support is not accidentally on").
>> 
>> Then we needed commit d55f518248 ("virtio: skip legacy support check on
>> machine types less than 5.1") to avoid migration issues.
>> 
>> And we merged the following commits to force 1.0 in virtio-vsock devices for
>> machine types >= 5.1 :
>> - 6209070503 ("vhost-vsock-pci: force virtio version 1")
>> - 27eda699f5 ("vhost-user-vsock-pci: force virtio version 1")
>
> Oh, the virtio_pci_force_virtio_1() call is conditional.  Hmm.
>
> The change will break vsock devices in legacy/transitional mode.  So, if
> that is allowed for old machine types for backward compatibility reasons
> I guess I should better drop this patch.

Maybe add a comment to prevent others from falling into the same trap?