Re: [libvirt] [PATCH 5/5] qemu: Avoid duplicate resume events and state changes

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

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"

(or you could just do the *StartCPUs again with the RESUMED_MIGRATED and close your eyes ;-)).

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.

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.