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

[Michael S. Tsirkin]

On Wed, Oct 16, 2013 at 11:18:42AM -0700, Anthony Liguori wrote:
> On Wed, Oct 16, 2013 at 11:18 AM, Michael S. Tsirkin <address@hidden> wrote:
> > 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?
> 
> No.
> 
> There is always some reason to delay releases.  We have a release
> every quarter.  It's not a big deal to just wait for a feature for the
> next release.  That's the whole point of doing frequent releases.
> 
> > 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.
> 
> There is nothing special and the feedback you are getting is no
> different than any other series.
> 
> >
> >> 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.
> 
> Unit level testing.  IOW, something that gets run during 'make check'
> to verify that we're generating proper ACPI tables.
> 
> Regards,
> 
> Anthony Liguori

In particular I do *not* want to write a consumer for
the guest interfaces in qemu. bios has one,
duplicating it in qemu is not a good idea.

By far the best way to test this is to boot some guest,
download tables, then run iasl -d on them.

I used linux guest but that's not necessary,
we could do this by integrating kvm unit test code.
But if we do integrate it, I think it's better to merge
than fork.

Gleb, Paolo, what do you think? OK to merge kvm unit test
into qemu? It depends on qemu anyway, in-tree will make it easier.
Maybe someone's looking at this already?

If people agree I'll try to make sure this happens but I hope it's not a
precondition, it's a lot of work (and it won't help if I'm distracted
rebasing patches :) )


> > 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