Re: [Qemu-devel] [PATCH] virtio: Provide version-specific variants of virtio PCI devices

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Oct 17, 2018 at 12:10:04PM -0300, Eduardo Habkost wrote:
> On Wed, Oct 17, 2018 at 10:22:37AM +0100, Stefan Hajnoczi wrote:
> > On Fri, Oct 12, 2018 at 11:54:35PM -0300, Eduardo Habkost wrote:
> > > This patch adds separate device types for each of those virtio
> > > device flavors:
> > > 
> > > - virtio-*-pci: the existing multi-purpose device types
> > >   - Configurable using `disable-legacy` and `disable-modern`
> > >     properties
> > >   - Legacy driver support is automatically enabled/disabled
> > >     depending on the bus where it is plugged
> > >   - Supports Conventional PCI and PCI Express buses
> > >     (but Conventional PCI is incompatible with
> > >     disable-legacy=off)
> > >   - Changes PCI vendor/device IDs at runtime
> > > - virtio-*-pci-0.9: legacy virtio device
> > >   - Supports Conventional PCI buses only, because
> > >     it has a PIO BAR
> > > - virtio-*-pci-1.0-transitional: virtio-1.0 device supporting legacy drivers
> > >   - Supports Conventional PCI buses only, because
> > >     it has a PIO BAR
> > > - virtio-*-pci-1.0: modern-only
> > >   - Supports both Conventional PCI and PCI Express buses
> > 
> > If new device types are being created, is it time to decouple the VIRTIO
> > PCI transport from the actual VIRTIO device (blk, net, scsi) like we
> > already have with virtio-mmio?
> > 
> >   -device virtio-pci-1.0,id=virtio-pci-0
> >   -device virtio-blk,bus=virtio-pci-0,drive=drive0,serial=mydisk
> > 
> > That way we avoid lots of boilerplate code and an explosion of new
> > device types.
> 
> I don't think that would work.  This would in turn make
> virtio-pci-1.0 a chameleon device that changes PCI ID depending
> on the backend device plugged to it.
> 
> Also, PCI IDs are not the only information contained inside the
> virtio-pci subclasses.  See all the PCI-specific +
> device-type-specific knowledge encoded inside the class_init and
> instance_init functions of each virtio-pci subclass.

I think we need to draw the line somewhere. We can't make new
classes and bother users each time there is a behaviour
change in the device.

But I also think the transport/device split it also an internal virtio
thing, no one who has not read the spec cares about it.

> -- 
> Eduardo