From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGSSt-0003vk-5i for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:38:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VGSSj-0000MZ-NW for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:38:39 -0400 Received: from cantor2.suse.de ([195.135.220.15]:48034 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGSSj-0000MM-Di for qemu-devel@nongnu.org; Mon, 02 Sep 2013 07:38:29 -0400 Message-ID: <522478B1.4030407@suse.de> Date: Mon, 02 Sep 2013 13:38:25 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1375057621-19961-1-git-send-email-afaerber@suse.de> <1375057621-19961-3-git-send-email-afaerber@suse.de> <20130902113628.GB20911@redhat.com> In-Reply-To: <20130902113628.GB20911@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom-next for-next v2 2/6] pci: Simplify VMSTATE_PCIE_DEVICE() macro List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, Anthony Liguori , Gerd Hoffmann Am 02.09.2013 13:36, schrieb Michael S. Tsirkin: > On Mon, Jul 29, 2013 at 02:26:57AM +0200, Andreas F=E4rber wrote: >> Drop the arguments to avoid QOM refactorings causing more churn. >> >> Signed-off-by: Andreas F=E4rber >=20 > We discussed and discarded something very similar. Anthony requested that I don't mix different types for the offsets in VMSTATE_*() macros. >=20 > This only works correctly if you put it in a struct which has the pci > device at offset 0, and there's no either runtime or compile-time check > to verify it's the correct type. As Anthony already said, QOM depends on the offset being 0, casts will fail otherwise. So there are runtime checks. >=20 > So please find some other way to do this. I did post an RFC, but that is significantly more invasive, since it affects not only PCI and needs a lot of review before we can move forward with it. Andreas > Or go back to give fields sane names instead > of "parent_obj". >=20 >=20 >> --- >> hw/pci-bridge/ioh3420.c | 2 +- >> hw/pci-bridge/xio3130_downstream.c | 2 +- >> hw/pci-bridge/xio3130_upstream.c | 2 +- >> hw/usb/hcd-xhci.c | 2 +- >> include/hw/pci/pcie.h | 6 +++--- >> 5 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c >> index e07c7e8..0657354 100644 >> --- a/hw/pci-bridge/ioh3420.c >> +++ b/hw/pci-bridge/ioh3420.c >> @@ -187,7 +187,7 @@ static const VMStateDescription vmstate_ioh3420 =3D= { >> .minimum_version_id_old =3D 1, >> .post_load =3D pcie_cap_slot_post_load, >> .fields =3D (VMStateField[]) { >> - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlo= t), >> + VMSTATE_PCIE_DEVICE(), >> VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log, >> PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog)= , >> VMSTATE_END_OF_LIST() >> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio313= 0_downstream.c >> index 2c84b1a..9022949 100644 >> --- a/hw/pci-bridge/xio3130_downstream.c >> +++ b/hw/pci-bridge/xio3130_downstream.c >> @@ -154,7 +154,7 @@ static const VMStateDescription vmstate_xio3130_do= wnstream =3D { >> .minimum_version_id_old =3D 1, >> .post_load =3D pcie_cap_slot_post_load, >> .fields =3D (VMStateField[]) { >> - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlo= t), >> + VMSTATE_PCIE_DEVICE(), >> VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log, >> PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog)= , >> VMSTATE_END_OF_LIST() >> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_= upstream.c >> index 82add15..046b790 100644 >> --- a/hw/pci-bridge/xio3130_upstream.c >> +++ b/hw/pci-bridge/xio3130_upstream.c >> @@ -133,7 +133,7 @@ static const VMStateDescription vmstate_xio3130_up= stream =3D { >> .minimum_version_id =3D 1, >> .minimum_version_id_old =3D 1, >> .fields =3D (VMStateField[]) { >> - VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort), >> + VMSTATE_PCIE_DEVICE(), >> VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0= , >> vmstate_pcie_aer_log, PCIEAERLog), >> VMSTATE_END_OF_LIST() >> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c >> index 9ba3e3e..a22861f 100644 >> --- a/hw/usb/hcd-xhci.c >> +++ b/hw/usb/hcd-xhci.c >> @@ -3544,7 +3544,7 @@ static const VMStateDescription vmstate_xhci =3D= { >> .version_id =3D 1, >> .post_load =3D usb_xhci_post_load, >> .fields =3D (VMStateField[]) { >> - VMSTATE_PCIE_DEVICE(parent_obj, XHCIState), >> + VMSTATE_PCIE_DEVICE(), >> VMSTATE_MSIX(parent_obj, XHCIState), >> =20 >> VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1, >> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h >> index c010007..fc4ebd3 100644 >> --- a/include/hw/pci/pcie.h >> +++ b/include/hw/pci/pcie.h >> @@ -132,12 +132,12 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offs= et, uint16_t nextfn); >> =20 >> extern const VMStateDescription vmstate_pcie_device; >> =20 >> -#define VMSTATE_PCIE_DEVICE(_field, _state) { = \ >> - .name =3D (stringify(_field)), = \ >> +#define VMSTATE_PCIE_DEVICE() { = \ >> + .name =3D "parent_obj", = \ >> .size =3D sizeof(PCIDevice), = \ >> .vmsd =3D &vmstate_pcie_device, = \ >> .flags =3D VMS_STRUCT, = \ >> - .offset =3D vmstate_offset_value(_state, _field, PCIDevice), = \ >> + .offset =3D 0, = \ >> } >> =20 >> #endif /* QEMU_PCIE_H */ >> --=20 >> 1.8.1.4 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg