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

[Michael S. Tsirkin]

On Wed, Oct 16, 2013 at 09:38:29AM -0700, Anthony Liguori wrote:
> On Tue, Oct 15, 2013 at 1:17 PM, Michael S. Tsirkin <address@hidden> wrote:
> > On Tue, Oct 15, 2013 at 09:27:33AM -0700, Anthony Liguori wrote:
> >> Paolo Bonzini <address@hidden> writes:
> >>
> >> > 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.
> >>
> >> Yes, removing tests is always a good thing.
> >>
> >> But I think subsections should always be used when they can.  We should
> >> not break compatibility (even if we technical don't guarantee it) unless
> >> we absolutely have to.
> >>
> >> Regards,
> >>
> >> Anthony Liguori
> >
> > OK so I can interpret this in 2 ways wrt bridge hotplug:
> > - it's in shape for 1.7 except the migration which should use
> >   subsections (and needs cross-version testing)
> > - it's not in shape for 1.7
> >
> > Can you tell me which it is please?
> 
> The code is not in shape.  Forget about the existence of 1.7.  Focus
> on getting the code right and it will make whatever release it is
> ready for.  If that's 1.7, great, but the fact that 1.7 is around the
> corner does not mean we're going to merge something that isn't ready
> just so it makes the release.

OK. Apropos 1.7, how about moving soft freeze and the release out
by a couple of weeks?
What with you moving over and the kvm forum, people
didn't have time to focus on it properly IMO.
In particular it's harder than usual to get reviews.

> Migration is one issue.

Right but what's special about this feature?
Almost anything we do affects migration in some way.

> As I said before, testing is another.  There
> really should be some test automation for this.
> 
> Regards,
> 
> Anthony Liguori

I'm not sure I understand what you are saying here.

If you just want testing hotplug, automation is there.

Automated testing for cross-version migration? that's not easy since you
need two versions around. I'll talk to autotest guys but don't hold your
breath.  But a bigger issue is that migration and hotplug don't work
well together in qemu ATM.

-- 
MST