On Wed, Sep 19, 2018 at 16:04:13 -0400, John Ferlan wrote:
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 825a9d399b..4771f26938 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
>
> [...]
>
> >
> > @@ -5074,13 +5060,8 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
> > goto endjob;
> > }
> >
> > - if (inPostCopy) {
> > + if (inPostCopy)
> > doKill = false;
> > - event = virDomainEventLifecycleNewFromObj(vm,
> > - VIR_DOMAIN_EVENT_RESUMED,
> > - VIR_DOMAIN_EVENT_RESUMED_POSTCOPY);
> > - virObjectEventStateQueue(driver->domainEventState, event);
> > - }
> > }
> >
> > if (mig->jobInfo) {
> > @@ -5111,11 +5092,6 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
> >
> > dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id);
> >
> > - event = virDomainEventLifecycleNewFromObj(vm,
> > - VIR_DOMAIN_EVENT_RESUMED,
> > - VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
> > - virObjectEventStateQueue(driver->domainEventState, event);
> > -
> > if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
> > virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER);
> > event = virDomainEventLifecycleNewFromObj(vm,
>
> For this path 2 events were generated if @inPostCopy == True - one right
> after the qemuProcessStartCPUs for RESUMED_POSTCOPY and then one for
> SUSPENDED_PAUSED once qemuMigrationDstWaitForCompletion occurred.
I guess you meant s/SUSPENDED_PAUSED/RESUMED_MIGRATED/. Anyway, you made
me think about this and you're right. Or docs even say that two RESUMED
events are sent during post-copy migration. RESUMED_POSTCOPY when the
domain switches from source to the destination host and RESUMED_MIGRATED
once migration is complete... v2 is in progress.
> IIUC, the changes here when @inPostCopy == true would only generate the
> POSTCOPY event, but not the MIGRATED event. Or is there something subtle
> I'm missing. Will the post copy processing generate two resume events?
> If so, we perhaps should document that. If not, then perhaps this second
> event that's being deleted needs to be moved inside that inPostCopy
> condition just above with the note that this one case is "abnormal"
Yes.
> (or you could just do the *StartCPUs again with the RESUMED_MIGRATED
> and close your eyes ;-)).
This wouldn't work, the RESUME handler ignores the event if the domain
is not paused.
> Curious, for the future, would it be useful to change the FakeReboot
> path to generate a different detail reason? It's hard to distinguish
> between someone using virsh resume (or recovery from Save, Dump,
> Snapshot failure) from that fake reboot path - at least from just the
> event. I guess it seems VIR_DOMAIN_EVENT_RESUMED_UNPAUSED is just too
> generic since it's more than just a "normal resume due to admin
> unpause", but that I would think would be extra given the "scope" of
> this particular problem.
Yeah, a new resume reason would be nice in this case, but I don't think
it's a big issue. The guest-agent driven reboot is much better and
hopefully used more often than fake reboot.
> Second curiosity... would it be useful/easy to generate an event on the
> failure scenario for StartCPUs? Knowing from whence we were called, we
> should be able to generate a suspended event rather than having the
> callers decide.
Not really, if StartCPUs fails the vCPUs just remain paused as if
nothing happened. That is, there's no state change to signal using an
event. And I think "cont" can't really fail anyway. Normally it would
just succeed and another STOP event would be sent right away in case the
reason for pausing the CPUs (such as I/O error) is still valid.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
