[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Always remove domain object if MigratePrepare fails



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]