VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT was defined but not used anywhere in our event generation code. This fixes qemuDomainRevertToSnapshot to properly report why the domain was resumed. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10d6bca186..0c1afbfd4e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16273,7 +16273,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, detail); } else if (!was_running) { /* Transition 8 */ - detail = VIR_DOMAIN_EVENT_RESUMED; + detail = VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, detail); -- 2.19.0

Whenever we get the RESUME event from QEMU, we change the state of the affected domain to VIR_DOMAIN_RUNNING with VIR_DOMAIN_RUNNING_UNPAUSED reason. This is fine if the domain is resumed unexpectedly, but when we sent "cont" to QEMU we usually have a better reason for the state change. The better reason is used in qemuProcessStartCPUs which also sets the domain state to running if qemuMonitorStartCPUs reports success. Thus we may end up with two state updates in a row, but the final reason is correct. This patch is a preparation for dropping the state change done in qemuMonitorStartCPUs for which we need to pass the actual running reason to the RESUME event handler and use it there instead of VIR_DOMAIN_RUNNING_UNPAUSED. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_process.c | 23 +++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ecbe2e8533..461859e37f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -366,6 +366,10 @@ struct _qemuDomainObjPrivate { /* counter for generating node names for qemu disks */ unsigned long long nodenameindex; + + /* qemuProcessStartCPUs stores the reason for starting vCPUs here for the + * RESUME event handler to use it */ + virDomainRunningReason runningReason; }; # define QEMU_DOMAIN_PRIVATE(vm) \ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 417e47ec29..f79db1d4cf 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -712,21 +712,28 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virQEMUDriverPtr driver = opaque; virObjectEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv; + virDomainRunningReason reason = VIR_DOMAIN_RUNNING_UNPAUSED; virObjectLock(vm); - if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - qemuDomainObjPrivatePtr priv = vm->privateData; + priv = vm->privateData; + if (priv->runningReason != VIR_DOMAIN_RUNNING_UNKNOWN) { + reason = priv->runningReason; + priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; + } + + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { if (priv->gotShutdown) { VIR_DEBUG("Ignoring RESUME event after SHUTDOWN"); goto unlock; } - VIR_DEBUG("Transitioned guest %s out of paused into resumed state", - vm->def->name); + VIR_DEBUG("Transitioned guest %s out of paused into resumed state, " + "reason '%s'", + vm->def->name, virDomainRunningReasonTypeToString(reason)); - virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, - VIR_DOMAIN_RUNNING_UNPAUSED); + virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); @@ -3087,6 +3094,8 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, } VIR_FREE(priv->lockState); + priv->runningReason = reason; + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto release; @@ -3104,6 +3113,7 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, return ret; release: + priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) VIR_WARN("Unable to release lease on %s", vm->def->name); VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState)); @@ -5987,6 +5997,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, priv->monError = false; priv->monStart = 0; priv->gotShutdown = false; + priv->runningReason = VIR_DOMAIN_RUNNING_UNKNOWN; VIR_DEBUG("Updating guest CPU definition"); if (qemuProcessUpdateGuestCPU(vm->def, priv->qemuCaps, caps, flags) < 0) -- 2.19.0

