From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50624) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1WdF-0003Pw-Sc for qemu-devel@nongnu.org; Tue, 23 Jul 2013 03:03:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V1WdB-0003hs-GQ for qemu-devel@nongnu.org; Tue, 23 Jul 2013 03:03:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44604) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1WdB-0003hJ-8m for qemu-devel@nongnu.org; Tue, 23 Jul 2013 03:03:33 -0400 Date: Tue, 23 Jul 2013 10:04:52 +0300 From: "Michael S. Tsirkin" Message-ID: <20130723070452.GA11106@redhat.com> References: <1374415744-6675-1-git-send-email-afaerber@suse.de> <1374415744-6675-5-git-send-email-afaerber@suse.de> <20130721202607.GA15187@redhat.com> <87y58y49tx.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <87y58y49tx.fsf@codemonkey.ws> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Andreas =?iso-8859-1?Q?F=E4rber?= , qemu-devel@nongnu.org On Mon, Jul 22, 2013 at 03:29:46PM -0500, Anthony Liguori wrote: > "Michael S. Tsirkin" writes: >=20 > > On Sun, Jul 21, 2013 at 04:09:04PM +0200, Andreas F=E4rber wrote: > >> Signed-off-by: Andreas F=E4rber > >> --- > >> hw/pci-bridge/ioh3420.c | 23 ++++++++++------------- > >> hw/pci-bridge/xio3130_downstream.c | 23 ++++++++++------------- > >> hw/pci-bridge/xio3130_upstream.c | 15 +++++++-------- > >> hw/pci/pcie_port.c | 22 ++++++++++++++++++++++ > >> include/hw/pci/pcie_port.h | 14 ++++++++++++-- > >> 5 files changed, 61 insertions(+), 36 deletions(-) > >>=20 > >> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c > >> index 728f658..83db054 100644 > >> --- a/hw/pci-bridge/ioh3420.c > >> +++ b/hw/pci-bridge/ioh3420.c > >> @@ -92,9 +92,8 @@ static void ioh3420_reset(DeviceState *qdev) > >> =20 > >> static int ioh3420_initfn(PCIDevice *d) > >> { > >> - PCIBridge *br =3D PCI_BRIDGE(d); > >> - PCIEPort *p =3D DO_UPCAST(PCIEPort, br, br); > >> - PCIESlot *s =3D DO_UPCAST(PCIESlot, port, p); > >> + PCIEPort *p =3D PCIE_PORT(d); > >> + PCIESlot *s =3D PCIE_SLOT(d); > >> int rc; > >> =20 > >> rc =3D pci_bridge_initfn(d, TYPE_PCIE_BUS); > >> @@ -148,9 +147,7 @@ err_bridge: > >> =20 > >> static void ioh3420_exitfn(PCIDevice *d) > >> { > >> - PCIBridge *br =3D PCI_BRIDGE(d); > >> - PCIEPort *p =3D DO_UPCAST(PCIEPort, br, br); > >> - PCIESlot *s =3D DO_UPCAST(PCIESlot, port, p); > >> + PCIESlot *s =3D PCIE_SLOT(d); > >> =20 > >> pcie_aer_exit(d); > >> pcie_chassis_del_slot(s); > >> @@ -180,7 +177,7 @@ PCIESlot *ioh3420_init(PCIBus *bus, int devfn, b= ool multifunction, > >> qdev_prop_set_uint16(qdev, "slot", slot); > >> qdev_init_nofail(qdev); > >> =20 > >> - return DO_UPCAST(PCIESlot, port, DO_UPCAST(PCIEPort, br, br)); > >> + return PCIE_SLOT(d); > >> } > >> =20 > >> static const VMStateDescription vmstate_ioh3420 =3D { > >> @@ -190,19 +187,19 @@ static const VMStateDescription vmstate_ioh342= 0 =3D { > >> .minimum_version_id_old =3D 1, > >> .post_load =3D pcie_cap_slot_post_load, > >> .fields =3D (VMStateField[]) { > >> - VMSTATE_PCIE_DEVICE(port.br.parent_obj, PCIESlot), > >> - VMSTATE_STRUCT(port.br.parent_obj.exp.aer_log, PCIESlot, 0, > >> + VMSTATE_PCIE_DEVICE(parent_obj, PCIBridge), > >> + VMSTATE_STRUCT(exp.aer_log, PCIDevice, 0, > >> vmstate_pcie_aer_log, PCIEAERLog), > >> VMSTATE_END_OF_LIST() > >> } > >> }; > >> =20 > >> static Property ioh3420_properties[] =3D { > >> - DEFINE_PROP_UINT8("port", PCIESlot, port.port, 0), > >> + DEFINE_PROP_UINT8("port", PCIEPort, port, 0), > >> DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0), > >> DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0), > >> - DEFINE_PROP_UINT16("aer_log_max", PCIESlot, > >> - port.br.parent_obj.exp.aer_log.log_max, > >> + DEFINE_PROP_UINT16("aer_log_max", PCIDevice, > >> + exp.aer_log.log_max, > >> PCIE_AER_LOG_MAX_DEFAULT), > >> DEFINE_PROP_END_OF_LIST(), > >> }; > > > > > > This looks scary. This does a cast to different types > > without any checks at all. >=20 > What cast? >=20 > VMstate takes a void *. >=20 > One an object is cast to a void *, it's just as much as PCIESlot as it > is a PCIEPort. >=20 > That said, for consistency, I think having everything be relatively to > *one* type for a Property list is pretty helpful. >=20 > Expecting someone to know the type hierarchy by heart such that this > doesn't look like a bug is too much IMHO. >=20 > Regards, >=20 > Anthony Liguori Exactly.