From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41756) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Te4YP-0000c0-T3 for qemu-devel@nongnu.org; Thu, 29 Nov 2012 08:53:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Te4YJ-00005T-OM for qemu-devel@nongnu.org; Thu, 29 Nov 2012 08:53:25 -0500 Received: from e28smtp03.in.ibm.com ([122.248.162.3]:49022) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Te4YJ-00005K-4w for qemu-devel@nongnu.org; Thu, 29 Nov 2012 08:53:19 -0500 Received: from /spool/local by e28smtp03.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 29 Nov 2012 19:23:15 +0530 Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qATDrC2u37486618 for ; Thu, 29 Nov 2012 19:23:12 +0530 Received: from d28av04.in.ibm.com (loopback [127.0.0.1]) by d28av04.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qATJN06n020793 for ; Fri, 30 Nov 2012 06:23:03 +1100 From: Anthony Liguori In-Reply-To: <50B75716.6030308@greensocs.com> References: <1353595852-30776-1-git-send-email-fred.konrad@greensocs.com> <1353595852-30776-2-git-send-email-fred.konrad@greensocs.com> <87k3t8qvrg.fsf@codemonkey.ws> <877gp8pafv.fsf@codemonkey.ws> <50B75716.6030308@greensocs.com> Date: Thu, 29 Nov 2012 07:52:47 -0600 Message-ID: <87k3t47byo.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konrad Frederic , Peter Maydell Cc: e.voevodin@samsung.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, stefanha@redhat.com, cornelia.huck@de.ibm.com, afaerber@suse.de Konrad Frederic writes: > On 26/11/2012 17:59, Anthony Liguori wrote: >> Peter Maydell writes: >> >>> On 26 November 2012 14:33, Anthony Liguori wrote: >>>> VirtioBusInfo is not a great name. This is a proxy class that allows >>>> for a device to implement the virtio bus interface. >>>> >>>> This could be done as an interface but since nothing else uses >>>> interfaces, I'm okay with something like this. But the first argument >>>> ought to be an opaque for all methods. >>> We have at least one user of Interface in the tree IIRC. >>> I'd much rather we did this the right way -- the only reason >>> it's the way Fred has coded it is that there's no obvious >>> body of code in the tree to copy, so we're thrashing around >>> a bit. If you tell us what the correct set of structs/classes/ >>> interfaces/etc is then we can implement it :-) >> I really think extending virtio-bus to a virtio-pci-bus and then >> initializing it with a link to the PCI device is the best approach. >> >> It's by far the simpliest approach in terms of coding. >> >> Did I explain it adequately? To recap: >> >> virtio-bus extends bus-state >> - implements everything that VirtIOBindings implements as methods >> >> virtio-pci-bus extends virtio-bus >> - is constructed with a pointer to a PCIDevice >> - implements the methods necessary to be a virtio bus > I still have trouble with that ^^. > > virtio-pci-bus extends virtio-bus so I put something like that : > > static void virtio_pci_bus_class_init(ObjectClass *klass, void *data) > { > BusClass *bc = BUS_CLASS(klass); > VirtioBusClass *k = VIRTIO_BUS_CLASS(klass); > /* VirtIOBindings */ > k->notify = virtio_pci_notify; > k->save_config = virtio_pci_save_config; > k->load_config = virtio_pci_load_config; > k->save_queue = virtio_pci_save_queue; > k->load_queue = virtio_pci_load_queue; > k->get_features = virtio_pci_get_features; > k->query_guest_notifiers = virtio_pci_query_guest_notifiers; > k->set_host_notifier = virtio_pci_set_host_notifier; > k->set_guest_notifiers = virtio_pci_set_guest_notifiers; > k->vmstate_change = virtio_pci_vmstate_change; > /* > * TODO : Init and exit function. > * void (*init)(void *opaque); > * void (*exit)(void *opaque); > */ > } > > static TypeInfo virtio_pci_bus_info = { > .name = TYPE_VIRTIO_PCI_BUS, > .parent = TYPE_VIRTIO_BUS, > .class_init = virtio_pci_bus_class_init, > }; > > and I have VirtioDevice which extends DeviceState like that : > > static void virtio_device_class_init(ObjectClass *klass, void *data) > { > /* Set the default value here. */ > DeviceClass *dc = DEVICE_CLASS(klass); > dc->bus_type = TYPE_VIRTIO_BUS; > dc->init = virtio_device_init; > } > > static TypeInfo virtio_device_info = { > .name = TYPE_VIRTIO_DEVICE, > .parent = TYPE_DEVICE, > .instance_size = sizeof(VirtioDeviceState), > /* Abstract the virtio-device */ > .class_init = virtio_device_class_init, > .abstract = true, > .class_size = sizeof(VirtioDeviceClass), > }; > > The problem is that the virtio devices can't be connected to the > virtio-pci-bus even if it extends virtio-bus because TYPE_VIRTIO_BUS != > TYPE_VIRTIO_PCI_BUS. That's just a bug. See the patch I just sent out to fix it. The type check is too strict in qdev_device_add(). Regards, Anthony Liguori > > Did I miss something ? > > Thanks, > > Fred > > >> virtio-device extends device-state >> - implements methods used by virtio-bus >> >> Regards, >> >> Anthony Liguori >> >>> -- PMM