On Mon, Mar 22, 2010 at 10:03:29AM -0500, Anthony Liguori wrote: > On 03/22/2010 09:50 AM, Michael S. Tsirkin wrote: >> On Mon, Mar 22, 2010 at 09:49:03AM -0500, Anthony Liguori wrote: >> >>> On 03/22/2010 08:30 AM, Paul Brook wrote: >>> >>>>> A VirtIOBlock device cannot be a VirtIODevice while being a >>>>> VirtIOPCIProxy (proxy is a poor name, btw). >>>>> >>>>> It really ought to be: >>>>> >>>>> DeviceState -> VirtIODevice -> VirtIOBlock >>>>> >>>>> and: >>>>> >>>>> PCIDevice -> VirtIOPCI : implements VirtIOBus >>>>> >>>>> The interface between the VirtIODevice and VirtIOBus is the virtio >>>>> transport. >>>>> >>>>> The main reason a separate bus is needed is the same reason it's needed >>>>> in Linux. VirtIOBlock has to be tied to some bus. It cannot be tied to >>>>> the PCI bus without having it be part of the implementation detail. >>>>> Introducing another bus type fixes this (and it's what we do in the >>>>> kernel). >>>>> >>>>> >>>> Why does virtio need a device state and bus at all? >>>> >>>> >>> Because you need VirtIOBlock to have qdev properties that can be set. >>> >>> You also need VirtIOPCI to have separate qdev properties that can be set. >>> >>> >>>> Can't it just be an internal implementation interface, which happens to be >>>> used by all devices that happen to exposed a block device over a virtio >>>> transport? >>>> >>>> >>> Theoretically, yes, but given the rest of the infrastructure's >>> interaction with qdev, making it a device makes the most sense IMHO. >>> >> Does this boil down to qdev wanting to be the 1st field >> in the structure, again? We can just lift that limitation. >> > > No, I don't think it's relevant at all. > > It's a classic OOP problem. > > VirtIOBlock is-a VirtIODevice, VirtIODevice is-a DeviceState > > VirtIOPCI is-a PCIDevice, PCIDevice is-a Device State. > > But VirtIODevice is-a VirtIOPCI device isn't always true so it can't be > an is-a relationship. Initially, this was true and that's why the code > was structured that way. Now that we have two type so of virtio > transports, we need to change the modelling. It needs to get inverted > into a has-a relationship. IOW, VirtIOPCI has-a VirtIODevice. > > When one device has-a one or more other devices, we model that as a Bus.
Hmm. Is anything wrong with VirtIOPCIBlock which would be both a VirtIOBlock and VirtIOPCI device? > It's just like SCSI. SCSIDisk is-a SCSIDevice, SCSIDevice is-a DeviceState. > > LSIState is-a PCIDevice, PCIDevice is-a DeviceState. > > LSIState has-a SCSIDevice because LSIState implements the SCSIBus interface. Yes but LSIState emulates a real HBI ... >>> I can't envision any reason why we would ever want to have two MSI >>> vectors for a given queue. >>> >> No. OTOH whether we want a shared vector or a per-vq vector >> might depend on the device being used. >> > > Yup. From a users perspective, we don't want them to create two > separate devices and manipulate parameters. We definitely want one > interface where we can manipulate both VirtIODevice and VirtIOPCI > parameters. > > Regards, > > Anthony Liguori Will a bus really help? Where would we put the # of vectors? I think this can't be a virtio-pci bus property as it might be different between different virtio pci devices. -- MST
