Re: [PATCH 0/2] virtio: non-legacy device handling

[David Hildenbrand <david@redhat.com>]

On 20.07.20 11:03, Michael S. Tsirkin wrote:
> On Mon, Jul 20, 2020 at 10:09:57AM +0200, David Hildenbrand wrote:
>> On 07.07.20 12:54, Cornelia Huck wrote:
>>> As discussed in "virtio-fs: force virtio 1.x usage", it seems like
>>> a good idea to make sure that any new virtio device (which does not
>>> support legacy virtio) is indeed a non-transitional device, just to
>>> catch accidental misconfigurations. We can easily compile a list
>>> of virtio devices with legacy support and have transports verify
>>> in their plugged callbacks that legacy support is off for any device
>>> not in that list.
>>>
>>> Most new virtio devices force non-transitional already, so nothing
>>> changes for them. vhost-user-fs-pci even does not allow to configure
>>> a non-transitional device, so it is fine as well.
>>>
>>> One problematic device, however, is virtio-iommu-pci. It currently
>>> offers both the transitional and the non-transitional variety of the
>>> device, and does not force anything. I'm unsure whether we should
>>> consider transitional virtio-iommu unsupported, or if we should add
>>> some compat handling. (The support for legacy or not generally may
>>> change based upon the bus, IIUC, so I'm unsure how to come up with
>>> something generic.)
>>>
>>> Cornelia Huck (2):
>>>   virtio: list legacy-capable devices
>>>   virtio: verify that legacy support is not accidentally on
>>
>> I'd squash both patches. Looking at patch #1, I wonder why we don't
>> store that information along with the device implementation? What was
>> the motivation to define this information separately?
> 
> Because people seem to cut and paste code, so when one
> enables it in an old device, it gets pasted into a new one.
> With a list in a central place, it's easier to figure out
> what's going on.

Makes sense, I suggest adding that to the patch description.

Both patches look sane to me (- squashing them).

-- 
Thanks,

David / dhildenb