From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0vRf-0001KZ-5o for qemu-devel@nongnu.org; Mon, 13 Aug 2012 10:16:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0vRZ-00065K-9l for qemu-devel@nongnu.org; Mon, 13 Aug 2012 10:16:39 -0400 Received: from mail-qa0-f45.google.com ([209.85.216.45]:53668) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0vRZ-00064w-5o for qemu-devel@nongnu.org; Mon, 13 Aug 2012 10:16:33 -0400 Received: by qadc10 with SMTP id c10so2050976qad.4 for ; Mon, 13 Aug 2012 07:16:32 -0700 (PDT) From: Anthony Liguori In-Reply-To: <20120813131659.GE16801@redhat.com> References: <1343872026-18189-1-git-send-email-afaerber@suse.de> <1343872026-18189-15-git-send-email-afaerber@suse.de> <20120813131659.GE16801@redhat.com> Date: Mon, 13 Aug 2012 09:16:30 -0500 Message-ID: <87obme5135.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Andreas =?utf-8?Q?F=C3=A4rber?= Cc: qemu-devel@nongnu.org, Alexander Graf , Andreas =?utf-8?Q?F=C3=A4rber?= , "open list:New World" , Scott Wood , David Gibson "Michael S. Tsirkin" writes: > On Thu, Aug 02, 2012 at 03:47:06AM +0200, Andreas F=C3=A4rber wrote: >> Uglify the parent field to enforce QOM-style access via casts. >> Don't just typedef PCIHostState, either use it directly or embed it. >>=20 >> Signed-off-by: Andreas F=C3=A4rber > > > IMHO only one chunk from this patch should be applied (below). > Below it is split out but needs to be rebased on top of patches > 1-13. I understand what your objection is but it's unreasonable IMHO. The purpose of QOM is to bring consistency across large swaths of code in QEMU that have historically done things there own way. This means expressing concepts like inheritence and casting in the same way across the board. The common way (the QOM way) is to make the parent type the first member of the struct (typically named parent or parent_obj) and then to use cast macros to upcast and downcast. This patch is 100% correct in that regard and I'm going to apply it once Andreas makes the change I requested. For my part, I'm long over due in writing up a device authoring style guide that I promised a few weeks ago. I'll write that up this afternoon and send it out today. We can debate the merits of this sort of thing in the style guide. Regards, Anthony Liguori > > --> > > From: Andreas F=C3=A4rber > > piix: minor code simplification > > There's no need to deal with qdev internals in piix - we get device > state from qdev_create so just use that. > > Signed-off-by: Andreas F=C3=A4rber > Signed-off-by: Michael S. Tsirkin > > --- > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > index c497a01..18554a6 100644 > --- a/hw/piix_pci.c > +++ b/hw/piix_pci.c > @@ -274,7 +274,7 @@ static PCIBus *i440fx_common_init(const char *device_= name, > dev =3D qdev_create(NULL, "i440FX-pcihost"); > s =3D FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev)); > s->address_space =3D address_space_mem; > - b =3D pci_bus_new(&s->busdev.qdev, NULL, pci_address_space, > + b =3D pci_bus_new(&dev, NULL, pci_address_space, > address_space_io, 0); > s->bus =3D b; > object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev),= NULL);