The only place where VIR_DOMAIN_EVENT_RESUMED should be generated is the RESUME event handler to make sure we don't generate duplicate events or state changes. In the worse case the duplicity can revert or cover changes done by other event handlers. For example, after QEMU sent RESUME, BLOCK_IO_ERROR, and STOP events we could happily mark the domain as running and report VIR_DOMAIN_EVENT_RESUMED to registered clients. https://bugzilla.redhat.com/show_bug.cgi?id=1612943 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - keep VIR_DOMAIN_EVENT_RESUMED_MIGRATED event at the end of post-copy migration src/qemu/qemu_driver.c | 13 ----------- src/qemu/qemu_migration.c | 49 ++++++++++++++++----------------------- src/qemu/qemu_process.c | 10 ++++---- 3 files changed, 24 insertions(+), 48 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0c1afbfd4e..126c783a0f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1861,7 +1861,6 @@ static int qemuDomainResume(virDomainPtr dom) virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; - virObjectEventPtr event = NULL; int state; int reason; virQEMUDriverConfigPtr cfg = NULL; @@ -1900,9 +1899,6 @@ static int qemuDomainResume(virDomainPtr dom) "%s", _("resume operation failed")); goto endjob; } - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) goto endjob; @@ -1913,7 +1909,6 @@ static int qemuDomainResume(virDomainPtr dom) cleanup: virDomainObjEndAPI(&vm); - virObjectEventStateQueue(driver->domainEventState, event); virObjectUnref(cfg); return ret; } @@ -15983,7 +15978,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainDefPtr config = NULL; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; - bool was_running = false; bool was_stopped = false; qemuDomainSaveCookiePtr cookie; virCPUDefPtr origCPU = NULL; @@ -16174,7 +16168,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, priv = vm->privateData; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { /* Transitions 5, 6 */ - was_running = true; if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_FROM_SNAPSHOT, QEMU_ASYNC_JOB_START) < 0) @@ -16271,12 +16264,6 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, detail); - } else if (!was_running) { - /* Transition 8 */ - detail = VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT; - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - detail); } } break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 825a9d399b..67940330aa 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2982,14 +2982,10 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, virFreeError(orig_err); if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && - reason == VIR_DOMAIN_PAUSED_POSTCOPY) { + reason == VIR_DOMAIN_PAUSED_POSTCOPY) qemuMigrationAnyPostcopyFailed(driver, vm); - } else if (qemuMigrationSrcRestoreDomainState(driver, vm)) { - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_MIGRATED); - virObjectEventStateQueue(driver->domainEventState, event); - } + else + qemuMigrationSrcRestoreDomainState(driver, vm); qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, priv->job.migParams, priv->job.apiFlags); @@ -4624,11 +4620,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, priv->job.migParams, priv->job.apiFlags); - if (qemuMigrationSrcRestoreDomainState(driver, vm)) { - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_MIGRATED); - } + qemuMigrationSrcRestoreDomainState(driver, vm); qemuMigrationJobFinish(driver, vm); if (!virDomainObjIsActive(vm) && ret == 0) { @@ -4672,7 +4664,6 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver, unsigned long resource) { qemuDomainObjPrivatePtr priv = vm->privateData; - virObjectEventPtr event = NULL; int ret = -1; /* If we didn't start the job in the begin phase, start it now. */ @@ -4694,11 +4685,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver, nmigrate_disks, migrate_disks, migParams); if (ret < 0) { - if (qemuMigrationSrcRestoreDomainState(driver, vm)) { - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_MIGRATED); - } + qemuMigrationSrcRestoreDomainState(driver, vm); goto endjob; } @@ -4722,7 +4709,6 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver, cleanup: virDomainObjEndAPI(&vm); - virObjectEventStateQueue(driver->domainEventState, event); return ret; } @@ -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,10 +5092,20 @@ 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 (inPostCopy) { + /* The only RESUME event during post-copy migration is triggered by + * QEMU when the running domain moves from the source to the + * destination host, but then the migration keeps running until all + * modified memory is transferred from the source host. This will + * result in VIR_DOMAIN_EVENT_RESUMED with RESUMED_POSTCOPY detail. + * However, our API documentation says we need to fire another RESUMED + * event at the very end of migration with RESUMED_MIGRATED detail. + */ + 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); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a7d71962da..29b0ba1590 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -456,7 +456,6 @@ qemuProcessFakeReboot(void *opaque) virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverPtr driver = priv->driver; - virObjectEventPtr event = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED; int ret = -1, rc; @@ -493,9 +492,6 @@ qemuProcessFakeReboot(void *opaque) goto endjob; } priv->gotShutdown = false; - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_RESUMED_UNPAUSED); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) { VIR_WARN("Unable to save status on vm %s after state change", @@ -511,7 +507,6 @@ qemuProcessFakeReboot(void *opaque) if (ret == -1) ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE)); virDomainObjEndAPI(&vm); - virObjectEventStateQueue(driver->domainEventState, event); virObjectUnref(cfg); } @@ -3109,7 +3104,10 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm, if (ret < 0) goto release; - virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason); + /* The RESUME event handler will change the domain state with the reason + * saved in priv->runningReason and it will also emit corresponding domain + * lifecycle event. + */ cleanup: virObjectUnref(cfg); -- 2.19.0