qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
Date: Mon, 22 Mar 2010 17:17:42 +0200
User-agent: Mutt/1.5.19 (2009-01-05)

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]