Re: [Qemu-devel] [PULL 42/43] piix4: add acpi pci hotplug support

[Michael S. Tsirkin]

On Tue, Oct 15, 2013 at 04:50:24PM +0200, Paolo Bonzini wrote:
> Il 15/10/2013 16:35, Michael S. Tsirkin ha scritto:
> > On Tue, Oct 15, 2013 at 04:31:31PM +0200, Paolo Bonzini wrote:
> >> Il 14/10/2013 17:01, Michael S. Tsirkin ha scritto:
> >>> -        VMSTATE_STRUCT(pci0_status, PIIX4PMState, 2, vmstate_pci_status,
> >>> -                       struct pci_status),
> >>> +        VMSTATE_STRUCT_TEST(pci0_status, PIIX4PMState,
> >>> +                            vmstate_test_no_use_acpi_pci_hotplug,
> >>> +                            2, vmstate_pci_status,
> >>> +                            struct pci_status),
> >>
> >> There's no reason to remove this from the stream when a new machine type
> >> is in use.  You'll just send out zeroes.
> > 
> > Seemed cleaner not to.
> 
> It certainly would be if we had a self-descriptive migration stream format.
> 
> However, what we have is "send bytes, parse them on the destination,
> hope the format matches".  Hence, anything that makes the format less
> declarative adds to the complication and complicates debugging.  This is
> the same reason why I prefer a subsection for the new hotplug stuff---it
> makes the format more declarative and self-descriptive.
> 
> Paolo

I understand for the subsection but why is removing
useless bytes from there making it less descriptive?

> >>> +        VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState,
> >>> +                            vmstate_test_use_acpi_pci_hotplug),
> >>
> >> This works, but it is a bit different from other cases that are already
> >> present, which use a subsection.  It is a bit ugly because it looks like
> >> a version-1 field, but in fact it is not version 1.
> >>
> >> I'll let other people decide whether it's acceptable or not, but I'm
> >> leaning towards asking you to use a subsection.
> >>
> >> Paolo