Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change tests/qemumonitorjsontest.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index c3ee771cbb..2de282dcba 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2044,7 +2044,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationCapabilities(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - const char *cap; + size_t cap; g_auto(GStrv) caps = NULL; g_autoptr(virBitmap) bitmap = NULL; g_autoptr(virJSONValue) json = NULL; @@ -2054,6 +2054,10 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationCapabilities(const void *opaque) " {" " \"state\": false," " \"capability\": \"xbzrle\"" + " }," + " {" + " \"state\": true," + " \"capability\": \"events\"" " }" " ]," " \"id\": \"libvirt-22\"" @@ -2072,11 +2076,25 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationCapabilities(const void *opaque) &caps) < 0) return -1; - cap = qemuMigrationCapabilityTypeToString(QEMU_MIGRATION_CAP_XBZRLE); - if (!g_strv_contains((const char **) caps, cap)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "Expected capability %s is missing", cap); - return -1; + for (cap = 0; cap < QEMU_MIGRATION_CAP_LAST; cap++) { + const char *capStr = qemuMigrationCapabilityTypeToString(cap); + bool present = g_strv_contains((const char **) caps, capStr); + + switch (cap) { + case QEMU_MIGRATION_CAP_XBZRLE: + case QEMU_MIGRATION_CAP_EVENTS: + if (!present) { + VIR_TEST_VERBOSE("Expected capability %s is missing", capStr); + return -1; + } + break; + + default: + if (present) { + VIR_TEST_VERBOSE("Unexpected capability %s found", capStr); + return -1; + } + } } bitmap = virBitmapNew(QEMU_MIGRATION_CAP_LAST); -- 2.35.1

On Wed, Jun 01, 2022 at 14:49:03 +0200, Jiri Denemark wrote: [...] I presume this bit is still up for discussion, right? Currently with the RFC patch 81 you'd attempt to pause it but qemu will break anyways IIUC. If that is the case, this should for now document it properly. Ditto.

On Mon, Jun 06, 2022 at 14:35:10 +0200, Peter Krempa wrote: But you were right, the text documents what RFC patch 81 is doing and should be moved there. Jirka

