From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ntju8-0004gs-Rt for qemu-devel@nongnu.org; Mon, 22 Mar 2010 11:51:00 -0400 Received: from [199.232.76.173] (port=33296 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ntju8-0004gY-9t for qemu-devel@nongnu.org; Mon, 22 Mar 2010 11:51:00 -0400 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1Ntju6-0001pQ-3V for qemu-devel@nongnu.org; Mon, 22 Mar 2010 11:51:00 -0400 Received: from mail-bw0-f218.google.com ([209.85.218.218]:42237) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Ntju5-0001ow-Da for qemu-devel@nongnu.org; Mon, 22 Mar 2010 11:50:57 -0400 Received: by bwz10 with SMTP id 10so7724876bwz.2 for ; Mon, 22 Mar 2010 08:50:53 -0700 (PDT) Message-ID: <4BA791D6.805@codemonkey.ws> Date: Mon, 22 Mar 2010 10:50:46 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups References: <4BA6DB3F.7010702@codemonkey.ws> <201003221330.02217.paul@codesourcery.com> <4BA7835F.10906@codemonkey.ws> <20100322145013.GA19447@redhat.com> <4BA786C1.1030105@codemonkey.ws> <20100322151742.GA19675@redhat.com> In-Reply-To: <20100322151742.GA19675@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Juan Quintela , Paul Brook , qemu-devel@nongnu.org On 03/22/2010 10:17 AM, Michael S. Tsirkin wrote: > 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? > The problem is, VirtIODevice needs to interact with VirtIOPCI. If you do this as: VirtIOBlock -> VirtIOPCIBlock VirtIOPCIDevice -> Then you need to redirect through the hierarchy. It gets messy pretty quickly. That's sort of what we do with VirtIOPCIProxy today and it's pretty ugly. >> 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 ... > But look at the lguest virtio implement. We would definitely model a VirtIOBus if we implemented something like that in qemu. VirtIO really is designed to be a bus. >>>> 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. > There would be a nvectors property of virtio-pci, you'd create a virtio-pci device, set nvectors, and add a VirtIODevice to it. Sounds obtuse from a user's perspective so we'd want to simplify the syntax. But in terms of internal modelling, it really simplifies things tremendously. Regards, Anthony Liguori