[libvirt] [PATCH] qemu: Always remove domain object if MigratePrepare fails
Eric Blake
eblake at redhat.com
Mon Sep 26 21:56:03 UTC 2011
On 09/26/2011 05:50 AM, Jiri Denemark wrote:
> If migration failed in Prepare phase after virDomainAssignDef and before
> a job is started, the domain object was not properly removed.
> ---
> src/qemu/qemu_migration.c | 11 ++++++-----
> 1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 0a5a13d..ea49093 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1178,8 +1178,12 @@ cleanup:
> virDomainDefFree(def);
> VIR_FORCE_CLOSE(dataFD[0]);
> VIR_FORCE_CLOSE(dataFD[1]);
> - if (vm)
> - virDomainObjUnlock(vm);
> + if (vm) {
> + if (vm->persistent)
> + virDomainObjUnlock(vm);
> + else
> + qemuDomainRemoveInactive(driver, vm);
I think this part is not quite right. This will also remove a transient
destination vm on the success path.
> + }
> if (event)
> qemuDomainEventQueue(driver, event);
> qemuMigrationCookieFree(mig);
> @@ -1188,9 +1192,6 @@ cleanup:
> endjob:
> if (qemuMigrationJobFinish(driver, vm) == 0) {
> vm = NULL;
> - } else if (!vm->persistent) {
> - qemuDomainRemoveInactive(driver, vm);
> - vm = NULL;
> }
I agree that moving this hunk out of endjob: and into cleanup: means
that you will properly clean up for more failure cases, but I think
you're missing a (ret < 0) check in cleanup: for this to be
bullet-proof. I'd also feel more comfortable if Dan could review this
and/or run the TCK migration suite on it.
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list