ch_driver: Introduce and use virCHDomainRemoveInactive()

There are few places where a call to virDomainObjListRemove() is
guarded with !vm->persistent check. And there are some places
which are missing this check completely (leading us to losing a
domain). To prevent such mistakes introduce
virCHDomainRemoveInactive() which does the check for us. Also
replace all occurrences of virDomainObjListRemove() with the call
to the new function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>

3 files changed, 22 insertions, 8 deletions

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index 0b4dbbf142..25f581c1c3 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -136,6 +136,15 @@ virCHDomainObjEndJob(virDomainObj *obj)
     virCondSignal(&priv->job.cond);
 }
 
+void
+virCHDomainRemoveInactive(virCHDriver *driver,
+                          virDomainObj *vm)
+{
+    if (vm->persistent) {
+        virDomainObjListRemove(driver->domains, vm);
+    }
+}
+
 static void *
 virCHDomainObjPrivateAlloc(void *opaque)
 {
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index c1d3be212e..11a20a874a 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -88,6 +88,10 @@ virCHDomainObjBeginJob(virDomainObj *obj, enum virCHDomainJob job)
 void
 virCHDomainObjEndJob(virDomainObj *obj);
 
+void
+virCHDomainRemoveInactive(virCHDriver *driver,
+                          virDomainObj *vm);
+
 int
 virCHDomainRefreshThreadInfo(virDomainObj *vm);
 
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index cd156a222b..ef74a00bf7 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -237,7 +237,7 @@ chDomainCreateXML(virConnectPtr conn,
 
  cleanup:
     if (vm && !dom) {
-        virDomainObjListRemove(driver->domains, vm);
+        virCHDomainRemoveInactive(driver, vm);
     }
     virDomainObjEndAPI(&vm);
     return dom;
@@ -342,10 +342,9 @@ chDomainUndefineFlags(virDomainPtr dom,
         goto cleanup;
     }
 
-    if (virDomainObjIsActive(vm)) {
-        vm->persistent = 0;
-    } else {
-        virDomainObjListRemove(driver->domains, vm);
+    vm->persistent = 0;
+    if (!virDomainObjIsActive(vm)) {
+        virCHDomainRemoveInactive(driver, vm);
     }
 
     ret = 0;
@@ -608,12 +607,14 @@ chDomainDestroyFlags(virDomainPtr dom, unsigned int flags)
     if (virDomainObjCheckActive(vm) < 0)
         goto endjob;
 
-    ret = virCHProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
+    if (virCHProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED) < 0)
+        goto endjob;
+
+    virCHDomainRemoveInactive(driver, vm);
+    ret = 0;
 
  endjob:
     virCHDomainObjEndJob(vm);
-    if (!vm->persistent)
-        virDomainObjListRemove(driver->domains, vm);
 
  cleanup:
     virDomainObjEndAPI(&vm);