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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list