The function never returns anything but zero. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_domain.c | 5 ++--- src/qemu/qemu_domain.h | 4 ++-- src/qemu/qemu_migration.c | 3 +-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6f358466c5..5dee9c6f26 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7475,7 +7475,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriver *driver, * The vm must be locked when any of the following cleanup functions is * called. */ -int +void qemuDomainCleanupAdd(virDomainObj *vm, qemuDomainCleanupCallback cb) { @@ -7486,7 +7486,7 @@ qemuDomainCleanupAdd(virDomainObj *vm, for (i = 0; i < priv->ncleanupCallbacks; i++) { if (priv->cleanupCallbacks[i] == cb) - return 0; + return; } VIR_RESIZE_N(priv->cleanupCallbacks, @@ -7494,7 +7494,6 @@ qemuDomainCleanupAdd(virDomainObj *vm, priv->ncleanupCallbacks, 1); priv->cleanupCallbacks[priv->ncleanupCallbacks++] = cb; - return 0; } void diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 547d85b5f9..ce2dba499c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -762,8 +762,8 @@ int qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, qemuDomainObjPrivate *priv, virQEMUDriverConfig *cfg); -int qemuDomainCleanupAdd(virDomainObj *vm, - qemuDomainCleanupCallback cb); +void qemuDomainCleanupAdd(virDomainObj *vm, + qemuDomainCleanupCallback cb); void qemuDomainCleanupRemove(virDomainObj *vm, qemuDomainCleanupCallback cb); void qemuDomainCleanupRun(virQEMUDriver *driver, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 274386d6d9..fee3e8826b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3071,8 +3071,7 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver, VIR_WARN("Unable to encode migration cookie"); } - if (qemuDomainCleanupAdd(vm, qemuMigrationDstPrepareCleanup) < 0) - goto stopjob; + qemuDomainCleanupAdd(vm, qemuMigrationDstPrepareCleanup); if (!(flags & VIR_MIGRATE_OFFLINE)) { virDomainAuditStart(vm, "migrated", true); -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 27 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 34ecc7e7a9..05fd64b1c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29249,6 +29249,28 @@ virDomainObjIsFailedPostcopy(virDomainObj *dom) } +bool +virDomainObjIsPostcopy(virDomainObj *dom, + virDomainJobOperation op) +{ + if (op != VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN && + op != VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT) + return false; + + if (op == VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN) { + return (dom->state.state == VIR_DOMAIN_PAUSED && + dom->state.reason == VIR_DOMAIN_PAUSED_POSTCOPY_FAILED) || + (dom->state.state == VIR_DOMAIN_RUNNING && + (dom->state.reason == VIR_DOMAIN_RUNNING_POSTCOPY || + dom->state.reason == VIR_DOMAIN_RUNNING_POSTCOPY_FAILED)); + } + + return dom->state.state == VIR_DOMAIN_PAUSED && + (dom->state.reason == VIR_DOMAIN_PAUSED_POSTCOPY || + dom->state.reason == VIR_DOMAIN_PAUSED_POSTCOPY_FAILED); +} + + void virDomainObjSetState(virDomainObj *dom, virDomainState state, int reason) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index da85c6ecd4..1efdb439ac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3889,6 +3889,10 @@ virDomainObjGetState(virDomainObj *obj, int *reason) bool virDomainObjIsFailedPostcopy(virDomainObj *obj) ATTRIBUTE_NONNULL(1); +bool +virDomainObjIsPostcopy(virDomainObj *dom, + virDomainJobOperation op) + ATTRIBUTE_NONNULL(1); virSecurityLabelDef * virDomainDefGetSecurityLabelDef(const virDomainDef *def, const char *model); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0c6dd4fa49..770dfe459a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -578,6 +578,7 @@ virDomainObjGetOneDefState; virDomainObjGetPersistentDef; virDomainObjGetState; virDomainObjIsFailedPostcopy; +virDomainObjIsPostcopy; virDomainObjNew; virDomainObjParseFile; virDomainObjParseNode; -- 2.35.1

It is used for saving job out of domain object. Just like virErrorPreserveLast is used for errors. Let's make the naming consistent as Restore would suggest different semantics. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_domainjob.c | 14 ++++++++++++-- src/qemu/qemu_domainjob.h | 4 ++-- src/qemu/qemu_process.c | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index cb20b798f7..1e5724b505 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -214,9 +214,19 @@ qemuDomainObjResetAsyncJob(qemuDomainJobObj *job) job->cb->resetJobPrivate(job->privateData); } + +/** + * qemuDomainObjPreserveJob + * @param obj domain with a job that needs to be preserved + * @param job structure where to store job details from @obj + * + * Saves the current job details from @obj to @job and resets the job in @obj. + * + * Returns 0 on success, -1 on failure. + */ int -qemuDomainObjRestoreJob(virDomainObj *obj, - qemuDomainJobObj *job) +qemuDomainObjPreserveJob(virDomainObj *obj, + qemuDomainJobObj *job) { qemuDomainObjPrivate *priv = obj->privateData; diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h index f67eace36c..2bbccf6329 100644 --- a/src/qemu/qemu_domainjob.h +++ b/src/qemu/qemu_domainjob.h @@ -158,8 +158,8 @@ void qemuDomainObjSetJobPhase(virDomainObj *obj, int phase); void qemuDomainObjSetAsyncJobMask(virDomainObj *obj, unsigned long long allowedJobs); -int qemuDomainObjRestoreJob(virDomainObj *obj, - qemuDomainJobObj *job); +int qemuDomainObjPreserveJob(virDomainObj *obj, + qemuDomainJobObj *job); void qemuDomainObjDiscardAsyncJob(virDomainObj *obj); void qemuDomainObjReleaseAsyncJob(virDomainObj *obj); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2f0167299f..18445855db 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8714,7 +8714,7 @@ qemuProcessReconnect(void *opaque) cfg = virQEMUDriverGetConfig(driver); priv = obj->privateData; - qemuDomainObjRestoreJob(obj, &oldjob); + qemuDomainObjPreserveJob(obj, &oldjob); if (oldjob.asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; if (oldjob.asyncJob == VIR_ASYNC_JOB_BACKUP && priv->backup) -- 2.35.1

When post-copy migration fails, we can't just abort the migration and resume the domain on the source host as it is already running on the destination host and no host has a complete state of the domain memory. Instead of the current approach of just marking the domain on both ends as paused/running with a post-copy failed sub state, we will keep the migration job active (even though the migration API will return failure) so that the state is more visible and we can better control what APIs can be called on the domains and even allow for resuming the migration. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - squashed in a one line change from "qemu: Implement VIR_MIGRATE_POSTCOPY_RESUME for Confirm phase" src/qemu/qemu_migration.c | 95 ++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fee3e8826b..0daf50d836 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2221,10 +2221,17 @@ qemuMigrationSrcCleanup(virDomainObj *vm, VIR_WARN("Migration of domain %s finished but we don't know if the" " domain was successfully started on destination or not", vm->def->name); - qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, - jobPriv->migParams, priv->job.apiFlags); - /* clear the job and let higher levels decide what to do */ - qemuMigrationJobFinish(vm); + + if (virDomainObjIsPostcopy(vm, VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT)) { + qemuMigrationSrcPostcopyFailed(vm); + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); + qemuMigrationJobContinue(vm); + } else { + qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, + jobPriv->migParams, priv->job.apiFlags); + /* clear the job and let higher levels decide what to do */ + qemuMigrationJobFinish(vm); + } break; case QEMU_MIGRATION_PHASE_PERFORM3: @@ -3400,6 +3407,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, qemuDomainObjPrivate *priv = vm->privateData; qemuDomainJobPrivate *jobPriv = priv->job.privateData; virDomainJobData *jobData = NULL; + qemuMigrationJobPhase phase; VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "flags=0x%x, retcode=%d", @@ -3408,10 +3416,17 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, virCheckFlags(QEMU_MIGRATION_FLAGS, -1); - qemuMigrationJobSetPhase(vm, - retcode == 0 - ? QEMU_MIGRATION_PHASE_CONFIRM3 - : QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED); + /* Keep the original migration phase in case post-copy failed as the job + * will stay active even though migration API finishes with an error. + */ + if (virDomainObjIsFailedPostcopy(vm)) + phase = priv->job.phase; + else if (retcode == 0) + phase = QEMU_MIGRATION_PHASE_CONFIRM3; + else + phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED; + + qemuMigrationJobSetPhase(vm, phase); if (!(mig = qemuMigrationCookieParse(driver, vm->def, priv->origname, priv, cookiein, cookieinlen, @@ -3480,13 +3495,14 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, virErrorRestore(&orig_err); if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && - reason == VIR_DOMAIN_PAUSED_POSTCOPY) + reason == VIR_DOMAIN_PAUSED_POSTCOPY) { qemuMigrationSrcPostcopyFailed(vm); - else + } else if (!virDomainObjIsFailedPostcopy(vm)) { qemuMigrationSrcRestoreDomainState(driver, vm); - qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, - jobPriv->migParams, priv->job.apiFlags); + qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, + jobPriv->migParams, priv->job.apiFlags); + } qemuDomainSaveStatus(vm); } @@ -3504,12 +3520,18 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, { qemuMigrationJobPhase phase; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivate *priv = vm->privateData; int ret = -1; if (!qemuMigrationJobIsActive(vm, VIR_ASYNC_JOB_MIGRATION_OUT)) goto cleanup; - if (cancelled) + /* Keep the original migration phase in case post-copy failed as the job + * will stay active even though migration API finishes with an error. + */ + if (virDomainObjIsFailedPostcopy(vm)) + phase = priv->job.phase; + else if (cancelled) phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED; else phase = QEMU_MIGRATION_PHASE_CONFIRM3; @@ -3517,12 +3539,19 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, qemuMigrationJobStartPhase(vm, phase); virCloseCallbacksUnset(driver->closeCallbacks, vm, qemuMigrationSrcCleanup); + qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); ret = qemuMigrationSrcConfirmPhase(driver, vm, cookiein, cookieinlen, flags, cancelled); - qemuMigrationJobFinish(vm); + if (virDomainObjIsFailedPostcopy(vm)) { + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); + qemuMigrationJobContinue(vm); + } else { + qemuMigrationJobFinish(vm); + } + if (!virDomainObjIsActive(vm)) { if (!cancelled && ret == 0 && flags & VIR_MIGRATE_UNDEFINE_SOURCE) { virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); @@ -5334,16 +5363,22 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, if (ret < 0) virErrorPreserveLast(&orig_err); - /* v2 proto has no confirm phase so we need to reset migration parameters - * here - */ - if (!v3proto && ret < 0) - qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, - jobPriv->migParams, priv->job.apiFlags); + if (virDomainObjIsFailedPostcopy(vm)) { + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); + qemuMigrationJobContinue(vm); + } else { + /* v2 proto has no confirm phase so we need to reset migration parameters + * here + */ + if (!v3proto && ret < 0) + qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, + jobPriv->migParams, priv->job.apiFlags); - qemuMigrationSrcRestoreDomainState(driver, vm); + qemuMigrationSrcRestoreDomainState(driver, vm); + + qemuMigrationJobFinish(vm); + } - qemuMigrationJobFinish(vm); if (!virDomainObjIsActive(vm) && ret == 0) { if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) { virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); @@ -5414,11 +5449,12 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, goto endjob; endjob: - if (ret < 0) { + if (ret < 0 && !virDomainObjIsFailedPostcopy(vm)) { qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, jobPriv->migParams, priv->job.apiFlags); qemuMigrationJobFinish(vm); } else { + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); qemuMigrationJobContinue(vm); } @@ -5879,10 +5915,17 @@ qemuMigrationDstFinish(virQEMUDriver *driver, g_clear_pointer(&priv->job.completed, virDomainJobDataFree); } - qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, - jobPriv->migParams, priv->job.apiFlags); + if (virDomainObjIsFailedPostcopy(vm)) { + qemuProcessAutoDestroyRemove(driver, vm); + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); + qemuMigrationJobContinue(vm); + } else { + qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + jobPriv->migParams, priv->job.apiFlags); + + qemuMigrationJobFinish(vm); + } - qemuMigrationJobFinish(vm); if (!virDomainObjIsActive(vm)) qemuDomainRemoveInactive(driver, vm); -- 2.35.1

Since we keep the migration job active when post-copy migration fails, we need to restore it when reconnecting to running domains. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_process.c | 128 ++++++++++++++++++++++++++++++---------- 1 file changed, 96 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f5a45c898d..081b049672 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3385,20 +3385,48 @@ qemuProcessCleanupMigrationJob(virQEMUDriver *driver, } +static void +qemuProcessRestoreMigrationJob(virDomainObj *vm, + qemuDomainJobObj *job) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuDomainJobPrivate *jobPriv = job->privateData; + virDomainJobOperation op; + unsigned long long allowedJobs; + + if (job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) { + op = VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN; + allowedJobs = VIR_JOB_NONE; + } else { + op = VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT; + allowedJobs = VIR_JOB_DEFAULT_MASK | JOB_MASK(VIR_JOB_MIGRATION_OP); + } + + qemuDomainObjRestoreAsyncJob(vm, job->asyncJob, job->phase, op, + QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION, + VIR_DOMAIN_JOB_STATUS_PAUSED, + allowedJobs); + + job->privateData = g_steal_pointer(&priv->job.privateData); + priv->job.privateData = jobPriv; + priv->job.apiFlags = job->apiFlags; + + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); +} + + +/* + * Returns + * -1 on error, the domain will be killed, + * 0 the domain should remain running with the migration job discarded, + * 1 the daemon was restarted during post-copy phase + */ static int qemuProcessRecoverMigrationIn(virQEMUDriver *driver, virDomainObj *vm, - const qemuDomainJobObj *job, - virDomainState state, - int reason) + qemuDomainJobObj *job, + virDomainState state) { - - qemuDomainJobPrivate *jobPriv = job->privateData; - bool postcopy = (state == VIR_DOMAIN_PAUSED && - reason == VIR_DOMAIN_PAUSED_POSTCOPY_FAILED) || - (state == VIR_DOMAIN_RUNNING && - reason == VIR_DOMAIN_RUNNING_POSTCOPY); - VIR_DEBUG("Active incoming migration in phase %s", qemuMigrationJobPhaseTypeToString(job->phase)); @@ -3435,32 +3463,37 @@ qemuProcessRecoverMigrationIn(virQEMUDriver *driver, /* migration finished, we started resuming the domain but didn't * confirm success or failure yet; killing it seems safest unless * we already started guest CPUs or we were in post-copy mode */ - if (postcopy) { + if (virDomainObjIsPostcopy(vm, VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN)) { qemuMigrationDstPostcopyFailed(vm); - } else if (state != VIR_DOMAIN_RUNNING) { + return 1; + } + + if (state != VIR_DOMAIN_RUNNING) { VIR_DEBUG("Killing migrated domain %s", vm->def->name); return -1; } break; } - qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_NONE, - jobPriv->migParams, job->apiFlags); return 0; } + +/* + * Returns + * -1 on error, the domain will be killed, + * 0 the domain should remain running with the migration job discarded, + * 1 the daemon was restarted during post-copy phase + */ static int qemuProcessRecoverMigrationOut(virQEMUDriver *driver, virDomainObj *vm, - const qemuDomainJobObj *job, + qemuDomainJobObj *job, virDomainState state, int reason, unsigned int *stopFlags) { - qemuDomainJobPrivate *jobPriv = job->privateData; - bool postcopy = state == VIR_DOMAIN_PAUSED && - (reason == VIR_DOMAIN_PAUSED_POSTCOPY || - reason == VIR_DOMAIN_PAUSED_POSTCOPY_FAILED); + bool postcopy = virDomainObjIsPostcopy(vm, VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT); bool resume = false; VIR_DEBUG("Active outgoing migration in phase %s", @@ -3500,8 +3533,10 @@ qemuProcessRecoverMigrationOut(virQEMUDriver *driver, * of Finish3 step; third party needs to check what to do next; in * post-copy mode we can use PAUSED_POSTCOPY_FAILED state for this */ - if (postcopy) + if (postcopy) { qemuMigrationSrcPostcopyFailed(vm); + return 1; + } break; case QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED: @@ -3511,11 +3546,12 @@ qemuProcessRecoverMigrationOut(virQEMUDriver *driver, */ if (postcopy) { qemuMigrationSrcPostcopyFailed(vm); - } else { - VIR_DEBUG("Resuming domain %s after failed migration", - vm->def->name); - resume = true; + return 1; } + + VIR_DEBUG("Resuming domain %s after failed migration", + vm->def->name); + resume = true; break; case QEMU_MIGRATION_PHASE_CONFIRM3: @@ -3539,15 +3575,49 @@ qemuProcessRecoverMigrationOut(virQEMUDriver *driver, } } + return 0; +} + + +static int +qemuProcessRecoverMigration(virQEMUDriver *driver, + virDomainObj *vm, + qemuDomainJobObj *job, + unsigned int *stopFlags) +{ + qemuDomainJobPrivate *jobPriv = job->privateData; + virDomainState state; + int reason; + int rc; + + state = virDomainObjGetState(vm, &reason); + + if (job->asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT) { + rc = qemuProcessRecoverMigrationOut(driver, vm, job, + state, reason, stopFlags); + } else { + rc = qemuProcessRecoverMigrationIn(driver, vm, job, state); + } + + if (rc < 0) + return -1; + + if (rc > 0) { + qemuProcessRestoreMigrationJob(vm, job); + return 0; + } + qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_NONE, jobPriv->migParams, job->apiFlags); + return 0; } + static int qemuProcessRecoverJob(virQEMUDriver *driver, virDomainObj *vm, - const qemuDomainJobObj *job, + qemuDomainJobObj *job, unsigned int *stopFlags) { qemuDomainObjPrivate *priv = vm->privateData; @@ -3565,14 +3635,8 @@ qemuProcessRecoverJob(virQEMUDriver *driver, switch (job->asyncJob) { case VIR_ASYNC_JOB_MIGRATION_OUT: - if (qemuProcessRecoverMigrationOut(driver, vm, job, - state, reason, stopFlags) < 0) - return -1; - break; - case VIR_ASYNC_JOB_MIGRATION_IN: - if (qemuProcessRecoverMigrationIn(driver, vm, job, - state, reason) < 0) + if (qemuProcessRecoverMigration(driver, vm, job, stopFlags) < 0) return -1; break; -- 2.35.1

Migration is a job which takes some time and if it succeeds, there's nothing to call another migration on. If a migration fails, it might make sense to rerun it with different arguments, but this would only be done once the first migration fails rather than while it is still running. If this was not enough, the migration job now stays active even if post-copy migration fails and anyone possibly retrying the migration would be waiting for the job timeout just to get a suboptimal error message. So let's special case getting a migration job when another one is already active. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b9a8326fbb..f81d1e65ff 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -82,6 +82,23 @@ VIR_ENUM_IMPL(qemuMigrationJobPhase, ); +static bool ATTRIBUTE_NONNULL(1) +qemuMigrationJobIsAllowed(virDomainObj *vm) +{ + qemuDomainObjPrivate *priv = vm->privateData; + + if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_IN || + priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("another migration job is already running for domain '%s'"), + vm->def->name); + return false; + } + + return true; +} + + static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT qemuMigrationJobStart(virQEMUDriver *driver, virDomainObj *vm, @@ -92,6 +109,9 @@ qemuMigrationJobStart(virQEMUDriver *driver, virDomainJobOperation op; unsigned long long mask; + if (!qemuMigrationJobIsAllowed(vm)) + return -1; + if (job == VIR_ASYNC_JOB_MIGRATION_IN) { op = VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN; mask = VIR_JOB_NONE; @@ -2602,6 +2622,9 @@ qemuMigrationSrcBegin(virConnectPtr conn, goto cleanup; asyncJob = VIR_ASYNC_JOB_MIGRATION_OUT; } else { + if (!qemuMigrationJobIsAllowed(vm)) + goto cleanup; + if (qemuDomainObjBeginJob(driver, vm, VIR_JOB_MODIFY) < 0) goto cleanup; asyncJob = VIR_ASYNC_JOB_NONE; -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - new patch src/qemu/qemu_process.c | 44 ++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a87dc9a1fb..f1264909de 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1472,6 +1472,7 @@ qemuProcessHandleMigrationStatus(qemuMonitor *mon G_GNUC_UNUSED, qemuDomainJobDataPrivate *privJob = NULL; virQEMUDriver *driver; virObjectEvent *event = NULL; + virDomainState state; int reason; virObjectLock(vm); @@ -1493,19 +1494,38 @@ qemuProcessHandleMigrationStatus(qemuMonitor *mon G_GNUC_UNUSED, privJob->stats.mig.status = status; virDomainObjBroadcast(vm); - if (status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY && - priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT && - virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && - reason == VIR_DOMAIN_PAUSED_MIGRATION) { - VIR_DEBUG("Correcting paused state reason for domain %s to %s", - vm->def->name, - virDomainPausedReasonTypeToString(VIR_DOMAIN_PAUSED_POSTCOPY)); + state = virDomainObjGetState(vm, &reason); - virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_POSTCOPY); - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY); - qemuDomainSaveStatus(vm); + switch ((qemuMonitorMigrationStatus) status) { + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY: + if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT && + state == VIR_DOMAIN_PAUSED && + reason == VIR_DOMAIN_PAUSED_MIGRATION) { + VIR_DEBUG("Correcting paused state reason for domain %s to %s", + vm->def->name, + virDomainPausedReasonTypeToString(VIR_DOMAIN_PAUSED_POSTCOPY)); + + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_POSTCOPY); + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY); + qemuDomainSaveStatus(vm); + } + break; + + case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: + case QEMU_MONITOR_MIGRATION_STATUS_SETUP: + case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: + case QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER: + case QEMU_MONITOR_MIGRATION_STATUS_DEVICE: + case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: + case QEMU_MONITOR_MIGRATION_STATUS_ERROR: + case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING: + case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED: + case QEMU_MONITOR_MIGRATION_STATUS_WAIT_UNPLUG: + case QEMU_MONITOR_MIGRATION_STATUS_LAST: + default: + break; } cleanup: -- 2.35.1

When connection breaks during post-copy migration, QEMU enters 'postcopy-paused' state. We need to handle this state and make the situation visible to upper layers. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - rebased on top of newly added "qemu: Use switch in qemuProcessHandleMigrationStatus" src/hypervisor/domain_job.c | 1 + src/hypervisor/domain_job.h | 1 + src/qemu/qemu_driver.c | 1 + src/qemu/qemu_migration.c | 11 +++++++++++ src/qemu/qemu_monitor.c | 1 + src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 1 + src/qemu/qemu_process.c | 11 +++++++++++ 8 files changed, 28 insertions(+) diff --git a/src/hypervisor/domain_job.c b/src/hypervisor/domain_job.c index ff4e008cb5..49867c3982 100644 --- a/src/hypervisor/domain_job.c +++ b/src/hypervisor/domain_job.c @@ -93,6 +93,7 @@ virDomainJobStatusToType(virDomainJobStatus status) case VIR_DOMAIN_JOB_STATUS_MIGRATING: case VIR_DOMAIN_JOB_STATUS_HYPERVISOR_COMPLETED: case VIR_DOMAIN_JOB_STATUS_POSTCOPY: + case VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED: case VIR_DOMAIN_JOB_STATUS_PAUSED: return VIR_DOMAIN_JOB_UNBOUNDED; diff --git a/src/hypervisor/domain_job.h b/src/hypervisor/domain_job.h index db8b8b1390..fce35ffbf5 100644 --- a/src/hypervisor/domain_job.h +++ b/src/hypervisor/domain_job.h @@ -78,6 +78,7 @@ typedef enum { VIR_DOMAIN_JOB_STATUS_HYPERVISOR_COMPLETED, VIR_DOMAIN_JOB_STATUS_PAUSED, VIR_DOMAIN_JOB_STATUS_POSTCOPY, + VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED, VIR_DOMAIN_JOB_STATUS_COMPLETED, VIR_DOMAIN_JOB_STATUS_FAILED, VIR_DOMAIN_JOB_STATUS_CANCELED, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4f9eb7b987..28cb454ab7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12540,6 +12540,7 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriver *driver, case VIR_DOMAIN_JOB_STATUS_HYPERVISOR_COMPLETED: case VIR_DOMAIN_JOB_STATUS_POSTCOPY: case VIR_DOMAIN_JOB_STATUS_PAUSED: + case VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED: if (qemuMigrationAnyFetchStats(driver, vm, VIR_ASYNC_JOB_NONE, jobData, NULL) < 0) return -1; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f81d1e65ff..de4be0e7f9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1747,6 +1747,10 @@ qemuMigrationUpdateJobType(virDomainJobData *jobData) jobData->status = VIR_DOMAIN_JOB_STATUS_POSTCOPY; break; + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_PAUSED: + jobData->status = VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED; + break; + case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: jobData->status = VIR_DOMAIN_JOB_STATUS_HYPERVISOR_COMPLETED; break; @@ -1871,6 +1875,12 @@ qemuMigrationJobCheckStatus(virQEMUDriver *driver, qemuMigrationJobName(vm), _("canceled by client")); return -1; + case VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED: + virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"), + qemuMigrationJobName(vm), + _("post-copy phase failed")); + return -1; + case VIR_DOMAIN_JOB_STATUS_COMPLETED: case VIR_DOMAIN_JOB_STATUS_ACTIVE: case VIR_DOMAIN_JOB_STATUS_MIGRATING: @@ -1973,6 +1983,7 @@ qemuMigrationAnyCompleted(virQEMUDriver *driver, case VIR_DOMAIN_JOB_STATUS_FAILED: case VIR_DOMAIN_JOB_STATUS_CANCELED: + case VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED: /* QEMU aborted the migration. */ return -1; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2cfe9dbb00..8d0f54e4a0 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -149,6 +149,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus, "inactive", "setup", "active", "pre-switchover", "device", "postcopy-active", + "postcopy-paused", "completed", "failed", "cancelling", "cancelled", "wait-unplug", diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index be341d5196..90532962fe 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -795,6 +795,7 @@ typedef enum { QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER, QEMU_MONITOR_MIGRATION_STATUS_DEVICE, QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY, + QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_PAUSED, QEMU_MONITOR_MIGRATION_STATUS_COMPLETED, QEMU_MONITOR_MIGRATION_STATUS_ERROR, QEMU_MONITOR_MIGRATION_STATUS_CANCELLING, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index dc05dfd047..ffff9e7103 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3242,6 +3242,7 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValue *reply, case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY: + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_PAUSED: case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING: case QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f1264909de..6acf5c7881 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1513,6 +1513,17 @@ qemuProcessHandleMigrationStatus(qemuMonitor *mon G_GNUC_UNUSED, } break; + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_PAUSED: + if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT && + state == VIR_DOMAIN_PAUSED) { + /* At this point no thread is watching the migration progress on + * the source as it is just waiting for the Finish phase to end. + * Thus we need to handle the event here. */ + qemuMigrationSrcPostcopyFailed(vm); + qemuDomainSaveStatus(vm); + } + break; + case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: case QEMU_MONITOR_MIGRATION_STATUS_SETUP: case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - rebased on top of newly added "qemu: Use switch in qemuProcessHandleMigrationStatus" - debug message on a single line src/qemu/qemu_migration.c | 1 + src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 1 + src/qemu/qemu_process.c | 25 +++++++++++++++++++++++++ 5 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index de4be0e7f9..12f06648d7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1744,6 +1744,7 @@ qemuMigrationUpdateJobType(virDomainJobData *jobData) switch ((qemuMonitorMigrationStatus) priv->stats.mig.status) { case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY: + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_RECOVER: jobData->status = VIR_DOMAIN_JOB_STATUS_POSTCOPY; break; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8d0f54e4a0..ec56d413da 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -149,7 +149,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus, "inactive", "setup", "active", "pre-switchover", "device", "postcopy-active", - "postcopy-paused", + "postcopy-paused", "postcopy-recover", "completed", "failed", "cancelling", "cancelled", "wait-unplug", diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 90532962fe..41e8db945c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -796,6 +796,7 @@ typedef enum { QEMU_MONITOR_MIGRATION_STATUS_DEVICE, QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY, QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_PAUSED, + QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_RECOVER, QEMU_MONITOR_MIGRATION_STATUS_COMPLETED, QEMU_MONITOR_MIGRATION_STATUS_ERROR, QEMU_MONITOR_MIGRATION_STATUS_CANCELLING, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ffff9e7103..83d0600a75 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3243,6 +3243,7 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValue *reply, case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY: case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_PAUSED: + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_RECOVER: case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING: case QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6acf5c7881..b039108ff7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1524,6 +1524,31 @@ qemuProcessHandleMigrationStatus(qemuMonitor *mon G_GNUC_UNUSED, } break; + case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY_RECOVER: + if (virDomainObjIsFailedPostcopy(vm)) { + int eventType = -1; + int eventDetail = -1; + + if (state == VIR_DOMAIN_PAUSED) { + reason = VIR_DOMAIN_PAUSED_POSTCOPY; + eventType = VIR_DOMAIN_EVENT_SUSPENDED; + eventDetail = qemuDomainPausedReasonToSuspendedEvent(reason); + } else { + reason = VIR_DOMAIN_RUNNING_POSTCOPY; + eventType = VIR_DOMAIN_EVENT_RESUMED; + eventDetail = qemuDomainRunningReasonToResumeEvent(reason); + } + + VIR_DEBUG("Post-copy migration recovered; correcting state for domain '%s' to %s/%s", + vm->def->name, + virDomainStateTypeToString(state), + NULLSTR(virDomainStateReasonToString(state, reason))); + virDomainObjSetState(vm, state, reason); + event = virDomainEventLifecycleNewFromObj(vm, eventType, eventDetail); + qemuDomainSaveStatus(vm); + } + break; + case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: case QEMU_MONITOR_MIGRATION_STATUS_SETUP: case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: -- 2.35.1

We don't need the object until we get to the "endjob" label. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 12f06648d7..8a1210c8c7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5949,11 +5949,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_MIGRATED); } - } - - dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id); - 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 @@ -5981,6 +5977,8 @@ qemuMigrationDstFinish(virQEMUDriver *driver, /* Guest is successfully running, so cancel previous auto destroy */ qemuProcessAutoDestroyRemove(driver, vm); + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id); + endjob: if (!dom && !(flags & VIR_MIGRATE_OFFLINE) && -- 2.35.1

Most of the code in "endjob" label is executed only on failure. Let's duplicate the rest so that the label can be used only in error path making the success path easier to follow and refactor. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 45 ++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f6d3200cba..d2a6e20550 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5823,7 +5823,9 @@ qemuMigrationDstFinish(virQEMUDriver *driver, QEMU_MIGRATION_COOKIE_STATS) < 0) VIR_WARN("Unable to encode migration cookie"); } - goto endjob; + + qemuMigrationJobFinish(vm); + goto cleanup; } if (retcode != 0) { @@ -6004,12 +6006,31 @@ qemuMigrationDstFinish(virQEMUDriver *driver, if (inPostCopy) g_clear_pointer(&priv->job.completed, virDomainJobDataFree); + qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + jobPriv->migParams, priv->job.apiFlags); + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, vm->def->id); + qemuMigrationJobFinish(vm); + + cleanup: + g_clear_pointer(&jobData, virDomainJobDataFree); + virPortAllocatorRelease(port); + if (priv->mon) + qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); + VIR_FREE(priv->origname); + virDomainObjEndAPI(&vm); + virErrorRestore(&orig_err); + + /* Set a special error if Finish is expected to return NULL as a result of + * successful call with retcode != 0 + */ + if (retcode != 0 && !dom && virGetLastErrorCode() == VIR_ERR_OK) + virReportError(VIR_ERR_MIGRATE_FINISH_OK, NULL); + return dom; + endjob: - if (!dom && - !(flags & VIR_MIGRATE_OFFLINE) && - virDomainObjIsActive(vm)) { + if (virDomainObjIsActive(vm)) { if (doKill) { qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, VIR_ASYNC_JOB_MIGRATION_IN, @@ -6038,21 +6059,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver, if (!virDomainObjIsActive(vm)) qemuDomainRemoveInactive(driver, vm); - cleanup: - g_clear_pointer(&jobData, virDomainJobDataFree); - virPortAllocatorRelease(port); - if (priv->mon) - qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); - VIR_FREE(priv->origname); - virDomainObjEndAPI(&vm); - virErrorRestore(&orig_err); - - /* Set a special error if Finish is expected to return NULL as a result of - * successful call with retcode != 0 - */ - if (retcode != 0 && !dom && virGetLastErrorCode() == VIR_ERR_OK) - virReportError(VIR_ERR_MIGRATE_FINISH_OK, NULL); - return dom; + goto cleanup; } -- 2.35.1

The final part of Finish phase will be refactored into a dedicated function and we don't want to generate the cookie there. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 792033daa0..170caf77e7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5948,12 +5948,27 @@ qemuMigrationDstFinish(virQEMUDriver *driver, qemuDomainJobDataUpdateDowntime(jobData); } + if (inPostCopy && + qemuMigrationDstWaitForCompletion(driver, vm, + VIR_ASYNC_JOB_MIGRATION_IN, + false) < 0) { + goto error; + } + + if (jobData) { + priv->job.completed = g_steal_pointer(&jobData); + priv->job.completed->status = VIR_DOMAIN_JOB_STATUS_COMPLETED; + qemuDomainJobSetStatsType(priv->job.completed, + QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION); + } + + if (qemuMigrationCookieFormat(mig, driver, vm, + QEMU_MIGRATION_DESTINATION, + cookieout, cookieoutlen, + QEMU_MIGRATION_COOKIE_STATS) < 0) + VIR_WARN("Unable to encode migration cookie"); + if (inPostCopy) { - if (qemuMigrationDstWaitForCompletion(driver, vm, - VIR_ASYNC_JOB_MIGRATION_IN, - false) < 0) { - goto error; - } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, @@ -5987,19 +6002,6 @@ qemuMigrationDstFinish(virQEMUDriver *driver, /* Guest is successfully running, so cancel previous auto destroy */ qemuProcessAutoDestroyRemove(driver, vm); - if (jobData) { - priv->job.completed = g_steal_pointer(&jobData); - priv->job.completed->status = VIR_DOMAIN_JOB_STATUS_COMPLETED; - qemuDomainJobSetStatsType(priv->job.completed, - QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION); - } - - if (qemuMigrationCookieFormat(mig, driver, vm, - QEMU_MIGRATION_DESTINATION, - cookieout, cookieoutlen, - QEMU_MIGRATION_COOKIE_STATS) < 0) - VIR_WARN("Unable to encode migration cookie"); - /* Remove completed stats for post-copy, everything but timing fields * is obsolete anyway. */ -- 2.35.1

The comment about QEMU < 0.10.6 has been irrelevant for years. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 10fc99180d..fdc93d7fe1 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5959,10 +5959,6 @@ qemuMigrationDstFinish(virQEMUDriver *driver, inPostCopy = true; if (!(flags & VIR_MIGRATE_PAUSED)) { - /* run 'cont' on the destination, which allows migration on qemu - * >= 0.10.6 to work properly. This isn't strictly necessary on - * older qemu's, but it also doesn't hurt anything there - */ if (qemuProcessStartCPUs(driver, vm, inPostCopy ? VIR_DOMAIN_RUNNING_POSTCOPY : VIR_DOMAIN_RUNNING_MIGRATED, -- 2.35.1

We want to prevent our error path that can potentially kill the domain on the destination host from overwriting an error reported earlier, but we were only doing so in one specific path when starting vCPUs fails. Let's do it in all paths. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fdc93d7fe1..10c4f7a470 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5966,11 +5966,6 @@ qemuMigrationDstFinish(virQEMUDriver *driver, if (virGetLastErrorCode() == VIR_ERR_OK) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); - /* Need to save the current error, in case shutting - * down the process overwrites it - */ - virErrorPreserveLast(&orig_err); - /* * In v3 protocol, the source VM is still available to * restart during confirm() step, so we kill it off @@ -6043,6 +6038,10 @@ qemuMigrationDstFinish(virQEMUDriver *driver, return dom; error: + /* Need to save the current error, in case shutting down the process + * overwrites it. */ + virErrorPreserveLast(&orig_err); + if (virDomainObjIsActive(vm)) { if (doKill) { qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, -- 2.35.1

Refactors qemuMigrationDstFinish by moving some parts to a dedicated function for easier introduction of postcopy resume code without duplicating common parts of the Finish phase. The goal is to have the following call graph: - qemuMigrationDstFinish - qemuMigrationDstFinishOffline - qemuMigrationDstFinishActive - qemuMigrationDstFinishFresh - qemuMigrationDstFinishResume Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 72f22f09e4..01e47e46cc 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5808,6 +5808,32 @@ qemuMigrationDstComplete(virQEMUDriver *driver, } +static virDomainPtr +qemuMigrationDstFinishOffline(virQEMUDriver *driver, + virConnectPtr dconn, + virDomainObj *vm, + qemuMigrationCookie *mig, + char **cookieout, + int *cookieoutlen) +{ + virDomainPtr dom = NULL; + + if (qemuMigrationDstPersist(driver, vm, mig, false) < 0) + return NULL; + + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, -1); + + if (dom && + qemuMigrationCookieFormat(mig, driver, vm, + QEMU_MIGRATION_DESTINATION, + cookieout, cookieoutlen, + QEMU_MIGRATION_COOKIE_STATS) < 0) + VIR_WARN("Unable to encode migration cookie"); + + return dom; +} + + /* * Perform Finish phase of a fresh (i.e., not recovery) migration of an active * domain. @@ -5998,16 +6024,9 @@ qemuMigrationDstFinish(virQEMUDriver *driver, goto error; if (flags & VIR_MIGRATE_OFFLINE) { - if (retcode == 0 && - qemuMigrationDstPersist(driver, vm, mig, false) == 0) { - dom = virGetDomain(dconn, vm->def->name, vm->def->uuid, -1); - - if (dom && - qemuMigrationCookieFormat(mig, driver, vm, - QEMU_MIGRATION_DESTINATION, - cookieout, cookieoutlen, - QEMU_MIGRATION_COOKIE_STATS) < 0) - VIR_WARN("Unable to encode migration cookie"); + if (retcode == 0) { + dom = qemuMigrationDstFinishOffline(driver, dconn, vm, mig, + cookieout, cookieoutlen); } qemuMigrationJobFinish(vm); -- 2.35.1

Refactors qemuMigrationDstFinish by moving some parts to a dedicated function for easier introduction of postcopy resume code without duplicating common parts of the Finish phase. The goal is to have the following call graph: - qemuMigrationDstFinish - qemuMigrationDstFinishOffline - qemuMigrationDstFinishActive - qemuMigrationDstFinishFresh - qemuMigrationDstFinishResume Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - dropped orig_err parameter from qemuMigrationDstFinishActive src/qemu/qemu_migration.c | 169 ++++++++++++++++++++++---------------- 1 file changed, 100 insertions(+), 69 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3c69ddbd45..195b0fd309 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5973,71 +5973,32 @@ qemuMigrationDstFinishFresh(virQEMUDriver *driver, } -virDomainPtr -qemuMigrationDstFinish(virQEMUDriver *driver, - virConnectPtr dconn, - virDomainObj *vm, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - unsigned long flags, - int retcode, - bool v3proto) +static virDomainPtr +qemuMigrationDstFinishActive(virQEMUDriver *driver, + virConnectPtr dconn, + virDomainObj *vm, + int cookie_flags, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + int retcode, + bool v3proto, + unsigned long long timeReceived) { + virErrorPtr orig_err = NULL; virDomainPtr dom = NULL; g_autoptr(qemuMigrationCookie) mig = NULL; - virErrorPtr orig_err = NULL; - int cookie_flags = 0; qemuDomainObjPrivate *priv = vm->privateData; qemuDomainJobPrivate *jobPriv = priv->job.privateData; - unsigned short port; - unsigned long long timeReceived = 0; virObjectEvent *event; bool inPostCopy = false; bool doKill = true; int rc; - VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " - "cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d", - driver, dconn, vm, NULLSTR(cookiein), cookieinlen, - cookieout, cookieoutlen, flags, retcode); - - port = priv->migrationPort; - priv->migrationPort = 0; - - if (!qemuMigrationJobIsActive(vm, VIR_ASYNC_JOB_MIGRATION_IN)) { - qemuMigrationDstErrorReport(driver, vm->def->name); - goto cleanup; - } - - ignore_value(virTimeMillisNow(&timeReceived)); - - qemuMigrationJobStartPhase(vm, - v3proto ? QEMU_MIGRATION_PHASE_FINISH3 - : QEMU_MIGRATION_PHASE_FINISH2); - - qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup); - g_clear_pointer(&priv->job.completed, virDomainJobDataFree); - - cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK | - QEMU_MIGRATION_COOKIE_STATS | - QEMU_MIGRATION_COOKIE_NBD; - /* Some older versions of libvirt always send persistent XML in the cookie - * even though VIR_MIGRATE_PERSIST_DEST was not used. */ - cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT; - - if (flags & VIR_MIGRATE_OFFLINE) { - if (retcode == 0) { - dom = qemuMigrationDstFinishOffline(driver, dconn, vm, - cookie_flags, - cookiein, cookieinlen, - cookieout, cookieoutlen); - } - - qemuMigrationJobFinish(vm); - goto cleanup; - } + VIR_DEBUG("vm=%p, flags=0x%lx, retcode=%d", + vm, flags, retcode); if (!(mig = qemuMigrationCookieParse(driver, vm->def, priv->origname, priv, cookiein, cookieinlen, cookie_flags))) @@ -6075,19 +6036,6 @@ qemuMigrationDstFinish(virQEMUDriver *driver, qemuMigrationJobFinish(vm); - cleanup: - virPortAllocatorRelease(port); - if (priv->mon) - qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); - VIR_FREE(priv->origname); - virDomainObjEndAPI(&vm); - virErrorRestore(&orig_err); - - /* Set a special error if Finish is expected to return NULL as a result of - * successful call with retcode != 0 - */ - if (retcode != 0 && !dom && virGetLastErrorCode() == VIR_ERR_OK) - virReportError(VIR_ERR_MIGRATE_FINISH_OK, NULL); return dom; error: @@ -6124,7 +6072,90 @@ qemuMigrationDstFinish(virQEMUDriver *driver, if (!virDomainObjIsActive(vm)) qemuDomainRemoveInactive(driver, vm); - goto cleanup; + virErrorRestore(&orig_err); + return NULL; +} + + +virDomainPtr +qemuMigrationDstFinish(virQEMUDriver *driver, + virConnectPtr dconn, + virDomainObj *vm, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + int retcode, + bool v3proto) +{ + virDomainPtr dom = NULL; + int cookie_flags = 0; + qemuDomainObjPrivate *priv = vm->privateData; + unsigned short port; + unsigned long long timeReceived = 0; + + VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " + "cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d", + driver, dconn, vm, NULLSTR(cookiein), cookieinlen, + cookieout, cookieoutlen, flags, retcode); + + port = priv->migrationPort; + priv->migrationPort = 0; + + if (!qemuMigrationJobIsActive(vm, VIR_ASYNC_JOB_MIGRATION_IN)) { + qemuMigrationDstErrorReport(driver, vm->def->name); + goto cleanup; + } + + ignore_value(virTimeMillisNow(&timeReceived)); + + qemuMigrationJobStartPhase(vm, + v3proto ? QEMU_MIGRATION_PHASE_FINISH3 + : QEMU_MIGRATION_PHASE_FINISH2); + + qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup); + g_clear_pointer(&priv->job.completed, virDomainJobDataFree); + + cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK | + QEMU_MIGRATION_COOKIE_STATS | + QEMU_MIGRATION_COOKIE_NBD; + /* Some older versions of libvirt always send persistent XML in the cookie + * even though VIR_MIGRATE_PERSIST_DEST was not used. */ + cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT; + + if (flags & VIR_MIGRATE_OFFLINE) { + if (retcode == 0) { + dom = qemuMigrationDstFinishOffline(driver, dconn, vm, + cookie_flags, + cookiein, cookieinlen, + cookieout, cookieoutlen); + } + + qemuMigrationJobFinish(vm); + goto cleanup; + } + + dom = qemuMigrationDstFinishActive(driver, dconn, vm, cookie_flags, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, retcode, v3proto, timeReceived); + if (!dom) + goto cleanup; + + cleanup: + virPortAllocatorRelease(port); + if (priv->mon) + qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); + VIR_FREE(priv->origname); + virDomainObjEndAPI(&vm); + + /* Set a special error if Finish is expected to return NULL as a result of + * successful call with retcode != 0 + */ + if (retcode != 0 && !dom && virGetLastErrorCode() == VIR_ERR_OK) + virReportError(VIR_ERR_MIGRATE_FINISH_OK, NULL); + return dom; } -- 2.35.1

The function which started a migration phase should also finish it by calling qemuMigrationJobFinish/qemuMigrationJobContinue so that the code is easier to follow. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - rewritten without goto src/qemu/qemu_migration.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 195b0fd309..20209a0dc8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5985,7 +5985,8 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, unsigned long flags, int retcode, bool v3proto, - unsigned long long timeReceived) + unsigned long long timeReceived, + bool *finishJob) { virErrorPtr orig_err = NULL; virDomainPtr dom = NULL; @@ -6034,8 +6035,6 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, qemuMigrationDstComplete(driver, vm, inPostCopy, VIR_ASYNC_JOB_MIGRATION_IN); - qemuMigrationJobFinish(vm); - return dom; error: @@ -6061,12 +6060,10 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, if (virDomainObjIsFailedPostcopy(vm)) { qemuProcessAutoDestroyRemove(driver, vm); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + *finishJob = false; } else { qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, jobPriv->migParams, priv->job.apiFlags); - - qemuMigrationJobFinish(vm); } if (!virDomainObjIsActive(vm)) @@ -6131,17 +6128,20 @@ qemuMigrationDstFinish(virQEMUDriver *driver, cookiein, cookieinlen, cookieout, cookieoutlen); } - qemuMigrationJobFinish(vm); - goto cleanup; - } + } else { + bool finishJob = true; - dom = qemuMigrationDstFinishActive(driver, dconn, vm, cookie_flags, - cookiein, cookieinlen, - cookieout, cookieoutlen, - flags, retcode, v3proto, timeReceived); - if (!dom) - goto cleanup; + dom = qemuMigrationDstFinishActive(driver, dconn, vm, cookie_flags, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, retcode, v3proto, timeReceived, + &finishJob); + if (finishJob) + qemuMigrationJobFinish(vm); + else + qemuMigrationJobContinue(vm); + } cleanup: virPortAllocatorRelease(port); -- 2.35.1

By separating it into a dedicated qemuMigrationSrcComplete function which can be later called in other places. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - fixed indentation src/qemu/qemu_migration.c | 70 ++++++++++++++++++++++++--------------- src/qemu/qemu_migration.h | 5 +++ 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 20209a0dc8..d80a250cac 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3491,6 +3491,48 @@ qemuMigrationAnyPrepareDef(virQEMUDriver *driver, } +void +qemuMigrationSrcComplete(virQEMUDriver *driver, + virDomainObj *vm, + virDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virDomainJobData *jobData = priv->job.completed; + virObjectEvent *event; + int reason; + + if (jobData) { + /* We need to refresh migration statistics after a completed post-copy + * migration since jobData contains obsolete data from the time we + * switched to post-copy mode. + */ + if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && + reason == VIR_DOMAIN_PAUSED_POSTCOPY) { + VIR_DEBUG("Refreshing migration statistics"); + if (qemuMigrationAnyFetchStats(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, + jobData, NULL) < 0) + VIR_WARN("Could not refresh migration statistics"); + } + + qemuDomainJobDataUpdateTime(jobData); + } + + /* If guest uses SPICE and supports seamless migration we have to hold + * up domain shutdown until SPICE server transfers its data */ + qemuMigrationSrcWaitForSpice(vm); + + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED, asyncJob, + VIR_QEMU_PROCESS_STOP_MIGRATED); + virDomainAuditStop(vm, "migrated"); + + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_MIGRATED); + virObjectEventStateQueue(driver->domainEventState, event); + qemuDomainEventEmitJobCompleted(driver, vm); +} + + static int qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, virDomainObj *vm, @@ -3500,7 +3542,6 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, int retcode) { g_autoptr(qemuMigrationCookie) mig = NULL; - virObjectEvent *event; qemuDomainObjPrivate *priv = vm->privateData; qemuDomainJobPrivate *jobPriv = priv->job.privateData; virDomainJobData *jobData = NULL; @@ -3537,21 +3578,9 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, /* Update times with the values sent by the destination daemon */ if (mig->jobData && jobData) { - int reason; qemuDomainJobDataPrivate *privJob = jobData->privateData; qemuDomainJobDataPrivate *privMigJob = mig->jobData->privateData; - /* We need to refresh migration statistics after a completed post-copy - * migration since priv->job.completed contains obsolete data from the - * time we switched to post-copy mode. - */ - if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && - reason == VIR_DOMAIN_PAUSED_POSTCOPY && - qemuMigrationAnyFetchStats(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, - jobData, NULL) < 0) - VIR_WARN("Could not refresh migration statistics"); - - qemuDomainJobDataUpdateTime(jobData); jobData->timeDeltaSet = mig->jobData->timeDeltaSet; jobData->timeDelta = mig->jobData->timeDelta; privJob->stats.mig.downtime_set = privMigJob->stats.mig.downtime_set; @@ -3565,20 +3594,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, * If something failed, resume CPUs, but only if we didn't use post-copy. */ if (retcode == 0) { - /* If guest uses SPICE and supports seamless migration we have to hold - * up domain shutdown until SPICE server transfers its data */ - qemuMigrationSrcWaitForSpice(vm); - - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED, - VIR_ASYNC_JOB_MIGRATION_OUT, - VIR_QEMU_PROCESS_STOP_MIGRATED); - virDomainAuditStop(vm, "migrated"); - - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_STOPPED, - VIR_DOMAIN_EVENT_STOPPED_MIGRATED); - virObjectEventStateQueue(driver->domainEventState, event); - qemuDomainEventEmitJobCompleted(driver, vm); + qemuMigrationSrcComplete(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT); } else { virErrorPtr orig_err; int reason; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index ecb0ad6302..1d6051859b 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -205,6 +205,11 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, unsigned int flags, int cancelled); +void +qemuMigrationSrcComplete(virQEMUDriver *driver, + virDomainObj *vm, + virDomainAsyncJob asyncJob); + bool qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainObj *vm, -- 2.35.1

When reconnecting to an active domain we need to use a different job structure than the one referenced from the VM object. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - fixed a typo in the commit message src/qemu/qemu_migration.c | 11 ++++++----- src/qemu/qemu_migration.h | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 53801a29ef..95b69108dc 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5773,10 +5773,11 @@ void qemuMigrationDstComplete(virQEMUDriver *driver, virDomainObj *vm, bool inPostCopy, - virDomainAsyncJob asyncJob) + virDomainAsyncJob asyncJob, + qemuDomainJobObj *job) { qemuDomainObjPrivate *priv = vm->privateData; - qemuDomainJobPrivate *jobPriv = priv->job.privateData; + qemuDomainJobPrivate *jobPriv = job->privateData; virObjectEvent *event; if (inPostCopy) { @@ -5817,10 +5818,10 @@ qemuMigrationDstComplete(virQEMUDriver *driver, * is obsolete anyway. */ if (inPostCopy) - g_clear_pointer(&priv->job.completed, virDomainJobDataFree); + g_clear_pointer(&job->completed, virDomainJobDataFree); qemuMigrationParamsReset(driver, vm, asyncJob, jobPriv->migParams, - priv->job.apiFlags); + job->apiFlags); virPortAllocatorRelease(priv->migrationPort); priv->migrationPort = 0; @@ -6052,7 +6053,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, VIR_WARN("Unable to encode migration cookie"); qemuMigrationDstComplete(driver, vm, inPostCopy, - VIR_ASYNC_JOB_MIGRATION_IN); + VIR_ASYNC_JOB_MIGRATION_IN, &priv->job); return dom; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 1d6051859b..c099cf99cf 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -195,7 +195,8 @@ void qemuMigrationDstComplete(virQEMUDriver *driver, virDomainObj *vm, bool inPostCopy, - virDomainAsyncJob asyncJob); + virDomainAsyncJob asyncJob, + qemuDomainJobObj *job); int qemuMigrationSrcConfirm(virQEMUDriver *driver, -- 2.35.1

We want to use query-migrate QMP command to check the current migration state when reconnecting to active domains, but the reply we get to this command may not contain any statistics at all if called on the destination host. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - dropped line breaks from error messages src/qemu/qemu_monitor_json.c | 85 +++++++++++++++++------------------- 1 file changed, 39 insertions(+), 46 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 83d0600a75..86b7f615c8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3249,56 +3249,49 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValue *reply, case QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER: case QEMU_MONITOR_MIGRATION_STATUS_DEVICE: ram = virJSONValueObjectGetObject(ret, "ram"); - if (!ram) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("migration was active, but no RAM info was set")); - return -1; - } + if (ram) { + if (virJSONValueObjectGetNumberUlong(ram, "transferred", + &stats->ram_transferred) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("migration was active, but RAM 'transferred' data was missing")); + return -1; + } + if (virJSONValueObjectGetNumberUlong(ram, "remaining", + &stats->ram_remaining) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("migration was active, but RAM 'remaining' data was missing")); + return -1; + } + if (virJSONValueObjectGetNumberUlong(ram, "total", + &stats->ram_total) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("migration was active, but RAM 'total' data was missing")); + return -1; + } - if (virJSONValueObjectGetNumberUlong(ram, "transferred", - &stats->ram_transferred) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("migration was active, but RAM 'transferred' " - "data was missing")); - return -1; - } - if (virJSONValueObjectGetNumberUlong(ram, "remaining", - &stats->ram_remaining) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("migration was active, but RAM 'remaining' " - "data was missing")); - return -1; - } - if (virJSONValueObjectGetNumberUlong(ram, "total", - &stats->ram_total) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("migration was active, but RAM 'total' " - "data was missing")); - return -1; - } + if (virJSONValueObjectGetNumberDouble(ram, "mbps", &mbps) == 0 && + mbps > 0) { + /* mpbs from QEMU reports Mbits/s (M as in 10^6 not Mi as 2^20) */ + stats->ram_bps = mbps * (1000 * 1000 / 8); + } - if (virJSONValueObjectGetNumberDouble(ram, "mbps", &mbps) == 0 && - mbps > 0) { - /* mpbs from QEMU reports Mbits/s (M as in 10^6 not Mi as 2^20) */ - stats->ram_bps = mbps * (1000 * 1000 / 8); + if (virJSONValueObjectGetNumberUlong(ram, "duplicate", + &stats->ram_duplicate) == 0) + stats->ram_duplicate_set = true; + ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal", + &stats->ram_normal)); + ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal-bytes", + &stats->ram_normal_bytes)); + ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-pages-rate", + &stats->ram_dirty_rate)); + ignore_value(virJSONValueObjectGetNumberUlong(ram, "page-size", + &stats->ram_page_size)); + ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-sync-count", + &stats->ram_iteration)); + ignore_value(virJSONValueObjectGetNumberUlong(ram, "postcopy-requests", + &stats->ram_postcopy_reqs)); } - if (virJSONValueObjectGetNumberUlong(ram, "duplicate", - &stats->ram_duplicate) == 0) - stats->ram_duplicate_set = true; - ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal", - &stats->ram_normal)); - ignore_value(virJSONValueObjectGetNumberUlong(ram, "normal-bytes", - &stats->ram_normal_bytes)); - ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-pages-rate", - &stats->ram_dirty_rate)); - ignore_value(virJSONValueObjectGetNumberUlong(ram, "page-size", - &stats->ram_page_size)); - ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-sync-count", - &stats->ram_iteration)); - ignore_value(virJSONValueObjectGetNumberUlong(ram, "postcopy-requests", - &stats->ram_postcopy_reqs)); - disk = virJSONValueObjectGetObject(ret, "disk"); if (disk) { rc = virJSONValueObjectGetNumberUlong(disk, "transferred", -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - dropped line breaks from error messages src/qemu/qemu_migration.c | 41 ++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5765647ad7..4e3d823806 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2512,6 +2512,25 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, return NULL; } + if (flags & VIR_MIGRATE_OFFLINE) { + if (flags & (VIR_MIGRATE_NON_SHARED_DISK | + VIR_MIGRATE_NON_SHARED_INC)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("offline migration cannot handle non-shared storage")); + return NULL; + } + if (!(flags & VIR_MIGRATE_PERSIST_DEST)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("offline migration must be specified with the persistent flag set")); + return NULL; + } + if (flags & VIR_MIGRATE_TUNNELLED) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("tunnelled offline migration does not make sense")); + return NULL; + } + } + if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { if (flags & VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES && !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { @@ -2591,28 +2610,6 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, cookieFlags) < 0) return NULL; - if (flags & VIR_MIGRATE_OFFLINE) { - if (flags & (VIR_MIGRATE_NON_SHARED_DISK | - VIR_MIGRATE_NON_SHARED_INC)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("offline migration cannot handle " - "non-shared storage")); - return NULL; - } - if (!(flags & VIR_MIGRATE_PERSIST_DEST)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("offline migration must be specified with " - "the persistent flag set")); - return NULL; - } - if (flags & VIR_MIGRATE_TUNNELLED) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("tunnelled offline migration does not " - "make sense")); - return NULL; - } - } - if (xmlin) { if (!(def = virDomainDefParseString(xmlin, driver->xmlopt, priv->qemuCaps, VIR_DOMAIN_DEF_PARSE_INACTIVE | -- 2.35.1

