Re: [libvirt] [PATCH] Fix peer2peer migration with transient VMs

The qemuMigrationConfirm method shouldn't deal with final VM cleanup, since it can be called from the peer2peer migration, which expects to still use the 'vm' object afterwards. Push the cleanup code out of qemuMigrationConfirm, into its caller, qemuDomainMigrateConfirm3 * src/qemu/qemu_driver.c: Add VM cleanup code to qemuDomainMigrateConfirm3 * src/qemu/qemu_migration.c, src/qemu/qemu_migration.h: Remove job handling cleanup from qemuMigrationConfirm --- src/qemu/qemu_driver.c | 25 +++++++++++++++++++++- src/qemu/qemu_migration.c | 51 +++++++++++++++------------------------------ src/qemu/qemu_migration.h | 3 +- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4fdf5e9..771e401 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6292,6 +6292,7 @@ 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 | @@ -6312,11 +6313,33 @@ qemuDomainMigrateConfirm3(virDomainPtr domain, goto cleanup; } + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + goto cleanup; + ret = qemuMigrationConfirm(driver, domain->conn, vm, cookiein, cookieinlen, - flags, cancelled, false); + flags, cancelled); + + orig_err = virSaveLastError(); + + if (qemuDomainObjEndJob(vm) == 0) { + vm = NULL; + } else if (!virDomainObjIsActive(vm) && + (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE))) { + if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) + virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm); + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } cleanup: + if (vm) + virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 40c40f3..88cab77 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1920,7 +1920,7 @@ finish: cookieoutlen = 0; ret = qemuMigrationConfirm(driver, sconn, vm, cookiein, cookieinlen, - flags, cancelled, true); + flags, cancelled); /* If Confirm3 returns -1, there's nothing more we can * do, but fortunately worst case is that there is a * domain left in 'paused' state on source. @@ -2086,12 +2086,6 @@ int qemuMigrationPerform(struct qemud_driver *driver, event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); - if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) { - virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm); - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, vm); - vm = NULL; - } } ret = 0; @@ -2113,9 +2107,17 @@ endjob: VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } - if (vm && - qemuDomainObjEndJob(vm) == 0) - vm = NULL; + if (vm) { + if (qemuDomainObjEndJob(vm) == 0) { + vm = NULL; + } else if (!virDomainObjIsActive(vm) && + (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE))) { + if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) + virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm); + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + } cleanup: if (vm) @@ -2306,9 +2308,8 @@ int qemuMigrationConfirm(struct qemud_driver *driver, virDomainObjPtr vm, const char *cookiein, int cookieinlen, - unsigned int flags, - int retcode, - bool skipJob) + unsigned int flags ATTRIBUTE_UNUSED, + int retcode) { qemuMigrationCookiePtr mig; virDomainEventPtr event = NULL; @@ -2317,14 +2318,10 @@ int qemuMigrationConfirm(struct qemud_driver *driver, if (!(mig = qemuMigrationEatCookie(vm, cookiein, cookieinlen, 0))) return -1; - if (!skipJob && - qemuDomainObjBeginJobWithDriver(driver, vm) < 0) - goto cleanup; - if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest unexpectedly quit")); - goto endjob; + goto cleanup; } /* Did the migration go as planned? If yes, kill off the @@ -2337,12 +2334,6 @@ int qemuMigrationConfirm(struct qemud_driver *driver, event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); - if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) { - virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm); - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, vm); - vm = NULL; - } } else { /* run 'cont' on the destination, which allows migration on qemu @@ -2354,7 +2345,7 @@ int qemuMigrationConfirm(struct qemud_driver *driver, if (virGetLastError() == NULL) qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); - goto endjob; + goto cleanup; } event = virDomainEventNewFromObj(vm, @@ -2362,22 +2353,14 @@ int qemuMigrationConfirm(struct qemud_driver *driver, VIR_DOMAIN_EVENT_RESUMED_MIGRATED); if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) { VIR_WARN("Failed to save status on vm %s", vm->def->name); - goto endjob; + goto cleanup; } } qemuMigrationCookieFree(mig); rv = 0; -endjob: - if (vm && - !skipJob && - qemuDomainObjEndJob(vm) == 0) - vm = NULL; - cleanup: - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); return rv; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index a350579..08e3acc 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -90,8 +90,7 @@ int qemuMigrationConfirm(struct qemud_driver *driver, const char *cookiein, int cookieinlen, unsigned int flags, - int retcode, - bool skipJob); + int retcode); int qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm,