The check can reveal a serious bug in our migration code and we should not silently ignore it. Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx> --- 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 5b6073b963..1c5dd9b391 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); } @@ -2596,8 +2598,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; @@ -3148,7 +3151,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; @@ -3668,7 +3673,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, @@ -3753,7 +3759,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); @@ -4920,7 +4928,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, @@ -5164,7 +5172,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; @@ -5189,7 +5197,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 @@ -5565,7 +5573,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, @@ -5657,7 +5667,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); @@ -5671,7 +5683,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) @@ -6238,9 +6250,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); @@ -6314,7 +6327,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