When recovering from a failed post-copy migration, we need to go through all migration phases again, but don't need to repeat all the steps in each phase. Let's create a new set of migration phases dedicated to post-copy recovery so that we can easily distinguish between normal and recovery code. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - additional comments src/qemu/qemu_migration.c | 20 +++++++++++++++++++- src/qemu/qemu_migration.h | 6 ++++++ src/qemu/qemu_process.c | 29 +++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 02827bd975..710aae3eb7 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -79,6 +79,12 @@ VIR_ENUM_IMPL(qemuMigrationJobPhase, "prepare", "finish2", "finish3", + "postcopy_failed", + "begin_resume", + "perform_resume", + "confirm_resume", + "prepare_resume", + "finish_resume", ); @@ -139,7 +145,8 @@ qemuMigrationJobSetPhase(virDomainObj *vm, { qemuDomainObjPrivate *priv = vm->privateData; - if (phase < priv->job.phase) { + if (phase < QEMU_MIGRATION_PHASE_POSTCOPY_FAILED && + phase < priv->job.phase) { VIR_ERROR(_("migration protocol going backwards %s => %s"), qemuMigrationJobPhaseTypeToString(priv->job.phase), qemuMigrationJobPhaseTypeToString(phase)); @@ -2328,18 +2335,29 @@ qemuMigrationSrcCleanup(virDomainObj *vm, } break; + case QEMU_MIGRATION_PHASE_BEGIN_RESUME: + case QEMU_MIGRATION_PHASE_PERFORM_RESUME: + qemuMigrationSrcPostcopyFailed(vm); + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); + qemuMigrationJobContinue(vm); + break; + case QEMU_MIGRATION_PHASE_PERFORM3: /* cannot be seen without an active migration API; unreachable */ case QEMU_MIGRATION_PHASE_CONFIRM3: case QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED: + case QEMU_MIGRATION_PHASE_CONFIRM_RESUME: /* all done; unreachable */ case QEMU_MIGRATION_PHASE_PREPARE: case QEMU_MIGRATION_PHASE_FINISH2: case QEMU_MIGRATION_PHASE_FINISH3: + case QEMU_MIGRATION_PHASE_PREPARE_RESUME: + case QEMU_MIGRATION_PHASE_FINISH_RESUME: /* incoming migration; unreachable */ case QEMU_MIGRATION_PHASE_PERFORM2: /* single phase outgoing migration; unreachable */ case QEMU_MIGRATION_PHASE_NONE: + case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_LAST: /* unreachable */ ; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 9351d6ac51..7eb0d4fe02 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -100,6 +100,12 @@ typedef enum { QEMU_MIGRATION_PHASE_PREPARE, QEMU_MIGRATION_PHASE_FINISH2, QEMU_MIGRATION_PHASE_FINISH3, + QEMU_MIGRATION_PHASE_POSTCOPY_FAILED, /* marker for resume phases */ + QEMU_MIGRATION_PHASE_BEGIN_RESUME, + QEMU_MIGRATION_PHASE_PERFORM_RESUME, + QEMU_MIGRATION_PHASE_CONFIRM_RESUME, + QEMU_MIGRATION_PHASE_PREPARE_RESUME, + QEMU_MIGRATION_PHASE_FINISH_RESUME, QEMU_MIGRATION_PHASE_LAST } qemuMigrationJobPhase; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6dd643a38b..f752668b2f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3507,6 +3507,10 @@ qemuProcessRecoverMigrationIn(virQEMUDriver *driver, case QEMU_MIGRATION_PHASE_PERFORM3_DONE: case QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED: case QEMU_MIGRATION_PHASE_CONFIRM3: + case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: + case QEMU_MIGRATION_PHASE_BEGIN_RESUME: + case QEMU_MIGRATION_PHASE_PERFORM_RESUME: + case QEMU_MIGRATION_PHASE_CONFIRM_RESUME: case QEMU_MIGRATION_PHASE_LAST: /* N/A for incoming migration */ break; @@ -3540,6 +3544,10 @@ qemuProcessRecoverMigrationIn(virQEMUDriver *driver, return -1; } break; + + case QEMU_MIGRATION_PHASE_PREPARE_RESUME: + case QEMU_MIGRATION_PHASE_FINISH_RESUME: + return 1; } return 0; @@ -3548,7 +3556,8 @@ qemuProcessRecoverMigrationIn(virQEMUDriver *driver, /* * Returns - * -1 on error, the domain will be killed, + * -1 the domain should be killed (either after a successful migration or + * on error), * 0 the domain should remain running with the migration job discarded, * 1 the daemon was restarted during post-copy phase */ @@ -3556,6 +3565,7 @@ static int qemuProcessRecoverMigrationOut(virQEMUDriver *driver, virDomainObj *vm, qemuDomainJobObj *job, + virDomainJobStatus migStatus, virDomainState state, int reason, unsigned int *stopFlags) @@ -3571,6 +3581,9 @@ qemuProcessRecoverMigrationOut(virQEMUDriver *driver, case QEMU_MIGRATION_PHASE_PREPARE: case QEMU_MIGRATION_PHASE_FINISH2: case QEMU_MIGRATION_PHASE_FINISH3: + case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: + case QEMU_MIGRATION_PHASE_PREPARE_RESUME: + case QEMU_MIGRATION_PHASE_FINISH_RESUME: case QEMU_MIGRATION_PHASE_LAST: /* N/A for outgoing migration */ break; @@ -3621,6 +3634,18 @@ qemuProcessRecoverMigrationOut(virQEMUDriver *driver, /* migration completed, we need to kill the domain here */ *stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; return -1; + + case QEMU_MIGRATION_PHASE_CONFIRM_RESUME: + if (migStatus == VIR_DOMAIN_JOB_STATUS_HYPERVISOR_COMPLETED) { + /* migration completed, we need to kill the domain here */ + *stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; + return -1; + } + return 1; + + case QEMU_MIGRATION_PHASE_BEGIN_RESUME: + case QEMU_MIGRATION_PHASE_PERFORM_RESUME: + return 1; } if (resume) { @@ -3659,7 +3684,7 @@ qemuProcessRecoverMigration(virQEMUDriver *driver, qemuMigrationAnyRefreshStatus(driver, vm, VIR_ASYNC_JOB_NONE, &migStatus); if (job->asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT) { - rc = qemuProcessRecoverMigrationOut(driver, vm, job, + rc = qemuProcessRecoverMigrationOut(driver, vm, job, migStatus, state, reason, stopFlags); } else { rc = qemuProcessRecoverMigrationIn(driver, vm, job, state); -- 2.35.1

The check can reveal a serious bug in our migration code and we should not silently ignore it. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 58 ++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index edd3ac2a87..4c09caeace 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -147,9 +147,10 @@ qemuMigrationCheckPhase(virDomainObj *vm, if (phase < QEMU_MIGRATION_PHASE_POSTCOPY_FAILED && phase < priv->job.phase) { - VIR_ERROR(_("migration protocol going backwards %s => %s"), - qemuMigrationJobPhaseTypeToString(priv->job.phase), - qemuMigrationJobPhaseTypeToString(phase)); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("migration protocol going backwards %s => %s"), + qemuMigrationJobPhaseTypeToString(priv->job.phase), + qemuMigrationJobPhaseTypeToString(phase)); return -1; } @@ -157,22 +158,23 @@ qemuMigrationCheckPhase(virDomainObj *vm, } -static void ATTRIBUTE_NONNULL(1) +static int G_GNUC_WARN_UNUSED_RESULT qemuMigrationJobSetPhase(virDomainObj *vm, qemuMigrationJobPhase phase) { if (qemuMigrationCheckPhase(vm, phase) < 0) - return; + return -1; qemuDomainObjSetJobPhase(vm, phase); + return 0; } -static void ATTRIBUTE_NONNULL(1) +static int G_GNUC_WARN_UNUSED_RESULT qemuMigrationJobStartPhase(virDomainObj *vm, qemuMigrationJobPhase phase) { - qemuMigrationJobSetPhase(vm, phase); + return qemuMigrationJobSetPhase(vm, phase); } @@ -2567,8 +2569,9 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, * Otherwise we will start the async job later in the perform phase losing * change protection. */ - if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT) - qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_BEGIN3); + if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT && + qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_BEGIN3) < 0) + return NULL; if (!qemuMigrationSrcIsAllowed(driver, vm, true, flags)) return NULL; @@ -3117,7 +3120,9 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver, if (qemuMigrationJobStart(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, flags) < 0) goto cleanup; - qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PREPARE); + + if (qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PREPARE) < 0) + goto stopjob; /* Domain starts inactive, even if the domain XML had an id field. */ vm->def->id = -1; @@ -3637,7 +3642,8 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, else phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED; - qemuMigrationJobSetPhase(vm, phase); + if (qemuMigrationJobSetPhase(vm, phase) < 0) + return -1; if (!(mig = qemuMigrationCookieParse(driver, vm->def, priv->origname, priv, cookiein, cookieinlen, @@ -3722,7 +3728,9 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, else phase = QEMU_MIGRATION_PHASE_CONFIRM3; - qemuMigrationJobStartPhase(vm, phase); + if (qemuMigrationJobStartPhase(vm, phase) < 0) + goto cleanup; + virCloseCallbacksUnset(driver->closeCallbacks, vm, qemuMigrationSrcCleanup); qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); @@ -4885,7 +4893,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriver *driver, * until the migration is complete. */ VIR_DEBUG("Perform %p", sconn); - qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2); + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2)); if (flags & VIR_MIGRATE_TUNNELLED) ret = qemuMigrationSrcPerformTunnel(driver, vm, st, NULL, NULL, 0, NULL, NULL, @@ -5129,7 +5137,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, * confirm migration completion. */ VIR_DEBUG("Perform3 %p uri=%s", sconn, NULLSTR(uri)); - qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3); + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3)); VIR_FREE(cookiein); cookiein = g_steal_pointer(&cookieout); cookieinlen = cookieoutlen; @@ -5154,7 +5162,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriver *driver, if (ret < 0) { virErrorPreserveLast(&orig_err); } else { - qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE); + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE)); } /* If Perform returns < 0, then we need to cancel the VM @@ -5530,7 +5538,9 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, migParams, flags, dname, resource, &v3proto); } else { - qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2); + if (qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM2) < 0) + goto endjob; + ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, resource, NULL, NULL, 0, NULL, @@ -5622,7 +5632,9 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, return ret; } - qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3); + if (qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3) < 0) + goto endjob; + virCloseCallbacksUnset(driver->closeCallbacks, vm, qemuMigrationSrcCleanup); @@ -5636,7 +5648,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, goto endjob; } - qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE); + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE)); if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, qemuMigrationSrcCleanup) < 0) @@ -6203,9 +6215,10 @@ qemuMigrationDstFinish(virQEMUDriver *driver, ignore_value(virTimeMillisNow(&timeReceived)); - qemuMigrationJobStartPhase(vm, - v3proto ? QEMU_MIGRATION_PHASE_FINISH3 - : QEMU_MIGRATION_PHASE_FINISH2); + if (qemuMigrationJobStartPhase(vm, + v3proto ? QEMU_MIGRATION_PHASE_FINISH3 + : QEMU_MIGRATION_PHASE_FINISH2) < 0) + goto cleanup; qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup); g_clear_pointer(&priv->job.completed, virDomainJobDataFree); @@ -6277,7 +6290,8 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, else phase = QEMU_MIGRATION_PHASE_CONFIRM3; - qemuMigrationJobStartPhase(vm, phase); + if (qemuMigrationJobStartPhase(vm, phase) < 0) + return; if (job == VIR_ASYNC_JOB_MIGRATION_IN) qemuMigrationDstComplete(driver, vm, true, job, &priv->job); -- 2.35.1

We will want to update migration phase without affecting job ownership. Either in the thread that already owns the job or from an event handler which only changes the phase (of a job no-one owns) without assuming it. Let's move the ownership change to a new qemuDomainObjStartJobPhase helper and let qemuDomainObjSetJobPhase set the phase without touching ownership. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_domainjob.c | 44 +++++++++++++++++++++++++++++++++++---- src/qemu/qemu_domainjob.h | 3 +++ src/qemu/qemu_migration.c | 2 +- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index e76eb7f2cf..1f6d976558 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -717,6 +717,10 @@ qemuDomainJobDataToParams(virDomainJobData *jobData, } +/* + * Sets the job phase without changing the job owner. The owner is supposed to + * be 0 or the current thread, a warning is issued otherwise. + */ void qemuDomainObjSetJobPhase(virDomainObj *obj, int phase) @@ -731,19 +735,51 @@ qemuDomainObjSetJobPhase(virDomainObj *obj, virDomainAsyncJobTypeToString(priv->job.asyncJob), qemuDomainAsyncJobPhaseToString(priv->job.asyncJob, phase)); + if (priv->job.asyncOwner != 0 && + priv->job.asyncOwner != me) { + VIR_WARN("'%s' async job is owned by thread %llu, API '%s'", + virDomainAsyncJobTypeToString(priv->job.asyncJob), + priv->job.asyncOwner, + NULLSTR(priv->job.asyncOwnerAPI)); + } + + priv->job.phase = phase; + qemuDomainSaveStatus(obj); +} + + +/* + * Changes the job owner and sets the job phase. The current owner is supposed + * to be 0 or the current thread, a warning is issued otherwise. + */ +void +qemuDomainObjStartJobPhase(virDomainObj *obj, + int phase) +{ + qemuDomainObjPrivate *priv = obj->privateData; + unsigned long long me = virThreadSelfID(); + + if (!priv->job.asyncJob) + return; + + VIR_DEBUG("Starting phase '%s' of '%s' job", + qemuDomainAsyncJobPhaseToString(priv->job.asyncJob, phase), + virDomainAsyncJobTypeToString(priv->job.asyncJob)); + if (priv->job.asyncOwner == 0) { priv->job.asyncOwnerAPI = g_strdup(virThreadJobGet()); } else if (me != priv->job.asyncOwner) { - VIR_WARN("'%s' async job is owned by thread %llu", + VIR_WARN("'%s' async job is owned by thread %llu, API '%s'", virDomainAsyncJobTypeToString(priv->job.asyncJob), - priv->job.asyncOwner); + priv->job.asyncOwner, + NULLSTR(priv->job.asyncOwnerAPI)); } - priv->job.phase = phase; priv->job.asyncOwner = me; - qemuDomainSaveStatus(obj); + qemuDomainObjSetJobPhase(obj, phase); } + void qemuDomainObjSetAsyncJobMask(virDomainObj *obj, unsigned long long allowedJobs) diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h index 707d4e91ed..e8021a7f04 100644 --- a/src/qemu/qemu_domainjob.h +++ b/src/qemu/qemu_domainjob.h @@ -156,6 +156,9 @@ void qemuDomainObjEndAsyncJob(virDomainObj *obj); void qemuDomainObjAbortAsyncJob(virDomainObj *obj); void qemuDomainObjSetJobPhase(virDomainObj *obj, int phase); +void +qemuDomainObjStartJobPhase(virDomainObj *obj, + int phase); void qemuDomainObjSetAsyncJobMask(virDomainObj *obj, unsigned long long allowedJobs); int qemuDomainObjPreserveJob(virDomainObj *obj, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4c09caeace..1c015c0bcb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -165,7 +165,7 @@ qemuMigrationJobSetPhase(virDomainObj *vm, if (qemuMigrationCheckPhase(vm, phase) < 0) return -1; - qemuDomainObjSetJobPhase(vm, phase); + qemuDomainObjStartJobPhase(vm, phase); return 0; } -- 2.35.1

This phase marks a migration protocol as broken in a post-copy phase. Libvirt is no longer actively watching the migration in this phase as the migration API that started the migration failed. This may either happen when post-copy migration really fails (QEMU enters postcopy-paused migration state) or when the migration still progresses between both QEMU processes, but libvirt lost control of it because the connection between libvirt daemons (in p2p migration) or a daemon and client (non-p2p migration) was closed. For example, when one of the daemons was restarted. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - moved most of the last hunk to a separate patch src/qemu/qemu_migration.c | 15 +++++++++++---- src/qemu/qemu_process.c | 11 ++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 88702c94e4..302589b63c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2341,6 +2341,7 @@ qemuMigrationSrcCleanup(virDomainObj *vm, vm->def->name); if (virDomainObjIsPostcopy(vm, VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT)) { + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuMigrationSrcPostcopyFailed(vm); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); qemuMigrationJobContinue(vm); @@ -2352,8 +2353,10 @@ qemuMigrationSrcCleanup(virDomainObj *vm, } break; + case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_BEGIN_RESUME: case QEMU_MIGRATION_PHASE_PERFORM_RESUME: + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuMigrationSrcPostcopyFailed(vm); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); qemuMigrationJobContinue(vm); @@ -2374,7 +2377,6 @@ qemuMigrationSrcCleanup(virDomainObj *vm, case QEMU_MIGRATION_PHASE_PERFORM2: /* single phase outgoing migration; unreachable */ case QEMU_MIGRATION_PHASE_NONE: - case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_LAST: /* unreachable */ ; @@ -3744,6 +3746,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, flags, cancelled); if (virDomainObjIsFailedPostcopy(vm)) { + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); qemuMigrationJobContinue(vm); } else { @@ -5572,6 +5575,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, virErrorPreserveLast(&orig_err); if (virDomainObjIsFailedPostcopy(vm)) { + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); qemuMigrationJobContinue(vm); } else { @@ -5664,6 +5668,8 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, jobPriv->migParams, priv->job.apiFlags); qemuMigrationJobFinish(vm); } else { + if (ret < 0) + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); qemuMigrationJobContinue(vm); } @@ -5903,7 +5909,7 @@ qemuMigrationDstComplete(virQEMUDriver *driver, /* Guest is successfully running, so cancel previous auto destroy. There's * nothing to remove when we are resuming post-copy migration. */ - if (!virDomainObjIsFailedPostcopy(vm)) + if (job->phase < QEMU_MIGRATION_PHASE_POSTCOPY_FAILED) qemuProcessAutoDestroyRemove(driver, vm); /* Remove completed stats for post-copy, everything but timing fields @@ -6170,6 +6176,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, } if (virDomainObjIsFailedPostcopy(vm)) { + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuProcessAutoDestroyRemove(driver, vm); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); *finishJob = false; @@ -6290,9 +6297,9 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, vm->def->name); if (job == VIR_ASYNC_JOB_MIGRATION_IN) - phase = QEMU_MIGRATION_PHASE_FINISH3; + phase = QEMU_MIGRATION_PHASE_FINISH_RESUME; else - phase = QEMU_MIGRATION_PHASE_CONFIRM3; + phase = QEMU_MIGRATION_PHASE_CONFIRM_RESUME; if (qemuMigrationJobStartPhase(vm, phase) < 0) return; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f752668b2f..8a98c03395 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1555,9 +1555,12 @@ qemuProcessHandleMigrationStatus(qemuMonitor *mon G_GNUC_UNUSED, * watching it in any thread. Let's make sure the migration is properly * finished in case we get a "completed" event. */ - if (virDomainObjIsFailedPostcopy(vm) && priv->job.asyncOwner == 0) + if (virDomainObjIsPostcopy(vm, priv->job.current->operation) && + priv->job.phase == QEMU_MIGRATION_PHASE_POSTCOPY_FAILED && + priv->job.asyncOwner == 0) { qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION, priv->job.asyncJob, status, NULL); + } break; case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: @@ -3507,7 +3510,6 @@ qemuProcessRecoverMigrationIn(virQEMUDriver *driver, case QEMU_MIGRATION_PHASE_PERFORM3_DONE: case QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED: case QEMU_MIGRATION_PHASE_CONFIRM3: - case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_BEGIN_RESUME: case QEMU_MIGRATION_PHASE_PERFORM_RESUME: case QEMU_MIGRATION_PHASE_CONFIRM_RESUME: @@ -3545,6 +3547,7 @@ qemuProcessRecoverMigrationIn(virQEMUDriver *driver, } break; + case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_PREPARE_RESUME: case QEMU_MIGRATION_PHASE_FINISH_RESUME: return 1; @@ -3581,7 +3584,6 @@ qemuProcessRecoverMigrationOut(virQEMUDriver *driver, case QEMU_MIGRATION_PHASE_PREPARE: case QEMU_MIGRATION_PHASE_FINISH2: case QEMU_MIGRATION_PHASE_FINISH3: - case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_PREPARE_RESUME: case QEMU_MIGRATION_PHASE_FINISH_RESUME: case QEMU_MIGRATION_PHASE_LAST: @@ -3643,6 +3645,7 @@ qemuProcessRecoverMigrationOut(virQEMUDriver *driver, } return 1; + case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_BEGIN_RESUME: case QEMU_MIGRATION_PHASE_PERFORM_RESUME: return 1; @@ -3694,6 +3697,8 @@ qemuProcessRecoverMigration(virQEMUDriver *driver, return -1; if (rc > 0) { + job->phase = QEMU_MIGRATION_PHASE_POSTCOPY_FAILED; + if (migStatus == VIR_DOMAIN_JOB_STATUS_POSTCOPY) { VIR_DEBUG("Post-copy migration of domain %s still running, it will be handled as unattended", vm->def->name); -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change docs/manpages/virsh.rst | 9 +++++++-- tools/virsh-domain.c | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 965b89c99d..ecdb54fcd5 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3246,7 +3246,8 @@ migrate migrate [--live] [--offline] [--direct] [--p2p [--tunnelled]] [--persistent] [--undefinesource] [--suspend] [--copy-storage-all] [--copy-storage-inc] [--change-protection] [--unsafe] [--verbose] - [--rdma-pin-all] [--abort-on-error] [--postcopy] [--postcopy-after-precopy] + [--rdma-pin-all] [--abort-on-error] [--postcopy] + [--postcopy-after-precopy] [--postcopy-resume] domain desturi [migrateuri] [graphicsuri] [listen-address] [dname] [--timeout seconds [--timeout-suspend | --timeout-postcopy]] [--xml file] [--migrate-disks disk-list] [--disks-port port] @@ -3302,7 +3303,11 @@ Once migration is running, the user may switch to post-copy using the automatically switch to post-copy after the first pass of pre-copy is finished. The maximum bandwidth consumed during the post-copy phase may be limited using *--postcopy-bandwidth*. The maximum bandwidth consumed during the pre-copy phase -may be limited using *--bandwidth*. +may be limited using *--bandwidth*. In case connection between the hosts breaks +while migration is in post-copy mode, the domain cannot be resumed on either +source or destination host and the ``migrate`` command will report an error +leaving the domain active on both hosts. To recover from such situation repeat +the original ``migrate`` command with an additional *--postcopy-resume* flag. *--auto-converge* forces convergence during live migration. The initial guest CPU throttling rate can be set with *auto-converge-initial*. If the diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bac4cc0fb6..dd7862b5e5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10807,6 +10807,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_BOOL, .help = N_("automatically switch to post-copy migration after one pass of pre-copy") }, + {.name = "postcopy-resume", + .type = VSH_OT_BOOL, + .help = N_("resume failed post-copy migration") + }, {.name = "migrateuri", .type = VSH_OT_STRING, .completer = virshCompleteEmpty, @@ -11210,6 +11214,9 @@ doMigrate(void *opaque) if (vshCommandOptBool(cmd, "postcopy")) flags |= VIR_MIGRATE_POSTCOPY; + if (vshCommandOptBool(cmd, "postcopy-resume")) + flags |= VIR_MIGRATE_POSTCOPY_RESUME; + if (vshCommandOptBool(cmd, "tls")) flags |= VIR_MIGRATE_TLS; @@ -11314,6 +11321,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS("live", "offline"); VSH_EXCLUSIVE_OPTIONS("timeout-suspend", "timeout-postcopy"); VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy"); + VSH_REQUIRE_OPTION("postcopy-resume", "postcopy"); VSH_REQUIRE_OPTION("timeout-postcopy", "postcopy"); VSH_REQUIRE_OPTION("persistent-xml", "persistent"); VSH_REQUIRE_OPTION("tls-destination", "tls"); -- 2.35.1

Mostly we just need to check whether the domain is in a failed post-copy migration that can be resumed. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - s/return NULL/return false/ - dropped line breaks from error messages - dropped driver argument when calling qemuMigrationSrcBeginXML src/qemu/qemu_migration.c | 139 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 302589b63c..45f2788a34 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2685,6 +2685,139 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, flags); } + +static bool +qemuMigrationAnyCanResume(virDomainObj *vm, + virDomainAsyncJob job, + unsigned long flags, + qemuMigrationJobPhase expectedPhase) +{ + qemuDomainObjPrivate *priv = vm->privateData; + + VIR_DEBUG("vm=%p, job=%s, flags=0x%lx, expectedPhase=%s", + vm, virDomainAsyncJobTypeToString(job), flags, + qemuDomainAsyncJobPhaseToString(VIR_ASYNC_JOB_MIGRATION_OUT, + expectedPhase)); + + if (!(flags & VIR_MIGRATE_POSTCOPY)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("resuming failed post-copy migration requires post-copy to be enabled")); + return false; + } + + /* This should never happen since POSTCOPY_RESUME is newer than + * CHANGE_PROTECTION, but let's check it anyway in case we're talking to + * a weired client. + */ + if (job == VIR_ASYNC_JOB_MIGRATION_OUT && + expectedPhase < QEMU_MIGRATION_PHASE_PERFORM_RESUME && + !(flags & VIR_MIGRATE_CHANGE_PROTECTION)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("resuming failed post-copy migration requires change protection")); + return false; + } + + if (!qemuMigrationJobIsActive(vm, job)) + return false; + + if (priv->job.asyncOwner != 0 && + priv->job.asyncOwner != virThreadSelfID()) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("migration of domain %s is being actively monitored by another thread"), + vm->def->name); + return false; + } + + if (!virDomainObjIsPostcopy(vm, priv->job.current->operation)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("migration of domain %s is not in post-copy phase"), + vm->def->name); + return false; + } + + if (priv->job.phase < QEMU_MIGRATION_PHASE_POSTCOPY_FAILED && + !virDomainObjIsFailedPostcopy(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("post-copy migration of domain %s has not failed"), + vm->def->name); + return false; + } + + if (priv->job.phase > expectedPhase) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("resuming failed post-copy migration of domain %s already in progress"), + vm->def->name); + return false; + } + + return true; +} + + +static char * +qemuMigrationSrcBeginResume(virQEMUDriver *driver, + virDomainObj *vm, + const char *xmlin, + char **cookieout, + int *cookieoutlen, + unsigned long flags) +{ + virDomainJobStatus status; + + if (qemuMigrationAnyRefreshStatus(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, + &status) < 0) + return NULL; + + if (status != VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("QEMU reports migration is still running")); + return NULL; + } + + return qemuMigrationSrcBeginXML(vm, xmlin, + cookieout, cookieoutlen, 0, NULL, 0, flags); +} + + +static char * +qemuMigrationSrcBeginResumePhase(virConnectPtr conn, + virQEMUDriver *driver, + virDomainObj *vm, + const char *xmlin, + char **cookieout, + int *cookieoutlen, + unsigned long flags) +{ + g_autofree char *xml = NULL; + + VIR_DEBUG("vm=%p", vm); + + if (!qemuMigrationAnyCanResume(vm, VIR_ASYNC_JOB_MIGRATION_OUT, flags, + QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)) + return NULL; + + if (qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_BEGIN_RESUME) < 0) + return NULL; + + virCloseCallbacksUnset(driver->closeCallbacks, vm, + qemuMigrationSrcCleanup); + qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); + + xml = qemuMigrationSrcBeginResume(driver, vm, xmlin, cookieout, cookieoutlen, flags); + + if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, + qemuMigrationSrcCleanup) < 0) + g_clear_pointer(&xml, g_free); + + if (!xml) + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); + + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); + qemuMigrationJobContinue(vm); + return g_steal_pointer(&xml); +} + + char * qemuMigrationSrcBegin(virConnectPtr conn, virDomainObj *vm, @@ -2710,6 +2843,12 @@ qemuMigrationSrcBegin(virConnectPtr conn, goto cleanup; } + if (flags & VIR_MIGRATE_POSTCOPY_RESUME) { + ret = qemuMigrationSrcBeginResumePhase(conn, driver, vm, xmlin, + cookieout, cookieoutlen, flags); + goto cleanup; + } + if ((flags & VIR_MIGRATE_CHANGE_PROTECTION)) { if (qemuMigrationJobStart(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, flags) < 0) -- 2.35.1

To make the code flow a bit more sensible. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - fixed "qmeu" typo in the commit message src/qemu/qemu_migration.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 45f2788a34..3ed1cf3498 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5780,29 +5780,27 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, } if (qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3) < 0) - goto endjob; + goto cleanup; virCloseCallbacksUnset(driver->closeCallbacks, vm, qemuMigrationSrcCleanup); - ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, - cookieout, cookieoutlen, - flags, resource, NULL, graphicsuri, - nmigrate_disks, migrate_disks, migParams, nbdURI); - - if (ret < 0) { - qemuMigrationSrcRestoreDomainState(driver, vm); - goto endjob; - } - - ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE)); + if (qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, resource, NULL, graphicsuri, + nmigrate_disks, migrate_disks, migParams, nbdURI) < 0) + goto cleanup; if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, qemuMigrationSrcCleanup) < 0) - goto endjob; + goto cleanup; - endjob: + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE)); + ret = 0; + + cleanup: if (ret < 0 && !virDomainObjIsFailedPostcopy(vm)) { + qemuMigrationSrcRestoreDomainState(driver, vm); qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, jobPriv->migParams, priv->job.apiFlags); qemuMigrationJobFinish(vm); -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 8 ++++++-- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 51feb785f5..9996227057 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4399,8 +4399,12 @@ qemuMigrationSrcStart(virDomainObj *vm, } /* log start of migration */ - if ((timestamp = virTimeStringNow()) != NULL) - qemuDomainLogAppendMessage(driver, vm, "%s: initiating migration\n", timestamp); + if ((timestamp = virTimeStringNow()) != NULL) { + if (migrateFlags & QEMU_MONITOR_MIGRATE_RESUME) + qemuDomainLogAppendMessage(driver, vm, "%s: resuming migration\n", timestamp); + else + qemuDomainLogAppendMessage(driver, vm, "%s: initiating migration\n", timestamp); + } switch (spec->destType) { case MIGRATION_DEST_HOST: diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 41e8db945c..98496b9037 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -870,6 +870,7 @@ typedef enum { QEMU_MONITOR_MIGRATE_BACKGROUND = 1 << 0, QEMU_MONITOR_MIGRATE_NON_SHARED_DISK = 1 << 1, /* migration with non-shared storage with full disk copy */ QEMU_MONITOR_MIGRATE_NON_SHARED_INC = 1 << 2, /* migration with non-shared storage with incremental copy */ + QEMU_MONITOR_MIGRATE_RESUME = 1 << 3, /* resume failed post-copy migration */ QEMU_MONITOR_MIGRATION_FLAGS_LAST } QEMU_MONITOR_MIGRATE; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 86b7f615c8..8b026e8ade 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3416,10 +3416,12 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon, bool detach = !!(flags & QEMU_MONITOR_MIGRATE_BACKGROUND); bool blk = !!(flags & QEMU_MONITOR_MIGRATE_NON_SHARED_DISK); bool inc = !!(flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC); + bool resume = !!(flags & QEMU_MONITOR_MIGRATE_RESUME); g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("migrate", "b:detach", detach, "b:blk", blk, "b:inc", inc, + "b:resume", resume, "s:uri", uri, NULL); g_autoptr(virJSONValue) reply = NULL; -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - moved "qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob)" to patch 12 (qemu: Keep migration job active after failed post-copy) src/qemu/qemu_migration.c | 48 +++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2bc479592c..f87d123836 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3777,15 +3777,19 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, virCheckFlags(QEMU_MIGRATION_FLAGS, -1); - /* Keep the original migration phase in case post-copy failed as the job - * will stay active even though migration API finishes with an error. - */ - if (virDomainObjIsFailedPostcopy(vm)) + if (flags & VIR_MIGRATE_POSTCOPY_RESUME) { + phase = QEMU_MIGRATION_PHASE_CONFIRM_RESUME; + } else if (virDomainObjIsFailedPostcopy(vm)) { + /* Keep the original migration phase in case post-copy failed as the + * job will stay active even though migration API finishes with an + * error. + */ phase = priv->job.phase; - else if (retcode == 0) + } else if (retcode == 0) { phase = QEMU_MIGRATION_PHASE_CONFIRM3; - else + } else { phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED; + } if (qemuMigrationJobStartPhase(vm, phase) < 0) return -1; @@ -3860,18 +3864,28 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, qemuDomainObjPrivate *priv = vm->privateData; int ret = -1; - if (!qemuMigrationJobIsActive(vm, VIR_ASYNC_JOB_MIGRATION_OUT)) - goto cleanup; + VIR_DEBUG("vm=%p, flags=0x%x, cancelled=%d", vm, flags, cancelled); - /* Keep the original migration phase in case post-copy failed as the job - * will stay active even though migration API finishes with an error. - */ - if (virDomainObjIsFailedPostcopy(vm)) - phase = priv->job.phase; - else if (cancelled) - phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED; - else - phase = QEMU_MIGRATION_PHASE_CONFIRM3; + if (flags & VIR_MIGRATE_POSTCOPY_RESUME) { + if (!qemuMigrationAnyCanResume(vm, VIR_ASYNC_JOB_MIGRATION_OUT, flags, + QEMU_MIGRATION_PHASE_PERFORM_RESUME)) + goto cleanup; + phase = QEMU_MIGRATION_PHASE_CONFIRM_RESUME; + } else { + if (!qemuMigrationJobIsActive(vm, VIR_ASYNC_JOB_MIGRATION_OUT)) + goto cleanup; + + /* Keep the original migration phase in case post-copy failed as the + * job will stay active even though migration API finishes with an + * error. + */ + if (virDomainObjIsFailedPostcopy(vm)) + phase = priv->job.phase; + else if (cancelled) + phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED; + else + phase = QEMU_MIGRATION_PHASE_CONFIRM3; + } if (qemuMigrationJobStartPhase(vm, phase) < 0) goto cleanup; -- 2.35.1

Offline migration jumps over a big part of qemuMigrationDstPrepareFresh. Let's move that part into a new qemuMigrationDstPrepareActive function to make the code easier to follow. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - dropped line breaks from error messages - fixed indentation - dropped VIR_MIGRATE_OFFLINE check from qemuMigrationDstPrepareActive src/qemu/qemu_migration.c | 371 +++++++++++++++++++++----------------- 1 file changed, 203 insertions(+), 168 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d4683a9922..53236b7239 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3083,6 +3083,197 @@ qemuMigrationDstPrepareAnyBlockDirtyBitmaps(virDomainObj *vm, } +static int +qemuMigrationDstPrepareActive(virQEMUDriver *driver, + virDomainObj *vm, + virConnectPtr dconn, + qemuMigrationCookie *mig, + virStreamPtr st, + const char *protocol, + unsigned short port, + const char *listenAddress, + size_t nmigrate_disks, + const char **migrate_disks, + int nbdPort, + const char *nbdURI, + qemuMigrationParams *migParams, + unsigned long flags) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuDomainJobPrivate *jobPriv = priv->job.privateData; + qemuProcessIncomingDef *incoming = NULL; + g_autofree char *tlsAlias = NULL; + virObjectEvent *event = NULL; + virErrorPtr origErr = NULL; + int dataFD[2] = { -1, -1 }; + bool stopProcess = false; + unsigned int startFlags; + bool relabel = false; + bool tunnel = !!st; + int ret = -1; + int rv; + + if (STREQ_NULLABLE(protocol, "rdma") && + !virMemoryLimitIsSet(vm->def->mem.hard_limit)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot start RDMA migration with no memory hard limit set")); + goto error; + } + + if (qemuMigrationDstPrecreateStorage(vm, mig->nbd, + nmigrate_disks, migrate_disks, + !!(flags & VIR_MIGRATE_NON_SHARED_INC)) < 0) + goto error; + + if (tunnel && + virPipe(dataFD) < 0) + goto error; + + startFlags = VIR_QEMU_PROCESS_START_AUTODESTROY; + + if (qemuProcessInit(driver, vm, mig->cpu, VIR_ASYNC_JOB_MIGRATION_IN, + true, startFlags) < 0) + goto error; + stopProcess = true; + + if (!(incoming = qemuMigrationDstPrepare(vm, tunnel, protocol, + listenAddress, port, + dataFD[0]))) + goto error; + + if (qemuProcessPrepareDomain(driver, vm, startFlags) < 0) + goto error; + + if (qemuProcessPrepareHost(driver, vm, startFlags) < 0) + goto error; + + rv = qemuProcessLaunch(dconn, driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + incoming, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, + startFlags); + if (rv < 0) { + if (rv == -2) + relabel = true; + goto error; + } + relabel = true; + + if (tunnel) { + if (virFDStreamOpen(st, dataFD[1]) < 0) { + virReportSystemError(errno, "%s", + _("cannot pass pipe for tunnelled migration")); + goto error; + } + dataFD[1] = -1; /* 'st' owns the FD now & will close it */ + } + + if (STREQ_NULLABLE(protocol, "rdma") && + vm->def->mem.hard_limit > 0 && + virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) { + goto error; + } + + if (qemuMigrationDstPrepareAnyBlockDirtyBitmaps(vm, mig, migParams, flags) < 0) + goto error; + + if (qemuMigrationParamsCheck(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + migParams, mig->caps->automatic) < 0) + goto error; + + /* Migrations using TLS need to add the "tls-creds-x509" object and + * set the migration TLS parameters */ + if (flags & VIR_MIGRATE_TLS) { + if (qemuMigrationParamsEnableTLS(driver, vm, true, + VIR_ASYNC_JOB_MIGRATION_IN, + &tlsAlias, NULL, + migParams) < 0) + goto error; + } else { + if (qemuMigrationParamsDisableTLS(vm, migParams) < 0) + goto error; + } + + if (qemuMigrationParamsApply(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + migParams) < 0) + goto error; + + if (mig->nbd && + flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { + const char *nbdTLSAlias = NULL; + + if (flags & VIR_MIGRATE_TLS) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_TLS)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("QEMU NBD server does not support TLS transport")); + goto error; + } + + nbdTLSAlias = tlsAlias; + } + + if (qemuMigrationDstStartNBDServer(driver, vm, incoming->address, + nmigrate_disks, migrate_disks, + nbdPort, nbdURI, + nbdTLSAlias) < 0) { + goto error; + } + } + + if (mig->lockState) { + VIR_DEBUG("Received lockstate %s", mig->lockState); + VIR_FREE(priv->lockState); + priv->lockState = g_steal_pointer(&mig->lockState); + } else { + VIR_DEBUG("Received no lockstate"); + } + + if (qemuMigrationDstRun(driver, vm, incoming->uri, + VIR_ASYNC_JOB_MIGRATION_IN) < 0) + goto error; + + if (qemuProcessFinishStartup(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + false, VIR_DOMAIN_PAUSED_MIGRATION) < 0) + goto error; + + virDomainAuditStart(vm, "migrated", true); + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_MIGRATED); + + ret = 0; + + cleanup: + qemuProcessIncomingDefFree(incoming); + VIR_FORCE_CLOSE(dataFD[0]); + VIR_FORCE_CLOSE(dataFD[1]); + virObjectEventStateQueue(driver->domainEventState, event); + virErrorRestore(&origErr); + return ret; + + error: + virErrorPreserveLast(&origErr); + qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + jobPriv->migParams, priv->job.apiFlags); + + if (stopProcess) { + unsigned int stopFlags = VIR_QEMU_PROCESS_STOP_MIGRATED; + if (!relabel) + stopFlags |= VIR_QEMU_PROCESS_STOP_NO_RELABEL; + virDomainAuditStart(vm, "migrated", false); + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + VIR_ASYNC_JOB_MIGRATION_IN, stopFlags); + /* release if port is auto selected which is not the case if + * it is given in parameters + */ + if (nbdPort == 0) + virPortAllocatorRelease(priv->nbdPort); + priv->nbdPort = 0; + } + goto cleanup; +} + + static int qemuMigrationDstPrepareFresh(virQEMUDriver *driver, virConnectPtr dconn, @@ -3105,32 +3296,20 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, unsigned long flags) { virDomainObj *vm = NULL; - virObjectEvent *event = NULL; virErrorPtr origErr; int ret = -1; - int dataFD[2] = { -1, -1 }; qemuDomainObjPrivate *priv = NULL; qemuMigrationCookie *mig = NULL; - qemuDomainJobPrivate *jobPriv = NULL; - bool tunnel = !!st; g_autofree char *xmlout = NULL; - unsigned int cookieFlags; - unsigned int startFlags; - qemuProcessIncomingDef *incoming = NULL; + unsigned int cookieFlags = 0; bool taint_hook = false; - bool stopProcess = false; - bool relabel = false; - int rv; - g_autofree char *tlsAlias = NULL; VIR_DEBUG("name=%s, origname=%s, protocol=%s, port=%hu, " "listenAddress=%s, nbdPort=%d, nbdURI=%s, flags=0x%lx", (*def)->name, NULLSTR(origname), protocol, port, listenAddress, nbdPort, NULLSTR(nbdURI), flags); - if (flags & VIR_MIGRATE_OFFLINE) { - cookieFlags = 0; - } else { + if (!(flags & VIR_MIGRATE_OFFLINE)) { cookieFlags = QEMU_MIGRATION_COOKIE_GRAPHICS | QEMU_MIGRATION_COOKIE_CAPS; } @@ -3203,7 +3382,6 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, goto cleanup; priv = vm->privateData; - jobPriv = priv->job.privateData; priv->origname = g_strdup(origname); if (taint_hook) { @@ -3211,19 +3389,6 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, priv->hookRun = true; } - if (STREQ_NULLABLE(protocol, "rdma") && - !virMemoryLimitIsSet(vm->def->mem.hard_limit)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot start RDMA migration with no memory hard " - "limit set")); - goto cleanup; - } - - if (qemuMigrationDstPrecreateStorage(vm, mig->nbd, - nmigrate_disks, migrate_disks, - !!(flags & VIR_MIGRATE_NON_SHARED_INC)) < 0) - goto cleanup; - if (qemuMigrationJobStart(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, flags) < 0) goto cleanup; @@ -3234,122 +3399,21 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, /* Domain starts inactive, even if the domain XML had an id field. */ vm->def->id = -1; - if (flags & VIR_MIGRATE_OFFLINE) - goto done; - - if (tunnel && - virPipe(dataFD) < 0) - goto stopjob; - - startFlags = VIR_QEMU_PROCESS_START_AUTODESTROY; - - if (qemuProcessInit(driver, vm, mig->cpu, VIR_ASYNC_JOB_MIGRATION_IN, - true, startFlags) < 0) - goto stopjob; - stopProcess = true; - - if (!(incoming = qemuMigrationDstPrepare(vm, tunnel, protocol, - listenAddress, port, - dataFD[0]))) - goto stopjob; - - if (qemuProcessPrepareDomain(driver, vm, startFlags) < 0) - goto stopjob; - - if (qemuProcessPrepareHost(driver, vm, startFlags) < 0) - goto stopjob; - - rv = qemuProcessLaunch(dconn, driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, - incoming, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START, - startFlags); - if (rv < 0) { - if (rv == -2) - relabel = true; - goto stopjob; - } - relabel = true; - - if (tunnel) { - if (virFDStreamOpen(st, dataFD[1]) < 0) { - virReportSystemError(errno, "%s", - _("cannot pass pipe for tunnelled migration")); - goto stopjob; - } - dataFD[1] = -1; /* 'st' owns the FD now & will close it */ - } - - if (STREQ_NULLABLE(protocol, "rdma") && - vm->def->mem.hard_limit > 0 && - virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) { - goto stopjob; - } - - if (qemuMigrationDstPrepareAnyBlockDirtyBitmaps(vm, mig, migParams, flags) < 0) - goto stopjob; - - if (qemuMigrationParamsCheck(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, - migParams, mig->caps->automatic) < 0) - goto stopjob; - - /* Migrations using TLS need to add the "tls-creds-x509" object and - * set the migration TLS parameters */ - if (flags & VIR_MIGRATE_TLS) { - if (qemuMigrationParamsEnableTLS(driver, vm, true, - VIR_ASYNC_JOB_MIGRATION_IN, - &tlsAlias, NULL, - migParams) < 0) - goto stopjob; - } else { - if (qemuMigrationParamsDisableTLS(vm, migParams) < 0) + if (!(flags & VIR_MIGRATE_OFFLINE)) { + if (qemuMigrationDstPrepareActive(driver, vm, dconn, mig, st, + protocol, port, listenAddress, + nmigrate_disks, migrate_disks, + nbdPort, nbdURI, + migParams, flags) < 0) { goto stopjob; - } - - if (qemuMigrationParamsApply(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, - migParams) < 0) - goto stopjob; - - if (mig->nbd && - flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) { - const char *nbdTLSAlias = NULL; - - if (flags & VIR_MIGRATE_TLS) { - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_TLS)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("QEMU NBD server does not support TLS transport")); - goto stopjob; - } - - nbdTLSAlias = tlsAlias; } - if (qemuMigrationDstStartNBDServer(driver, vm, incoming->address, - nmigrate_disks, migrate_disks, - nbdPort, nbdURI, - nbdTLSAlias) < 0) { - goto stopjob; - } - cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; + if (mig->nbd && + flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) + cookieFlags |= QEMU_MIGRATION_COOKIE_NBD; } - if (mig->lockState) { - VIR_DEBUG("Received lockstate %s", mig->lockState); - VIR_FREE(priv->lockState); - priv->lockState = g_steal_pointer(&mig->lockState); - } else { - VIR_DEBUG("Received no lockstate"); - } - - if (qemuMigrationDstRun(driver, vm, incoming->uri, - VIR_ASYNC_JOB_MIGRATION_IN) < 0) - goto stopjob; - - if (qemuProcessFinishStartup(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, - false, VIR_DOMAIN_PAUSED_MIGRATION) < 0) - goto stopjob; - - done: if (qemuMigrationCookieFormat(mig, driver, vm, QEMU_MIGRATION_DESTINATION, cookieout, cookieoutlen, cookieFlags) < 0) { @@ -3362,13 +3426,6 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, qemuDomainCleanupAdd(vm, qemuMigrationDstPrepareCleanup); - if (!(flags & VIR_MIGRATE_OFFLINE)) { - virDomainAuditStart(vm, "migrated", true); - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_STARTED_MIGRATED); - } - /* We keep the job active across API calls until the finish() call. * This prevents any other APIs being invoked while incoming * migration is taking place. @@ -3386,41 +3443,19 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, cleanup: virErrorPreserveLast(&origErr); - qemuProcessIncomingDefFree(incoming); - VIR_FORCE_CLOSE(dataFD[0]); - VIR_FORCE_CLOSE(dataFD[1]); if (ret < 0 && priv) { /* priv is set right after vm is added to the list of domains * and there is no 'goto cleanup;' in the middle of those */ VIR_FREE(priv->origname); - /* release if port is auto selected which is not the case if - * it is given in parameters - */ - if (nbdPort == 0) - virPortAllocatorRelease(priv->nbdPort); - priv->nbdPort = 0; virDomainObjRemoveTransientDef(vm); qemuDomainRemoveInactive(driver, vm); } virDomainObjEndAPI(&vm); - virObjectEventStateQueue(driver->domainEventState, event); qemuMigrationCookieFree(mig); virErrorRestore(&origErr); return ret; stopjob: - qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, - jobPriv->migParams, priv->job.apiFlags); - - if (stopProcess) { - unsigned int stopFlags = VIR_QEMU_PROCESS_STOP_MIGRATED; - if (!relabel) - stopFlags |= VIR_QEMU_PROCESS_STOP_NO_RELABEL; - virDomainAuditStart(vm, "migrated", false); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, - VIR_ASYNC_JOB_MIGRATION_IN, stopFlags); - } - qemuMigrationJobFinish(vm); goto cleanup; } -- 2.35.1

The function is now called qemuMigrationAnyConnectionClosed to make it clear it is supposed to handle broken connection during migration. It will soon be used on both sides of migration so the "Src" part was changed to "Any" to avoid renaming the function twice in a row. The original *Cleanup name could easily be confused with cleanup callbacks called when a domain is destroyed. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e888384570..eee73d47ca 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2309,8 +2309,8 @@ qemuMigrationDstRun(virQEMUDriver *driver, * qemuDomainMigratePerform3 and qemuDomainMigrateConfirm3. */ static void -qemuMigrationSrcCleanup(virDomainObj *vm, - virConnectPtr conn) +qemuMigrationAnyConnectionClosed(virDomainObj *vm, + virConnectPtr conn) { qemuDomainObjPrivate *priv = vm->privateData; virQEMUDriver *driver = priv->driver; @@ -2800,13 +2800,13 @@ qemuMigrationSrcBeginResumePhase(virConnectPtr conn, return NULL; virCloseCallbacksUnset(driver->closeCallbacks, vm, - qemuMigrationSrcCleanup); + qemuMigrationAnyConnectionClosed); qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); xml = qemuMigrationSrcBeginResume(driver, vm, xmlin, cookieout, cookieoutlen, flags); if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, - qemuMigrationSrcCleanup) < 0) + qemuMigrationAnyConnectionClosed) < 0) g_clear_pointer(&xml, g_free); if (!xml) @@ -2886,7 +2886,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, * place. */ if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, - qemuMigrationSrcCleanup) < 0) + qemuMigrationAnyConnectionClosed) < 0) goto endjob; } @@ -3962,7 +3962,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, goto cleanup; virCloseCallbacksUnset(driver->closeCallbacks, vm, - qemuMigrationSrcCleanup); + qemuMigrationAnyConnectionClosed); qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); ret = qemuMigrationSrcConfirmPhase(driver, vm, @@ -5933,7 +5933,7 @@ qemuMigrationSrcPerformResume(virQEMUDriver *driver, return -1; virCloseCallbacksUnset(driver->closeCallbacks, vm, - qemuMigrationSrcCleanup); + qemuMigrationAnyConnectionClosed); qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); ret = qemuMigrationSrcPerformNative(driver, vm, NULL, uri, @@ -5942,7 +5942,7 @@ qemuMigrationSrcPerformResume(virQEMUDriver *driver, 0, NULL, NULL, 0, NULL, NULL, NULL); if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, - qemuMigrationSrcCleanup) < 0) + qemuMigrationAnyConnectionClosed) < 0) ret = -1; if (ret < 0) @@ -5998,7 +5998,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, goto cleanup; virCloseCallbacksUnset(driver->closeCallbacks, vm, - qemuMigrationSrcCleanup); + qemuMigrationAnyConnectionClosed); if (qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen, cookieout, cookieoutlen, @@ -6007,7 +6007,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, goto cleanup; if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, - qemuMigrationSrcCleanup) < 0) + qemuMigrationAnyConnectionClosed) < 0) goto cleanup; ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE)); -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - part of this patch squashed into the previous one src/qemu/qemu_migration.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 92833ccbe3..1396c1d9b4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2350,12 +2350,12 @@ qemuMigrationAnyConnectionClosed(virDomainObj *vm, case QEMU_MIGRATION_PHASE_POSTCOPY_FAILED: case QEMU_MIGRATION_PHASE_BEGIN_RESUME: case QEMU_MIGRATION_PHASE_PERFORM_RESUME: + case QEMU_MIGRATION_PHASE_PREPARE_RESUME: VIR_DEBUG("Connection closed while resuming failed post-copy migration"); postcopy = true; break; case QEMU_MIGRATION_PHASE_PREPARE: - case QEMU_MIGRATION_PHASE_PREPARE_RESUME: /* incoming migration; the domain will be autodestroyed */ return; @@ -2379,6 +2379,8 @@ qemuMigrationAnyConnectionClosed(virDomainObj *vm, if (postcopy) { if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT) qemuMigrationSrcPostcopyFailed(vm); + else + qemuMigrationDstPostcopyFailed(vm); ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); qemuMigrationJobContinue(vm); -- 2.35.1

The QEMU process is already running, all we need to do is to call migrate-recover QMP command. Except for some checks and cookie handling, of course. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 99 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 616f812ef1..dbebd215b4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3474,6 +3474,98 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, } +static int +qemuMigrationDstPrepareResume(virQEMUDriver *driver, + virConnectPtr conn, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + virDomainDef *def, + const char *origname, + const char *protocol, + unsigned short port, + const char *listenAddress, + unsigned long flags) +{ + g_autoptr(qemuMigrationCookie) mig = NULL; + qemuProcessIncomingDef *incoming = NULL; + qemuDomainObjPrivate *priv; + virDomainJobStatus status; + virDomainObj *vm; + int ret = -1; + + VIR_DEBUG("name=%s, origname=%s, protocol=%s, port=%hu, " + "listenAddress=%s, flags=0x%lx", + def->name, NULLSTR(origname), protocol, port, + NULLSTR(listenAddress), flags); + + vm = virDomainObjListFindByName(driver->domains, def->name); + if (!vm) { + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching name '%s'"), def->name); + qemuMigrationDstErrorReport(driver, def->name); + return -1; + } + priv = vm->privateData; + + if (!qemuMigrationAnyCanResume(vm, VIR_ASYNC_JOB_MIGRATION_IN, flags, + QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)) + goto cleanup; + + if (qemuMigrationJobStartPhase(vm, QEMU_MIGRATION_PHASE_PREPARE_RESUME) < 0) + goto cleanup; + + qemuDomainCleanupRemove(vm, qemuProcessCleanupMigrationJob); + + if (qemuMigrationAnyRefreshStatus(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, + &status) < 0) + goto cleanup; + + if (status != VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("QEMU reports migration is still running")); + goto cleanup; + } + + if (!(mig = qemuMigrationCookieParse(driver, def, origname, NULL, + cookiein, cookieinlen, + QEMU_MIGRATION_COOKIE_CAPS))) + goto cleanup; + + if (!(incoming = qemuMigrationDstPrepare(vm, false, protocol, + listenAddress, port, -1))) + goto cleanup; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN) < 0) + goto cleanup; + + ret = qemuMonitorMigrateRecover(priv->mon, incoming->uri); + qemuDomainObjExitMonitor(vm); + + if (ret < 0) + goto cleanup; + + if (qemuMigrationCookieFormat(mig, driver, vm, + QEMU_MIGRATION_DESTINATION, + cookieout, cookieoutlen, + QEMU_MIGRATION_COOKIE_CAPS) < 0) + VIR_WARN("Unable to encode migration cookie"); + + virCloseCallbacksSet(driver->closeCallbacks, vm, conn, + qemuMigrationAnyConnectionClosed); + + cleanup: + qemuProcessIncomingDefFree(incoming); + if (ret < 0) + ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); + qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); + qemuMigrationJobContinue(vm); + virDomainObjEndAPI(&vm); + return ret; +} + + static int qemuMigrationDstPrepareAny(virQEMUDriver *driver, virConnectPtr dconn, @@ -3538,6 +3630,13 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver, return -1; } + if (flags & VIR_MIGRATE_POSTCOPY_RESUME) { + return qemuMigrationDstPrepareResume(driver, dconn, cookiein, cookieinlen, + cookieout, cookieoutlen, + *def, origname, protocol, port, + listenAddress, flags); + } + return qemuMigrationDstPrepareFresh(driver, dconn, cookiein, cookieinlen, cookieout, cookieoutlen, -- 2.35.1

On Fri, Jun 03, 2022 at 14:27:55 +0200, Jiri Denemark wrote: Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Fri, Jun 03, 2022 at 16:10:07 +0200, Jiri Denemark wrote: Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Wed, Jun 01, 2022 at 14:50:07 +0200, Jiri Denemark wrote: Consider the following diff squashed in, otherwise the domain will be killed on destination in case a resume attempt fails early. Jirka diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 644c123702..0d60961f86 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -6599,7 +6599,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, qemuDomainJobPrivate *jobPriv = priv->job.privateData; virObjectEvent *event; bool inPostCopy = false; - bool doKill = true; + bool doKill = priv->job.phase != QEMU_MIGRATION_PHASE_FINISH_RESUME; int rc; VIR_DEBUG("vm=%p, flags=0x%lx, retcode=%d",

Normally the structure is created once the source reports completed migration, but with post-copy migration we can get here even after libvirt daemon was restarted. It doesn't make sense to preserve the structure in our status XML as we're going to rewrite almost all of it while refreshing the stats anyway. So we just create the structure here if it doesn't exist to make sure we can properly report statistics of a completed migration. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a078cbcd41..49985aba9a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3908,22 +3908,26 @@ qemuMigrationSrcComplete(virQEMUDriver *driver, virObjectEvent *event; int reason; - if (jobData) { - /* We need to refresh migration statistics after a completed post-copy - * migration since jobData contains obsolete data from the time we - * switched to post-copy mode. - */ - if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && - reason == VIR_DOMAIN_PAUSED_POSTCOPY) { - VIR_DEBUG("Refreshing migration statistics"); - if (qemuMigrationAnyFetchStats(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, - jobData, NULL) < 0) - VIR_WARN("Could not refresh migration statistics"); - } + if (!jobData) { + priv->job.completed = virDomainJobDataCopy(priv->job.current); + jobData = priv->job.completed; + jobData->status = VIR_DOMAIN_JOB_STATUS_COMPLETED; + } - qemuDomainJobDataUpdateTime(jobData); + /* We need to refresh migration statistics after a completed post-copy + * migration since jobData contains obsolete data from the time we + * switched to post-copy mode. + */ + if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && + reason == VIR_DOMAIN_PAUSED_POSTCOPY) { + VIR_DEBUG("Refreshing migration statistics"); + if (qemuMigrationAnyFetchStats(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, + jobData, NULL) < 0) + VIR_WARN("Could not refresh migration statistics"); } + qemuDomainJobDataUpdateTime(jobData); + /* If guest uses SPICE and supports seamless migration we have to hold * up domain shutdown until SPICE server transfers its data */ qemuMigrationSrcWaitForSpice(vm); -- 2.35.1

Every single call to qemuMigrationJobContinue needs to register a cleanup callback in case the migrating domain dies between phases or when migration is paused due to a failure in postcopy mode. Let's integrate registering the callback in qemuMigrationJobContinue to make sure the current thread does not release a migration job without setting a cleanup callback. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.c | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 57919b014d..79766b502e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -183,8 +183,10 @@ qemuMigrationJobStartPhase(virDomainObj *vm, static void ATTRIBUTE_NONNULL(1) -qemuMigrationJobContinue(virDomainObj *vm) +qemuMigrationJobContinue(virDomainObj *vm, + qemuDomainCleanupCallback cleanup) { + qemuDomainCleanupAdd(vm, cleanup); qemuDomainObjReleaseAsyncJob(vm); } @@ -2388,8 +2390,7 @@ qemuMigrationAnyConnectionClosed(virDomainObj *vm, qemuMigrationSrcPostcopyFailed(vm); else qemuMigrationDstPostcopyFailed(vm); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); } else { qemuMigrationParamsReset(driver, vm, priv->job.asyncJob, jobPriv->migParams, priv->job.apiFlags); @@ -2826,8 +2827,7 @@ qemuMigrationSrcBeginResumePhase(virConnectPtr conn, if (!xml) ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); return g_steal_pointer(&xml); } @@ -2902,8 +2902,6 @@ qemuMigrationSrcBegin(virConnectPtr conn, if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn, qemuMigrationAnyConnectionClosed) < 0) goto endjob; - - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); } ret = g_steal_pointer(&xml); @@ -2911,7 +2909,7 @@ qemuMigrationSrcBegin(virConnectPtr conn, endjob: if (flags & VIR_MIGRATE_CHANGE_PROTECTION) { if (ret) - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); else qemuMigrationJobFinish(vm); } else { @@ -3440,13 +3438,11 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, VIR_WARN("Unable to encode migration cookie"); } - qemuDomainCleanupAdd(vm, qemuMigrationDstPrepareCleanup); - /* We keep the job active across API calls until the finish() call. * This prevents any other APIs being invoked while incoming * migration is taking place. */ - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuMigrationDstPrepareCleanup); if (autoPort) priv->migrationPort = port; @@ -3561,8 +3557,7 @@ qemuMigrationDstPrepareResume(virQEMUDriver *driver, qemuProcessIncomingDefFree(incoming); if (ret < 0) ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); virDomainObjEndAPI(&vm); return ret; } @@ -4090,8 +4085,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, if (virDomainObjIsFailedPostcopy(vm)) { ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); } else { qemuMigrationJobFinish(vm); } @@ -5998,8 +5992,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, if (virDomainObjIsFailedPostcopy(vm)) { ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); } else { /* v2 proto has no confirm phase so we need to reset migration parameters * here @@ -6067,8 +6060,7 @@ qemuMigrationSrcPerformResume(virQEMUDriver *driver, if (ret < 0) ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); return ret; } @@ -6141,8 +6133,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, } else { if (ret < 0) ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); } if (!virDomainObjIsActive(vm)) @@ -6671,7 +6662,6 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, if (virDomainObjIsFailedPostcopy(vm)) { ignore_value(qemuMigrationJobSetPhase(vm, QEMU_MIGRATION_PHASE_POSTCOPY_FAILED)); qemuProcessAutoDestroyRemove(driver, vm); - qemuDomainCleanupAdd(vm, qemuProcessCleanupMigrationJob); *finishJob = false; } else { qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, @@ -6765,7 +6755,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver, if (finishJob) qemuMigrationJobFinish(vm); else - qemuMigrationJobContinue(vm); + qemuMigrationJobContinue(vm, qemuProcessCleanupMigrationJob); } cleanup: -- 2.35.1

Since all parts of post-copy recovery have been implemented now, it's time to enable the corresponding flag. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - no change src/qemu/qemu_migration.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 7eb0d4fe02..fbc0549b34 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -60,6 +60,7 @@ VIR_MIGRATE_TLS | \ VIR_MIGRATE_PARALLEL | \ VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES | \ + VIR_MIGRATE_POSTCOPY_RESUME | \ 0) /* All supported migration parameters and their types. */ -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - VIR_DEBUG added src/qemu/qemu_driver.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4edf5635c0..cab2859a6c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12783,7 +12783,9 @@ qemuDomainAbortJobMigration(virDomainObj *vm) } -static int qemuDomainAbortJob(virDomainPtr dom) +static int +qemuDomainAbortJobFlags(virDomainPtr dom, + unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm; @@ -12791,10 +12793,14 @@ static int qemuDomainAbortJob(virDomainPtr dom) qemuDomainObjPrivate *priv; int reason; + VIR_DEBUG("flags=0x%x", flags); + + virCheckFlags(0, -1); + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; - if (virDomainAbortJobEnsureACL(dom->conn, vm->def) < 0) + if (virDomainAbortJobFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (qemuDomainObjBeginJob(driver, vm, VIR_JOB_ABORT) < 0) @@ -12873,6 +12879,13 @@ static int qemuDomainAbortJob(virDomainPtr dom) } +static int +qemuDomainAbortJob(virDomainPtr dom) +{ + return qemuDomainAbortJobFlags(dom, 0); +} + + static int qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, unsigned long long downtime, @@ -21037,6 +21050,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetJobInfo = qemuDomainGetJobInfo, /* 0.7.7 */ .domainGetJobStats = qemuDomainGetJobStats, /* 1.0.3 */ .domainAbortJob = qemuDomainAbortJob, /* 0.7.7 */ + .domainAbortJobFlags = qemuDomainAbortJobFlags, /* 8.5.0 */ .domainMigrateGetMaxDowntime = qemuDomainMigrateGetMaxDowntime, /* 3.7.0 */ .domainMigrateSetMaxDowntime = qemuDomainMigrateSetMaxDowntime, /* 0.8.0 */ .domainMigrateGetCompressionCache = qemuDomainMigrateGetCompressionCache, /* 1.0.3 */ -- 2.35.1

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - dropped DeviceNotFound QMP error handling and replaced it with reporting an error if the VIR_DOMAIN_ABORT_JOB_POSTCOPY flag is used for something else than outgoing post-copy migration src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++--------- src/qemu/qemu_monitor.c | 9 +++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 19 ++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 5 files changed, 71 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cab2859a6c..01c4a8470a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12783,6 +12783,30 @@ qemuDomainAbortJobMigration(virDomainObj *vm) } +static int +qemuDomainAbortJobPostcopy(virDomainObj *vm, + unsigned int flags) +{ + qemuDomainObjPrivate *priv = vm->privateData; + int rc; + + if (!(flags & VIR_DOMAIN_ABORT_JOB_POSTCOPY)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot abort migration in post-copy mode")); + return -1; + } + + VIR_DEBUG("Suspending post-copy migration at client request"); + + qemuDomainObjAbortAsyncJob(vm); + qemuDomainObjEnterMonitor(priv->driver, vm); + rc = qemuMonitorMigratePause(priv->mon); + qemuDomainObjExitMonitor(vm); + + return rc; +} + + static int qemuDomainAbortJobFlags(virDomainPtr dom, unsigned int flags) @@ -12791,11 +12815,10 @@ qemuDomainAbortJobFlags(virDomainPtr dom, virDomainObj *vm; int ret = -1; qemuDomainObjPrivate *priv; - int reason; VIR_DEBUG("flags=0x%x", flags); - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_ABORT_JOB_POSTCOPY, -1); if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; @@ -12811,6 +12834,14 @@ qemuDomainAbortJobFlags(virDomainPtr dom, priv = vm->privateData; + if (flags & VIR_DOMAIN_ABORT_JOB_POSTCOPY && + (priv->job.asyncJob != VIR_ASYNC_JOB_MIGRATION_OUT || + !virDomainObjIsPostcopy(vm, VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT))) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("current job is not outgoing migration in post-copy mode")); + goto endjob; + } + switch (priv->job.asyncJob) { case VIR_ASYNC_JOB_NONE: virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -12830,15 +12861,10 @@ qemuDomainAbortJobFlags(virDomainPtr dom, break; case VIR_ASYNC_JOB_MIGRATION_OUT: - if ((priv->job.current->status == VIR_DOMAIN_JOB_STATUS_POSTCOPY || - (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && - reason == VIR_DOMAIN_PAUSED_POSTCOPY))) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot abort migration in post-copy mode")); - goto endjob; - } - - ret = qemuDomainAbortJobMigration(vm); + if (virDomainObjIsPostcopy(vm, VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT)) + ret = qemuDomainAbortJobPostcopy(vm, flags); + else + ret = qemuDomainAbortJobMigration(vm); break; case VIR_ASYNC_JOB_SAVE: diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index e9b9390c80..37bcbde31e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2425,6 +2425,15 @@ qemuMonitorMigrateCancel(qemuMonitor *mon) } +int +qemuMonitorMigratePause(qemuMonitor *mon) +{ + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONMigratePause(mon); +} + + int qemuMonitorQueryDump(qemuMonitor *mon, qemuMonitorDumpStats *stats) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 4f6c7e40fd..91f2d0941c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -890,6 +890,9 @@ int qemuMonitorMigrateToSocket(qemuMonitor *mon, int qemuMonitorMigrateCancel(qemuMonitor *mon); +int +qemuMonitorMigratePause(qemuMonitor *mon); + int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitor *mon, const char *capability); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 99c5e1b40f..8b81a07429 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3455,6 +3455,25 @@ int qemuMonitorJSONMigrateCancel(qemuMonitor *mon) } +int +qemuMonitorJSONMigratePause(qemuMonitor *mon) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("migrate-pause", NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + return -1; + + return 0; +} + + /* qemuMonitorJSONQueryDump: * @mon: Monitor pointer * @stats: Monitor dump stats diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 305faafce4..3b55e380b3 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -207,6 +207,9 @@ qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitor *mon, int qemuMonitorJSONMigrateCancel(qemuMonitor *mon); +int +qemuMonitorJSONMigratePause(qemuMonitor *mon); + int qemuMonitorJSONQueryDump(qemuMonitor *mon, qemuMonitorDumpStats *stats); -- 2.35.1

On Mon, Jun 06, 2022 at 14:24:05 +0200, Peter Krempa wrote: With the above, which was btw requested also in previous review: Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Notes: Version 2: - s/fecovering/recovering/ - added `` around flag name and virsh command NEWS.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 28ed46e361..94ab4a860e 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,11 @@ v8.5.0 (unreleased) * **New features** + * qemu: Add support for post-copy recovery + + A new ``VIR_MIGRATE_POSTCOPY_RESUME`` flag (``virsh migrate --postcopy-resume``) + was introduced for recovering from a failed post-copy migration. + * **Improvements** * **Bug fixes** -- 2.35.1

This is a special job for operations that need to modify domain state during an active migration. The modification must not affect any state that could conflict with the migration code. This is useful mainly for event handlers that need to be processed during migration and which could otherwise time out on acquiring a normal MODIFY job. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - new patch src/hypervisor/domain_job.c | 1 + src/hypervisor/domain_job.h | 4 ++++ src/qemu/qemu_migration.c | 1 + src/qemu/qemu_process.c | 7 +++++++ 4 files changed, 13 insertions(+) diff --git a/src/hypervisor/domain_job.c b/src/hypervisor/domain_job.c index 49867c3982..191dc1c700 100644 --- a/src/hypervisor/domain_job.c +++ b/src/hypervisor/domain_job.c @@ -18,6 +18,7 @@ VIR_ENUM_IMPL(virDomainJob, "modify", "abort", "migration operation", + "migration safe", "none", /* async job is never stored in job.active */ "async nested", ); diff --git a/src/hypervisor/domain_job.h b/src/hypervisor/domain_job.h index fce35ffbf5..24bb93c59f 100644 --- a/src/hypervisor/domain_job.h +++ b/src/hypervisor/domain_job.h @@ -31,6 +31,10 @@ typedef enum { VIR_JOB_MODIFY, /* May change state */ VIR_JOB_ABORT, /* Abort current async job */ VIR_JOB_MIGRATION_OP, /* Operation influencing outgoing migration */ + VIR_JOB_MIGRATION_SAFE, /* Internal only job for event handlers which need + to be processed even during migration. The code + may only change state in a way that does not + affect migration. */ /* The following two items must always be the last items before JOB_LAST */ VIR_JOB_ASYNC, /* Asynchronous job */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4bf942e6ab..0314fb1148 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -127,6 +127,7 @@ qemuMigrationJobStart(virQEMUDriver *driver, JOB_MASK(VIR_JOB_SUSPEND) | JOB_MASK(VIR_JOB_MIGRATION_OP); } + mask |= JOB_MASK(VIR_JOB_MIGRATION_SAFE); if (qemuDomainObjBeginAsyncJob(driver, vm, job, op, apiFlags) < 0) return -1; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8a98c03395..ad529dabb4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3472,6 +3472,7 @@ qemuProcessRestoreMigrationJob(virDomainObj *vm, op = VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT; allowedJobs = VIR_JOB_DEFAULT_MASK | JOB_MASK(VIR_JOB_MIGRATION_OP); } + allowedJobs |= JOB_MASK(VIR_JOB_MIGRATION_SAFE); qemuDomainObjRestoreAsyncJob(vm, job->asyncJob, job->phase, job->asyncStarted, op, @@ -3834,6 +3835,12 @@ qemuProcessRecoverJob(virQEMUDriver *driver, */ break; + case VIR_JOB_MIGRATION_SAFE: + /* event handlers, the reconnection code already handles them as we + * might as well just missed the event while we were not running + */ + break; + case VIR_JOB_MIGRATION_OP: case VIR_JOB_ABORT: case VIR_JOB_ASYNC: -- 2.35.1

When a domain has a guest agent channel enabled and the agent is running in the guest, we will get VSERPORT_CHANGE event on a destination host as soon as we start vCPUs there. This is not an issue for normal migration, but post-copy migration will remain running after we started vCPUs on the destination. If it runs for more than 30s, the VSERPORT_CHANGE event handler will fail to get a job and log the following error message: Timed out during operation: cannot acquire state change lock (held by monitor=remoteDispatchDomainMigrateFinish3Params) and of course we will think the guest agent is not connected and thus all APIs talking to it will fail. Until the agent or libvirt daemon is restarted. Luckily we only need to update the channel state (to mark it as connected) and connect to the agent neither of which conflicts with migration. Thus we can safely enable processing this event during migration. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - new patch 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 01c4a8470a..637106f1b3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3993,7 +3993,7 @@ processSerialChangedEvent(virQEMUDriver *driver, memset(&dev, 0, sizeof(dev)); } - if (qemuDomainObjBeginJob(driver, vm, VIR_JOB_MODIFY) < 0) + if (qemuDomainObjBeginJob(driver, vm, VIR_JOB_MIGRATION_SAFE) < 0) return; if (!virDomainObjIsActive(vm)) { -- 2.35.1

QEMU keeps guest CPUs running even in postcopy-paused migration state so that processes that already have all memory pages they need migrated to the destination can keep running. However, this behavior might bring unexpected delays in interprocess communication as some processes will be stopped until migration is recover and their memory pages migrated. So let's make sure all guest CPUs are paused while postcopy migration is paused. --- Notes: Version 2: - new patch - this patch does not currently work as QEMU cannot handle "stop" QMP command while in postcopy-paused state... the monitor just hangs (see https://gitlab.com/qemu-project/qemu/-/issues/1052 ) - an ideal solution of the QEMU bug would be if QEMU itself paused the CPUs for us and we just got notified about it via QMP events - but Peter Xu thinks this behavior is actually worse than keeping vCPUs running - so let's take this patch as a base for discussing what we should be doing with vCPUs in postcopy-paused migration state src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 30 +++++++++++++++++++++++++ src/qemu/qemu_migration.c | 47 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 6 +++++ src/qemu/qemu_process.c | 32 ++++++++++++++++++++++++++ 6 files changed, 117 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d04ec6cd0c..dcd6d5e1b5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11115,6 +11115,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event) break; case QEMU_PROCESS_EVENT_PR_DISCONNECT: case QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION: + case QEMU_PROCESS_EVENT_MIGRATION_CPU_STATE: case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 153dfe3a23..f5cdb2235f 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -427,6 +427,7 @@ typedef enum { QEMU_PROCESS_EVENT_GUEST_CRASHLOADED, QEMU_PROCESS_EVENT_MEMORY_DEVICE_SIZE_CHANGE, QEMU_PROCESS_EVENT_UNATTENDED_MIGRATION, + QEMU_PROCESS_EVENT_MIGRATION_CPU_STATE, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 637106f1b3..d0498ef2aa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4255,6 +4255,33 @@ processMemoryDeviceSizeChange(virQEMUDriver *driver, } +static void +processMigrationCPUState(virDomainObj *vm, + virDomainState state, + int reason) +{ + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; + + if (qemuDomainObjBeginJob(driver, vm, VIR_JOB_MIGRATION_SAFE) < 0) + return; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain '%s' is not running", vm->def->name); + goto endjob; + } + + if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_IN && + virDomainObjIsPostcopy(vm, VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN)) { + qemuMigrationUpdatePostcopyCPUState(vm, state, reason, + VIR_ASYNC_JOB_NONE); + } + + endjob: + qemuDomainObjEndJob(vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4312,6 +4339,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) processEvent->action, processEvent->status); break; + case QEMU_PROCESS_EVENT_MIGRATION_CPU_STATE: + processMigrationCPUState(vm, processEvent->action, processEvent->status); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0314fb1148..58d7009363 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -6831,6 +6831,53 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, } +void +qemuMigrationUpdatePostcopyCPUState(virDomainObj *vm, + virDomainState state, + int reason, + int asyncJob) +{ + virQEMUDriver *driver = QEMU_DOMAIN_PRIVATE(vm)->driver; + int current; + + if (state == VIR_DOMAIN_PAUSED) { + VIR_DEBUG("Post-copy migration of domain '%s' was paused, stopping guest CPUs", + vm->def->name); + } else { + VIR_DEBUG("Post-copy migration of domain '%s' was resumed, starting guest CPUs", + vm->def->name); + } + + if (virDomainObjGetState(vm, &current) == state) { + int eventType = -1; + int eventDetail = -1; + + if (current == reason) { + VIR_DEBUG("Guest CPUs are already in the right state"); + return; + } + + VIR_DEBUG("Fixing domain state reason"); + if (state == VIR_DOMAIN_PAUSED) { + eventType = VIR_DOMAIN_EVENT_SUSPENDED; + eventDetail = qemuDomainPausedReasonToSuspendedEvent(reason); + } else { + eventType = VIR_DOMAIN_EVENT_RESUMED; + eventDetail = qemuDomainRunningReasonToResumeEvent(reason); + } + virDomainObjSetState(vm, state, reason); + qemuDomainSaveStatus(vm); + virObjectEventStateQueue(driver->domainEventState, + virDomainEventLifecycleNewFromObj(vm, eventType, + eventDetail)); + } else if (state == VIR_DOMAIN_PAUSED) { + qemuProcessStopCPUs(driver, vm, reason, asyncJob); + } else { + qemuProcessStartCPUs(driver, vm, reason, asyncJob); + } +} + + /* Helper function called while vm is active. */ int qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index fbc0549b34..a1e2d8d171 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -224,6 +224,12 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, virDomainAsyncJob job, qemuMonitorMigrationStatus status); +void +qemuMigrationUpdatePostcopyCPUState(virDomainObj *vm, + virDomainState state, + int reason, + int asyncJob); + bool qemuMigrationSrcIsAllowed(virQEMUDriver *driver, virDomainObj *vm, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ad529dabb4..7fff68c0db 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1521,6 +1521,10 @@ qemuProcessHandleMigrationStatus(qemuMonitor *mon G_GNUC_UNUSED, * Thus we need to handle the event here. */ qemuMigrationSrcPostcopyFailed(vm); qemuDomainSaveStatus(vm); + } else if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) { + qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_MIGRATION_CPU_STATE, + VIR_DOMAIN_PAUSED, + VIR_DOMAIN_PAUSED_POSTCOPY_FAILED, NULL); } break; @@ -1547,6 +1551,12 @@ qemuProcessHandleMigrationStatus(qemuMonitor *mon G_GNUC_UNUSED, event = virDomainEventLifecycleNewFromObj(vm, eventType, eventDetail); qemuDomainSaveStatus(vm); } + + if (priv->job.asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) { + qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_MIGRATION_CPU_STATE, + VIR_DOMAIN_RUNNING, + VIR_DOMAIN_RUNNING_POSTCOPY, NULL); + } break; case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: @@ -3703,10 +3713,32 @@ qemuProcessRecoverMigration(virQEMUDriver *driver, if (migStatus == VIR_DOMAIN_JOB_STATUS_POSTCOPY) { VIR_DEBUG("Post-copy migration of domain %s still running, it will be handled as unattended", vm->def->name); + + if (job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN && + state == VIR_DOMAIN_PAUSED) { + qemuMigrationUpdatePostcopyCPUState(vm, VIR_DOMAIN_RUNNING, + VIR_DOMAIN_RUNNING_POSTCOPY, + VIR_ASYNC_JOB_NONE); + } else { + if (state == VIR_DOMAIN_RUNNING) + reason = VIR_DOMAIN_RUNNING_POSTCOPY; + else + reason = VIR_DOMAIN_PAUSED_POSTCOPY; + + virDomainObjSetState(vm, state, reason); + } + qemuProcessRestoreMigrationJob(vm, job); return 0; } + if (job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN && + migStatus == VIR_DOMAIN_JOB_STATUS_POSTCOPY_PAUSED) { + qemuMigrationUpdatePostcopyCPUState(vm, VIR_DOMAIN_PAUSED, + VIR_DOMAIN_PAUSED_POSTCOPY, + VIR_ASYNC_JOB_NONE); + } + if (migStatus != VIR_DOMAIN_JOB_STATUS_HYPERVISOR_COMPLETED) { if (job->asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT) qemuMigrationSrcPostcopyFailed(vm); -- 2.35.1