Fix preservation of errors across migration steps

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 a9d12c2444 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.
This commit is contained in:
Daniel P. Berrange 2011-05-23 12:48:36 -04:00
parent 03547eee92
commit 5e31df9335
3 changed files with 25 additions and 27 deletions

View File

@ -3842,6 +3842,12 @@ finish:
*/ */
cancelled = ret == 0 && ddomain == NULL ? 1 : 0; 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 * If cancelled, then src VM will be restarted, else
* it will be killed * it will be killed

View File

@ -6002,7 +6002,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
struct qemud_driver *driver = dconn->privateData; struct qemud_driver *driver = dconn->privateData;
virDomainObjPtr vm; virDomainObjPtr vm;
virDomainPtr dom = NULL; virDomainPtr dom = NULL;
virErrorPtr orig_err;
virCheckFlags(VIR_MIGRATE_LIVE | virCheckFlags(VIR_MIGRATE_LIVE |
VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_PEER2PEER |
@ -6013,9 +6012,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC, NULL); VIR_MIGRATE_NON_SHARED_INC, NULL);
/* Migration failed. Save the current error so nothing squashes it */
orig_err = virSaveLastError();
qemuDriverLock(driver); qemuDriverLock(driver);
vm = virDomainFindByName(&driver->domains, dname); vm = virDomainFindByName(&driver->domains, dname);
if (!vm) { if (!vm) {
@ -6033,10 +6029,6 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
flags, retcode, false); flags, retcode, false);
cleanup: cleanup:
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
qemuDriverUnlock(driver); qemuDriverUnlock(driver);
return dom; return dom;
} }
@ -6255,7 +6247,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
{ {
struct qemud_driver *driver = dconn->privateData; struct qemud_driver *driver = dconn->privateData;
virDomainObjPtr vm; virDomainObjPtr vm;
virErrorPtr orig_err;
int ret = -1; int ret = -1;
virCheckFlags(VIR_MIGRATE_LIVE | virCheckFlags(VIR_MIGRATE_LIVE |
@ -6267,9 +6258,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_DISK |
VIR_MIGRATE_NON_SHARED_INC, -1); VIR_MIGRATE_NON_SHARED_INC, -1);
/* Migration failed. Save the current error so nothing squashes it */
orig_err = virSaveLastError();
qemuDriverLock(driver); qemuDriverLock(driver);
vm = virDomainFindByName(&driver->domains, dname); vm = virDomainFindByName(&driver->domains, dname);
if (!vm) { if (!vm) {
@ -6286,10 +6274,6 @@ qemuDomainMigrateFinish3(virConnectPtr dconn,
ret = 0; ret = 0;
cleanup: cleanup:
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
qemuDriverUnlock(driver); qemuDriverUnlock(driver);
return ret; return ret;
} }
@ -6304,7 +6288,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
struct qemud_driver *driver = domain->conn->privateData; struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm; virDomainObjPtr vm;
int ret = -1; int ret = -1;
virErrorPtr orig_err;
virCheckFlags(VIR_MIGRATE_LIVE | virCheckFlags(VIR_MIGRATE_LIVE |
VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_PEER2PEER |
@ -6332,8 +6315,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
cookiein, cookieinlen, cookiein, cookieinlen,
flags, cancelled); flags, cancelled);
orig_err = virSaveLastError();
if (qemuDomainObjEndJob(vm) == 0) { if (qemuDomainObjEndJob(vm) == 0) {
vm = NULL; vm = NULL;
} else if (!virDomainObjIsActive(vm) && } else if (!virDomainObjIsActive(vm) &&
@ -6344,11 +6325,6 @@ qemuDomainMigrateConfirm3(virDomainPtr domain,
vm = NULL; vm = NULL;
} }
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
cleanup: cleanup:
if (vm) if (vm)
virDomainObjUnlock(vm); virDomainObjUnlock(vm);

View File

@ -1956,6 +1956,12 @@ finish:
*/ */
cancelled = ret == 0 && ddomain == NULL ? 1 : 0; 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 * If cancelled, then src VM will be restarted, else
* it will be killed * it will be killed
@ -2249,6 +2255,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
"cookieout=%p, cookieoutlen=%p, flags=%lu, retcode=%d", "cookieout=%p, cookieoutlen=%p, flags=%lu, retcode=%d",
driver, dconn, vm, NULLSTR(cookiein), cookieinlen, driver, dconn, vm, NULLSTR(cookiein), cookieinlen,
cookieout, cookieoutlen, flags, retcode); cookieout, cookieoutlen, flags, retcode);
virErrorPtr orig_err = NULL;
priv = vm->privateData; priv = vm->privateData;
if (priv->jobActive != QEMU_JOB_MIGRATION_IN) { if (priv->jobActive != QEMU_JOB_MIGRATION_IN) {
@ -2314,6 +2321,14 @@ qemuMigrationFinish(struct qemud_driver *driver,
*/ */
if (qemuProcessStartCPUs(driver, vm, dconn, if (qemuProcessStartCPUs(driver, vm, dconn,
VIR_DOMAIN_RUNNING_MIGRATED) < 0) { 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 * In v3 protocol, the source VM is still available to
* restart during confirm() step, so we kill it off * restart during confirm() step, so we kill it off
@ -2334,9 +2349,6 @@ qemuMigrationFinish(struct qemud_driver *driver,
vm = NULL; vm = NULL;
} }
} }
if (virGetLastError() == NULL)
qemuReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("resume operation failed"));
goto endjob; goto endjob;
} }
} }
@ -2384,6 +2396,10 @@ cleanup:
if (event) if (event)
qemuDomainEventQueue(driver, event); qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig); qemuMigrationCookieFree(mig);
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
return dom; return dom;
} }