[Top][All Lists]
[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
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, (continued)
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Jamie Lokier, 2010/03/18
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Michael S. Tsirkin, 2010/03/21
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Jamie Lokier, 2010/03/21
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Michael S. Tsirkin, 2010/03/21
- [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Juan Quintela, 2010/03/21
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Anthony Liguori, 2010/03/21
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Paul Brook, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Anthony Liguori, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Michael S. Tsirkin, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Anthony Liguori, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups,
Michael S. Tsirkin <=
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Anthony Liguori, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Paul Brook, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Anthony Liguori, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Paul Brook, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Anthony Liguori, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Paul Brook, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Michael S. Tsirkin, 2010/03/22
- [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Juan Quintela, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Paul Brook, 2010/03/22
- Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Anthony Liguori, 2010/03/22