This is a follow-on to 528d37. It's fixing several other cases in
datatypes.c where we try to report an error while holding the conn's
lock, which can't work because the error reporting also tries to lock
the conn.

---
 src/datatypes.c |  133 ++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 82 insertions(+), 51 deletions(-)

diff --git a/src/datatypes.c b/src/datatypes.c
index ba5401d..c2030dd 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -270,11 +270,13 @@ virGetDomain(virConnectPtr conn, const char *name, const 
unsigned char *uuid) {
     /* TODO check the UUID */
     if (ret == NULL) {
         if (VIR_ALLOC(ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
         ret->name = strdup(name);
         if (ret->name == NULL) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
@@ -285,6 +287,7 @@ virGetDomain(virConnectPtr conn, const char *name, const 
unsigned char *uuid) {
             memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
 
         if (virHashAddEntry(conn->domains, name, ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                             _("failed to add domain to connection hash 
table"));
             goto error;
@@ -299,7 +302,6 @@ virGetDomain(virConnectPtr conn, const char *name, const 
unsigned char *uuid) {
     return(ret);
 
  error:
-    virMutexUnlock(&conn->lock);
     if (ret != NULL) {
         VIR_FREE(ret->name);
         VIR_FREE(ret);
@@ -325,24 +327,28 @@ virReleaseDomain(virDomainPtr domain) {
     DEBUG("release domain %p %s", domain, domain->name);
 
     /* TODO search by UUID first as they are better differenciators */
-    if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0)
+    if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0) {
+        virMutexUnlock(&conn->lock);
         virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                         _("domain missing from connection hash table"));
+        conn = NULL;
+    }
 
     domain->magic = -1;
     domain->id = -1;
     VIR_FREE(domain->name);
     VIR_FREE(domain);
 
-    DEBUG("unref connection %p %d", conn, conn->refs);
-    conn->refs--;
-    if (conn->refs == 0) {
-        virReleaseConnect(conn);
-        /* Already unlocked mutex */
-        return;
+    if (conn) {
+        DEBUG("unref connection %p %d", conn, conn->refs);
+        conn->refs--;
+        if (conn->refs == 0) {
+            virReleaseConnect(conn);
+            /* Already unlocked mutex */
+            return;
+        }
+        virMutexUnlock(&conn->lock);
     }
-
-    virMutexUnlock(&conn->lock);
 }
 
 
@@ -406,11 +412,13 @@ virGetNetwork(virConnectPtr conn, const char *name, const 
unsigned char *uuid) {
     /* TODO check the UUID */
     if (ret == NULL) {
         if (VIR_ALLOC(ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
         ret->name = strdup(name);
         if (ret->name == NULL) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
@@ -420,6 +428,7 @@ virGetNetwork(virConnectPtr conn, const char *name, const 
unsigned char *uuid) {
             memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
 
         if (virHashAddEntry(conn->networks, name, ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                             _("failed to add network to connection hash 
table"));
             goto error;
@@ -431,7 +440,6 @@ virGetNetwork(virConnectPtr conn, const char *name, const 
unsigned char *uuid) {
     return(ret);
 
  error:
-    virMutexUnlock(&conn->lock);
     if (ret != NULL) {
         VIR_FREE(ret->name);
         VIR_FREE(ret);
@@ -457,23 +465,27 @@ virReleaseNetwork(virNetworkPtr network) {
     DEBUG("release network %p %s", network, network->name);
 
     /* TODO search by UUID first as they are better differenciators */
-    if (virHashRemoveEntry(conn->networks, network->name, NULL) < 0)
+    if (virHashRemoveEntry(conn->networks, network->name, NULL) < 0) {
+        virMutexUnlock(&conn->lock);
         virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                         _("network missing from connection hash table"));
+        conn = NULL;
+    }
 
     network->magic = -1;
     VIR_FREE(network->name);
     VIR_FREE(network);
 
-    DEBUG("unref connection %p %d", conn, conn->refs);
-    conn->refs--;
-    if (conn->refs == 0) {
-        virReleaseConnect(conn);
-        /* Already unlocked mutex */
-        return;
+    if (conn) {
+        DEBUG("unref connection %p %d", conn, conn->refs);
+        conn->refs--;
+        if (conn->refs == 0) {
+            virReleaseConnect(conn);
+            /* Already unlocked mutex */
+            return;
+        }
+        virMutexUnlock(&conn->lock);
     }
-
-    virMutexUnlock(&conn->lock);
 }
 
 
@@ -714,11 +726,13 @@ virGetStoragePool(virConnectPtr conn, const char *name, 
const unsigned char *uui
     /* TODO check the UUID */
     if (ret == NULL) {
         if (VIR_ALLOC(ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
         ret->name = strdup(name);
         if (ret->name == NULL) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
@@ -728,6 +742,7 @@ virGetStoragePool(virConnectPtr conn, const char *name, 
const unsigned char *uui
             memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
 
         if (virHashAddEntry(conn->storagePools, name, ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                             _("failed to add storage pool to connection hash 
table"));
             goto error;
@@ -739,7 +754,6 @@ virGetStoragePool(virConnectPtr conn, const char *name, 
const unsigned char *uui
     return(ret);
 
 error:
-    virMutexUnlock(&conn->lock);
     if (ret != NULL) {
         VIR_FREE(ret->name);
         VIR_FREE(ret);
@@ -766,23 +780,27 @@ virReleaseStoragePool(virStoragePoolPtr pool) {
     DEBUG("release pool %p %s", pool, pool->name);
 
     /* TODO search by UUID first as they are better differenciators */
-    if (virHashRemoveEntry(conn->storagePools, pool->name, NULL) < 0)
+    if (virHashRemoveEntry(conn->storagePools, pool->name, NULL) < 0) {
+        virMutexUnlock(&conn->lock);
         virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                         _("pool missing from connection hash table"));
+        conn = NULL;
+    }
 
     pool->magic = -1;
     VIR_FREE(pool->name);
     VIR_FREE(pool);
 
-    DEBUG("unref connection %p %d", conn, conn->refs);
-    conn->refs--;
-    if (conn->refs == 0) {
-        virReleaseConnect(conn);
-        /* Already unlocked mutex */
-        return;
+    if (conn) {
+        DEBUG("unref connection %p %d", conn, conn->refs);
+        conn->refs--;
+        if (conn->refs == 0) {
+            virReleaseConnect(conn);
+            /* Already unlocked mutex */
+            return;
+        }
+        virMutexUnlock(&conn->lock);
     }
-
-    virMutexUnlock(&conn->lock);
 }
 
 
@@ -845,16 +863,19 @@ virGetStorageVol(virConnectPtr conn, const char *pool, 
const char *name, const c
     ret = (virStorageVolPtr) virHashLookup(conn->storageVols, key);
     if (ret == NULL) {
         if (VIR_ALLOC(ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
         ret->pool = strdup(pool);
         if (ret->pool == NULL) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
         ret->name = strdup(name);
         if (ret->name == NULL) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
@@ -864,6 +885,7 @@ virGetStorageVol(virConnectPtr conn, const char *pool, 
const char *name, const c
         ret->conn = conn;
 
         if (virHashAddEntry(conn->storageVols, key, ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                             _("failed to add storage vol to connection hash 
table"));
             goto error;
@@ -875,7 +897,6 @@ virGetStorageVol(virConnectPtr conn, const char *pool, 
const char *name, const c
     return(ret);
 
 error:
-    virMutexUnlock(&conn->lock);
     if (ret != NULL) {
         VIR_FREE(ret->name);
         VIR_FREE(ret->pool);
@@ -903,24 +924,28 @@ virReleaseStorageVol(virStorageVolPtr vol) {
     DEBUG("release vol %p %s", vol, vol->name);
 
     /* TODO search by UUID first as they are better differenciators */
-    if (virHashRemoveEntry(conn->storageVols, vol->key, NULL) < 0)
+    if (virHashRemoveEntry(conn->storageVols, vol->key, NULL) < 0) {
+        virMutexUnlock(&conn->lock);
         virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                         _("vol missing from connection hash table"));
+        conn = NULL;
+    }
 
     vol->magic = -1;
     VIR_FREE(vol->name);
     VIR_FREE(vol->pool);
     VIR_FREE(vol);
 
-    DEBUG("unref connection %p %d", conn, conn->refs);
-    conn->refs--;
-    if (conn->refs == 0) {
-        virReleaseConnect(conn);
-        /* Already unlocked mutex */
-        return;
+    if (conn) {
+        DEBUG("unref connection %p %d", conn, conn->refs);
+        conn->refs--;
+        if (conn->refs == 0) {
+            virReleaseConnect(conn);
+            /* Already unlocked mutex */
+            return;
+        }
+        virMutexUnlock(&conn->lock);
     }
-
-    virMutexUnlock(&conn->lock);
 }
 
 
@@ -981,7 +1006,8 @@ virGetNodeDevice(virConnectPtr conn, const char *name)
 
     ret = (virNodeDevicePtr) virHashLookup(conn->nodeDevices, name);
     if (ret == NULL) {
-       if (VIR_ALLOC(ret) < 0) {
+        if (VIR_ALLOC(ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
@@ -989,11 +1015,13 @@ virGetNodeDevice(virConnectPtr conn, const char *name)
         ret->conn = conn;
         ret->name = strdup(name);
         if (ret->name == NULL) {
+            virMutexUnlock(&conn->lock);
             virReportOOMError(conn);
             goto error;
         }
 
         if (virHashAddEntry(conn->nodeDevices, name, ret) < 0) {
+            virMutexUnlock(&conn->lock);
             virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                             _("failed to add node dev to conn hash table"));
             goto error;
@@ -1005,7 +1033,6 @@ virGetNodeDevice(virConnectPtr conn, const char *name)
     return(ret);
 
 error:
-    virMutexUnlock(&conn->lock);
     if (ret != NULL) {
         VIR_FREE(ret->name);
         VIR_FREE(ret);
@@ -1031,24 +1058,28 @@ virReleaseNodeDevice(virNodeDevicePtr dev) {
     virConnectPtr conn = dev->conn;
     DEBUG("release dev %p %s", dev, dev->name);
 
-    if (virHashRemoveEntry(conn->nodeDevices, dev->name, NULL) < 0)
+    if (virHashRemoveEntry(conn->nodeDevices, dev->name, NULL) < 0) {
+        virMutexUnlock(&conn->lock);
         virLibConnError(conn, VIR_ERR_INTERNAL_ERROR,
                         _("dev missing from connection hash table"));
+        conn = NULL;
+    }
 
     dev->magic = -1;
     VIR_FREE(dev->name);
     VIR_FREE(dev->parent);
     VIR_FREE(dev);
 
-    DEBUG("unref connection %p %d", conn, conn->refs);
-    conn->refs--;
-    if (conn->refs == 0) {
-        virReleaseConnect(conn);
-        /* Already unlocked mutex */
-        return;
+    if (conn) {
+        DEBUG("unref connection %p %d", conn, conn->refs);
+        conn->refs--;
+        if (conn->refs == 0) {
+            virReleaseConnect(conn);
+            /* Already unlocked mutex */
+            return;
+        }
+        virMutexUnlock(&conn->lock);
     }
-
-    virMutexUnlock(&conn->lock);
 }
 
 
-- 
1.6.0.6

--
Libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to