From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3QWl-0004A3-Bx for qemu-devel@nongnu.org; Sun, 28 Jul 2013 08:56:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V3QWf-0005cx-Ck for qemu-devel@nongnu.org; Sun, 28 Jul 2013 08:56:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17475) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3QWf-0005cm-3r for qemu-devel@nongnu.org; Sun, 28 Jul 2013 08:56:41 -0400 Date: Sun, 28 Jul 2013 15:58:03 +0300 From: "Michael S. Tsirkin" Message-ID: <20130728125803.GC16177@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> <51F51051.7050801@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <51F51051.7050801@suse.de> 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: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Paolo Bonzini , qemu-devel@nongnu.org, Anthony Liguori , Juan Quintela On Sun, Jul 28, 2013 at 02:36:33PM +0200, Andreas F=E4rber wrote: > Hi Anthony, >=20 > Am 22.07.2013 22:29, schrieb Anthony Liguori: > > 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 > I have changed v2 not to mix different-but-compatible struct types in > one VMStateDescription. >=20 > Could you clarify if that was what you meant with the above? >=20 > Or would you also be opposed to - post-1.6 - changing > VMSTATE_PCIE_DEVICE(parent_obj[.parent_obj], MyStruct) > to > VMSTATE_PCIE_DEVICE() > as suggested elsewhere in this thread? >=20 > I'm thinking that writing VMSTATE_PCIE_DEVICE() already clearly > indicates the developer knows the device inherits from TYPE_PCI_DEVICE. >=20 > All PCIe devices using VMSTATE_PCIE_DEVICE() today use it at an offset > of 0 and so do all PCI devices using VMSTATE_PCI_DEVICE() apparently. > VMSTATE_PCI_DEVICE_POINTER() would be unaffected, but is unused anyway. >=20 > My survey also concluded that luckily all VMSTATE_PCIE_DEVICE() and > VMSTATE_PCI_DEVICE() are placed as first VMStateField, so moving parent > state to its class might be possible, similar to qdev props todays with > class_base_init clearing it for derived types. > However this would require to either refactor core VMState code to > operate on a list, aggregated from one or more zero-terminated arrays, > which I would consider invasive and error-prone, or simply have Device > code allocate a new VMStateDescription before registering it in QOM > realize (so it can be free'd on unrealize). Thoughts? >=20 > Either way, it would work for CPU but not for PCI, since there are two > different macros, VMSTATE_PCI_DEVICE() and VMSTATE_PCIE_DEVICE() both > for PCIDeviceClass. Not sure how to solve that without multi-inheritenc= e. Maybe combine them and use is_express to select the correct format. > SHPC_VMSTATE() seems to be another macro beyond VMSTATE_MSIX() operatin= g > on PCIDevice but placed in an individual device (pci-bridge-dev). Can i= t > be turned into a subsection, Michael? >=20 > Regards, > Andreas Not without breaking cross-version migration I think? > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg