At 05/20/2011 06:04 PM, Daniel P. Berrange Write:
> 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,
I test migration with this patch, and it can finish successfully.
I read this patch, and it looks fine to me.
ACK.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list