Re: [Qemu-devel] [PATCH RFC qom-next 4/4] pcie_port: Turn PCIEPort and PCIESlot into abstract QOM types

[Michael S. Tsirkin]

On Sun, Jul 28, 2013 at 02:36:33PM +0200, Andreas Färber wrote:
> Hi Anthony,
> 
> 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.
> > 
> > Expecting someone to know the type hierarchy by heart such that this
> > doesn't look like a bug is too much IMHO.
> 
> I have changed v2 not to mix different-but-compatible struct types in
> one VMStateDescription.
> 
> Could you clarify if that was what you meant with the above?
> 
> 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?
> 
> I'm thinking that writing VMSTATE_PCIE_DEVICE() already clearly
> indicates the developer knows the device inherits from TYPE_PCI_DEVICE.
> 
> 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.
> 
> 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?
> 
> 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-inheritence.

Maybe combine them and use is_express to select the correct
format.

> SHPC_VMSTATE() seems to be another macro beyond VMSTATE_MSIX() operating
> on PCIDevice but placed in an individual device (pci-bridge-dev). Can it
> be turned into a subsection, Michael?
> 
> Regards,
> Andreas

Not without breaking cross-version migration I think?

> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg