When doing migration, if an error occurs in Perform, it must not
be overwritten during Finish/Confirm steps. If an error occurs
in Finish, it must not be overwritten in Confirm.
Previous commit a9d12c2444e43a0d3e5135eb15b4b62a7c011427 added
code to qemudDomainMigrateFinish2 to preserve the error. This
is not the right place, because it is not applicable in non-p2p
migration. The src/libvirt.c virDomainMigrateV2/3 methods need
code to preserve errors for non-p2p migration, while the
doPeer2PeerMigrate2 and doPeer2PeerMigrate3 methods contain
code to preverse errors for p2p migration.
Remove the bogus error preservation from qemudDomainMigrateFinish2
and qemudDomainMigrateFinish3.
Fix virDomainMigrateV3 and doPeer2PeerMigrate3 so that they
preserve any error hit during the Finish3 step, before invoking
Confirm3.
Finally if qemuMigrationFinish fails to resume the CPUs, it must
preserve the error before tearing down the VM, so that VM cleanup
doesn't overwrite it.
* src/libvirt.c: Preserve error before invoking Confirm3
* src/qemu/qemu_driver.c: Remove bogus error preservation
code in qemudDomainMigrateFinish2/qemudDomainMigrateFinish3
* src/qemu/qemu_migration.c: Preserve error before invoking Confirm3
and after resume fails in qemuMigrationFinish.
---
src/libvirt.c | 6 ++++++
src/qemu/qemu_driver.c | 24 ------------------------
src/qemu/qemu_migration.c | 23 ++++++++++++++++++++---
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index baff80b..e714468 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3842,6 +3842,12 @@ finish:
*/
cancelled = ret == 0 && ddomain == NULL ? 1 : 0;
+ /* If finish3 set an error, and we don't have an earlier
+ * one we need to preserve it in case confirm3 overwrites
+ */
+ if (!orig_err)
+ orig_err = virSaveLastError();
+
/*
* If cancelled, then src VM will be restarted, else
* it will be killed
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 43425e6..877f86f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5992,7 +5992,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
struct qemud_driver *driver = dconn->privateData;
virDomainObjPtr vm;
virDomainPtr dom = NULL;
- virErrorPtr orig_err;
virCheckFlags(VIR_MIGRATE_LIVE |
VIR_MIGRATE_PEER2PEER |
@@ -6003,9 +6002,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC, NULL);
- /* Migration failed. Save the current error so nothing squashes it */
- orig_err = virSaveLastError();
-
qemuDriverLock(driver);
vm = virDomainFindByName(&driver->domains, dname);
if (!vm) {
@@ -6023,10 +6019,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
flags, retcode, false);
cleanup:
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
qemuDriverUnlock(driver);
return dom;
}
@@ -6245,7 +6237,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
{
struct qemud_driver *driver = dconn->privateData;
virDomainObjPtr vm;
- virErrorPtr orig_err;
int ret = -1;
virCheckFlags(VIR_MIGRATE_LIVE |
@@ -6257,9 +6248,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC, -1);
- /* Migration failed. Save the current error so nothing squashes it */
- orig_err = virSaveLastError();
-
qemuDriverLock(driver);
vm = virDomainFindByName(&driver->domains, dname);
if (!vm) {
@@ -6276,10 +6264,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
ret = 0;
cleanup:
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
qemuDriverUnlock(driver);
return ret;
}
@@ -6294,7 +6278,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
- virErrorPtr orig_err;
virCheckFlags(VIR_MIGRATE_LIVE |
VIR_MIGRATE_PEER2PEER |
@@ -6322,8 +6305,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
cookiein, cookieinlen,
flags, cancelled);
- orig_err = virSaveLastError();
-
if (qemuDomainObjEndJob(vm) == 0) {
vm = NULL;
} else if (!virDomainObjIsActive(vm) &&
@@ -6334,11 +6315,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
vm = NULL;
}
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
-
cleanup:
if (vm)
virDomainObjUnlock(vm);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 280b8cb..a0365ac 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1955,6 +1955,12 @@ finish:
*/
cancelled = ret == 0 && ddomain == NULL ? 1 : 0;
+ /* If finish3 set an error, and we don't have an earlier
+ * one we need to preserve it in case confirm3 overwrites
+ */
+ if (!orig_err)
+ orig_err = virSaveLastError();
+
/*
* If cancelled, then src VM will be restarted, else
* it will be killed
@@ -2247,6 +2253,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
"cookieout=%p, cookieoutlen=%p, flags=%lu, retcode=%d",
driver, dconn, vm, NULLSTR(cookiein), cookieinlen,
cookieout, cookieoutlen, flags, retcode);
+ virErrorPtr orig_err = NULL;
priv = vm->privateData;
if (priv->jobActive != QEMU_JOB_MIGRATION_IN) {
@@ -2312,6 +2319,14 @@ qemuMigrationFinish(struct qemud_driver *driver,
*/
if (qemuProcessStartCPUs(driver, vm, dconn,
VIR_DOMAIN_RUNNING_MIGRATED) < 0) {
+ if (virGetLastError() == NULL)
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("resume operation failed"));
+ /* Need to save the current error, in case shutting
+ * down the process overwrites it
+ */
+ orig_err = virSaveLastError();
+
/*
* In v3 protocol, the source VM is still available to
* restart during confirm() step, so we kill it off
@@ -2332,9 +2347,6 @@ qemuMigrationFinish(struct qemud_driver *driver,
vm = NULL;
}
}
- if (virGetLastError() == NULL)
- qemuReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("resume operation failed"));
goto endjob;
}
}
@@ -2382,6 +2394,11 @@ cleanup:
if (event)
qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig);
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ orig_err = virGetLastError();
+ }
return dom;
}
--
1.7.4.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list