qemu: avoid rare race when undefining domain

When one domain is being undefined and at the same time started, for
example, there is a possibility of a rare problem occuring.

 - Thread 1 does virDomainUndefine(), has the lock, checks that the
   domain is active and because it's not, calls
   virDomainObjListRemove().

 - Thread 2 does virDomainCreate() and tries to lock the domain.

 - Thread 1 needs to lock domain list in order to remove the domain from
   it, but must unlock domain first (proper order is to lock domain list
   first and the domain itself second).

 - Thread 2 grabs the lock, starts the domain and releases the lock.

 - Thread 1 grabs the lock and removes the domain from list.

With this patch:

 - The undefining domain gets marked as "to undefine" before it is
    unlocked.

 - If domain is found in any of the search APIs, it's returned only if
   it is not marked as "to undefine".  The check is done while the
   domain is locked.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150505

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d4ac301d6d00855f6a87790c31da5bf4b17b26be..5cf0b1a33f1048525611eb4ed792affafc0a11e2 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1062,8 +1062,13 @@ virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
     virDomainObjPtr obj;
     virObjectLock(doms);
     obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
-    if (obj)
+    if (obj) {
         virObjectLock(obj);
+        if (obj->removing) {
+            virObjectUnlock(obj);
+            obj = NULL;
+        }
+    }
     virObjectUnlock(doms);
     return obj;
 }
@@ -1079,8 +1084,13 @@ virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
     virUUIDFormat(uuid, uuidstr);
 
     obj = virHashLookup(doms->objs, uuidstr);
-    if (obj)
+    if (obj) {
         virObjectLock(obj);
+        if (obj->removing) {
+            virObjectUnlock(obj);
+            obj = NULL;
+        }
+    }
     virObjectUnlock(doms);
     return obj;
 }
@@ -1105,8 +1115,13 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
     virDomainObjPtr obj;
     virObjectLock(doms);
     obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
-    if (obj)
+    if (obj) {
         virObjectLock(obj);
+        if (obj->removing) {
+            virObjectUnlock(obj);
+            obj = NULL;
+        }
+    }
     virObjectUnlock(doms);
     return obj;
 }
@@ -2551,6 +2566,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
 {
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
+    dom->removing = true;
     virUUIDFormat(dom->def->uuid, uuidstr);
     virObjectRef(dom);
     virObjectUnlock(dom);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e10b3c5d7fe5ef25d7b418436473fd8a2060c837..75e3d2a28355bbe54e77d48e720273b23a487805 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2187,6 +2187,7 @@ struct _virDomainObj {
     unsigned int autostart : 1;
     unsigned int persistent : 1;
     unsigned int updated : 1;
+    unsigned int removing : 1;
 
     virDomainDefPtr def; /* The current definition */
     virDomainDefPtr newDef; /* New definition to activate at shutdown */