migration, PCIE ports, and parent_obj.parent_obj.parent_obj

migration, PCIE ports, and parent_obj.parent_obj.parent_obj

[Peter Maydell]

In reviewing a new pcie host bridge, I noticed (because this new device
copied existing code) that the vmstate structs for pcie root ports do
this in their vmstate structs:

    .fields = (const VMStateField[]) {
        VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
        VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
                       PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
        VMSTATE_MSIX(parent_obj.parent_obj.parent_obj.parent_obj,
                     GenPCIERootPort),
        VMSTATE_END_OF_LIST()
    }

(this specific example is hw/pci-bridge/gen_pcie_root_port.c).

Devices shouldn't have to have ugly long chains of "parent_obj"
like this and shouldn't have to care what in their parents needs
migration; compare e.g. SCSI devices, which start with a
"VMSTATE_SCSI_DEVICE" that captures all the parent class's needs,
followed by only the state that the child added.

How can we get to something more like that for PCIE port classes?

thanks
-- PMM

Re: migration, PCIE ports, and parent_obj.parent_obj.parent_obj

[Michael S. Tsirkin]

On Mon, Oct 26, 2026 at 04:11:57PM +0000, Peter Maydell wrote:
> In reviewing a new pcie host bridge, I noticed (because this new device
> copied existing code) that the vmstate structs for pcie root ports do
> this in their vmstate structs:
> 
>     .fields = (const VMStateField[]) {
>         VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
>         VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
>                        PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
>         VMSTATE_MSIX(parent_obj.parent_obj.parent_obj.parent_obj,
>                      GenPCIERootPort),
>         VMSTATE_END_OF_LIST()
>     }
> 
> (this specific example is hw/pci-bridge/gen_pcie_root_port.c).
> 
> Devices shouldn't have to have ugly long chains of "parent_obj"
> like this and shouldn't have to care what in their parents needs
> migration; compare e.g. SCSI devices, which start with a
> "VMSTATE_SCSI_DEVICE" that captures all the parent class's needs,
> followed by only the state that the child added.
> 
> How can we get to something more like that for PCIE port classes?
> 
> thanks
> -- PMM

Yea makes total sense. Anyone has the bandwidth to take a stab?
I'm buried under a pile of ai generated security reports.

-- 
MST

Re: migration, PCIE ports, and parent_obj.parent_obj.parent_obj

[Peter Xu]

On Mon, Oct 26, 2026 at 12:19:07PM -0400, Michael S. Tsirkin wrote:
> On Mon, Oct 26, 2026 at 04:11:57PM +0000, Peter Maydell wrote:
> > In reviewing a new pcie host bridge, I noticed (because this new device
> > copied existing code) that the vmstate structs for pcie root ports do
> > this in their vmstate structs:
> > 
> >     .fields = (const VMStateField[]) {
> >         VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> >         VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> >                        PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> >         VMSTATE_MSIX(parent_obj.parent_obj.parent_obj.parent_obj,
> >                      GenPCIERootPort),
> >         VMSTATE_END_OF_LIST()
> >     }
> > 
> > (this specific example is hw/pci-bridge/gen_pcie_root_port.c).
> > 
> > Devices shouldn't have to have ugly long chains of "parent_obj"
> > like this and shouldn't have to care what in their parents needs
> > migration; compare e.g. SCSI devices, which start with a
> > "VMSTATE_SCSI_DEVICE" that captures all the parent class's needs,
> > followed by only the state that the child added.
> > 
> > How can we get to something more like that for PCIE port classes?
> > 
> > thanks
> > -- PMM
> 
> Yea makes total sense. Anyone has the bandwidth to take a stab?
> I'm buried under a pile of ai generated security reports.

/me buried too on other things downstream.  I'm prepared to not be able to
work much upstream until after forum..

A quick look is this is not too bad yet:

$ git grep parent_obj.parent_obj | wc -l
20
$ git grep parent_obj.parent_obj.parent_obj | wc -l
11
(and, some of above lines are part of vm static checker script)

Looks not too bad yet? Still, I'll add a todo at least to myself (the other
place is https://wiki.qemu.org/ToDo/LiveMigration, feel free to update if
one sees it fit).

Thanks,

-- 
Peter Xu

Re: migration, PCIE ports, and parent_obj.parent_obj.parent_obj

[Peter Maydell]

