Re: [libvirt PATCH 47/80] qemu: Make qemuMigrationCheckPhase failure fatal
On Tue, May 10, 2022 at 17:21:08 +0200, Jiri Denemark wrote:
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>
---
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));
This bit seems to belongs to the previous commit actually, but since
it's not used anywhere else ...
return -1;
}
[...]
@@ -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;
Any reason why you want to ignore this before the migration was
performed?
@@ -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
I could somehwat understand it here after the migration is done, but a
bug could be also in this code.
@@ -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)
Same here.