On Mon, 26 Oct 2026 at 17:39, Peter Xu <peterx@redhat.com> wrote:
>
> On Mon, Oct 26, 2026 at 12:19:07PM -0400, Michael S. Tsirkin wrote:
> > On Mon, Oct 26, 2026 at 04:11:57PM +0000, Peter Maydell wrote:
> > > In reviewing a new pcie host bridge, I noticed (because this new device
> > > copied existing code) that the vmstate structs for pcie root ports do
> > > this in their vmstate structs:
> > >
> > >     .fields = (const VMStateField[]) {
> > >         VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > >         VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > >                        PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > >         VMSTATE_MSIX(parent_obj.parent_obj.parent_obj.parent_obj,
> > >                      GenPCIERootPort),
> > >         VMSTATE_END_OF_LIST()
> > >     }
> > >
> > > (this specific example is hw/pci-bridge/gen_pcie_root_port.c).
> > >
> > > Devices shouldn't have to have ugly long chains of "parent_obj"
> > > like this and shouldn't have to care what in their parents needs
> > > migration; compare e.g. SCSI devices, which start with a
> > > "VMSTATE_SCSI_DEVICE" that captures all the parent class's needs,
> > > followed by only the state that the child added.
> > >
> > > How can we get to something more like that for PCIE port classes?
> > >
> > > thanks
> > > -- PMM
> >
> > Yea makes total sense. Anyone has the bandwidth to take a stab?
> > I'm buried under a pile of ai generated security reports.
>
> /me buried too on other things downstream.  I'm prepared to not be able to
> work much upstream until after forum..
>
> A quick look is this is not too bad yet:
>
> $ git grep parent_obj.parent_obj | wc -l
> 20
> $ git grep parent_obj.parent_obj.parent_obj | wc -l
> 11
> (and, some of above lines are part of vm static checker script)
>
> Looks not too bad yet?

Depends on your definition of "bad" :-)  This is basically
the only vmstate that's doing this thing, and it would be
nice to correct it before we add more devices that end up
following the same pattern.

I agree it's not super urgent; I posted mostly because I have
no idea what the right fix is (especially given we want to
retain migration compat here).

thanks
-- PMM

Re: migration, PCIE ports, and parent_obj.parent_obj.parent_obj

[Peter Xu]

On Mon, Oct 26, 2026 at 07:10:21PM +0000, Peter Maydell wrote:
> On Mon, 26 Oct 2026 at 17:39, Peter Xu <peterx@redhat.com> wrote:
> >
> > On Mon, Oct 26, 2026 at 12:19:07PM -0400, Michael S. Tsirkin wrote:
> > > On Mon, Oct 26, 2026 at 04:11:57PM +0000, Peter Maydell wrote:
> > > > In reviewing a new pcie host bridge, I noticed (because this new device
> > > > copied existing code) that the vmstate structs for pcie root ports do
> > > > this in their vmstate structs:
> > > >
> > > >     .fields = (const VMStateField[]) {
> > > >         VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> > > >         VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
> > > >                        PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
> > > >         VMSTATE_MSIX(parent_obj.parent_obj.parent_obj.parent_obj,
> > > >                      GenPCIERootPort),
> > > >         VMSTATE_END_OF_LIST()
> > > >     }
> > > >
> > > > (this specific example is hw/pci-bridge/gen_pcie_root_port.c).
> > > >
> > > > Devices shouldn't have to have ugly long chains of "parent_obj"
> > > > like this and shouldn't have to care what in their parents needs
> > > > migration; compare e.g. SCSI devices, which start with a
> > > > "VMSTATE_SCSI_DEVICE" that captures all the parent class's needs,
> > > > followed by only the state that the child added.
> > > >
> > > > How can we get to something more like that for PCIE port classes?
> > > >
> > > > thanks
> > > > -- PMM
> > >
> > > Yea makes total sense. Anyone has the bandwidth to take a stab?
> > > I'm buried under a pile of ai generated security reports.
> >
> > /me buried too on other things downstream.  I'm prepared to not be able to
> > work much upstream until after forum..
> >
> > A quick look is this is not too bad yet:
> >
> > $ git grep parent_obj.parent_obj | wc -l
> > 20
> > $ git grep parent_obj.parent_obj.parent_obj | wc -l
> > 11
> > (and, some of above lines are part of vm static checker script)
> >
> > Looks not too bad yet?
> 
> Depends on your definition of "bad" :-)  This is basically
> the only vmstate that's doing this thing, and it would be
> nice to correct it before we add more devices that end up
> following the same pattern.

I agree.

> 
> I agree it's not super urgent; I posted mostly because I have
> no idea what the right fix is (especially given we want to
> retain migration compat here).

I confess I didn't even try to think further than counting lines. :(
I'll try some day.

Thanks,

-- 
Peter Xu