Re: [Libvir] RFC: PATCH 4/5: make test driver thread safe

Friday, 4 January 2008

The current test driver keeps all its state in memory and is
not thread safe. Since background jobs will need to access
state from a thread periodically we need to make the test
driver thread safe. This is done with 2 levels of locking

 - A global driver lock.
 - A per-connection lock

Before taking the per-connection lock, the global driver lock
must be acquired. The driver lock can be released the moment
the connection lock is acquired. The connection lock must be
held for the duration of all driver methods which are accessing
the 'struct testConn' or 'struct testDom' or 'struct testNet'
data.

To keep this reasonably unobtrusive, the GET_CONNECT, and
GET_DOMAIN / GET_NETWORK macros have been altered to do the
neccessary lock acquisition. The RELEASE_DOMAIN and RELEASE_NETWORL
macros are new, and must be used prior to returning from any
driver method so that mutex are released.

This is a fairly coarse level of locking, but effective enough
for the test driver which is not performance critical - merely
safety critical.

This model should apply reasonably safely to the QEMU driver
too, though we may wish to make it finer grained.

 test.c |  245 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 214 insertions(+), 31 deletions(-)

Dan

diff -r d56f1d6ccb4a src/test.c

--- a/src/test.c	Fri Jan 04 17:48:15 2008 -0500
+++ b/src/test.c	Fri Jan 04 17:52:09 2008 -0500
@@ -37,12 +37,31 @@
 #include <unistd.h>
 #include <net/if.h>
 #include <netinet/in.h>
+#include <pthread.h>
 
 #include "internal.h"
 #include "test.h"
 #include "xml.h"
 #include "buf.h"
 #include "uuid.h"
+#include "job.h"
+
+
+/**
+ * Description of thread locking requirements:
+ *
+ * Locks on objects:
+ *
+ *  - Global driver lock
+ *  - Per connection lock
+ *
+ * If we wanted better granularity we could lock per-domain but
+ * that's overkill for now
+ *
+ * The driver lock must be held before acquiring a connection lock
+ *
+ * The GET / RELEASE macros should be used for this
+ */
 
 /* Flags that determine the action to take on a shutdown or crash of a domain
  */
@@ -106,6 +125,7 @@ typedef struct _testNet *testNetPtr;
 #define MAX_NETWORKS 20
 
 struct _testConn {
+    pthread_mutex_t lock;
     char path[PATH_MAX];
     int nextDomID;
     virNodeInfo nodeInfo;
@@ -117,6 +137,8 @@ typedef struct _testConn testConn;
 typedef struct _testConn testConn;
 typedef struct _testConn *testConnPtr;
 
+static pthread_mutex_t driverLock = PTHREAD_MUTEX_INITIALIZER;
+
 #define TEST_MODEL "i686"
 #define TEST_MODEL_WORDSIZE "32"
 
@@ -131,36 +153,62 @@ static const virNodeInfo defaultNodeInfo
     2,
 };
 
+/* Must call RELEASE_DOMAIN after this before returning */
 #define GET_DOMAIN(dom, ret)                                            \
     int domidx;                                                         \
     testConnPtr privconn;                                               \
     testDomPtr privdom;                                                 \
                                                                         \
+    pthread_mutex_lock(&driverLock);                                    \
     privconn = (testConnPtr)dom->conn->privateData;                     \
-    if ((domidx = getDomainIndex(dom)) < 0) {                           \
+    pthread_mutex_lock(&privconn->lock);                                \
+    pthread_mutex_unlock(&driverLock);                                  \
+    if ((domidx = getDomainIndex(privconn, dom)) < 0) {                 \
         testError((dom)->conn, (dom), NULL, VIR_ERR_INVALID_ARG,        \
                   __FUNCTION__);                                        \
+        pthread_mutex_unlock(&privconn->lock);                          \
         return (ret);                                                   \
     }                                                                   \
     privdom = &privconn->domains[domidx];
 
+/* Must call RELEASE_NETWORK after this before returning */
 #define GET_NETWORK(net, ret)                                           \
     int netidx;                                                         \
     testConnPtr privconn;                                               \
     testNetPtr privnet;                                                 \
                                                                         \
+    pthread_mutex_lock(&driverLock);                                    \
     privconn = (testConnPtr)net->conn->privateData;                     \
-    if ((netidx = getNetworkIndex(net)) < 0) {                          \
+    pthread_mutex_lock(&privconn->lock);                                \
+    pthread_mutex_unlock(&driverLock);                                  \
+    if ((netidx = getNetworkIndex(privconn, net)) < 0) {                \
         testError((net)->conn, NULL, (net), VIR_ERR_INVALID_ARG,        \
                   __FUNCTION__);                                        \
+        pthread_mutex_unlock(&privconn->lock);                          \
         return (ret);                                                   \
     }                                                                   \
     privnet = &privconn->networks[netidx];
 
+/* Must call RELEASE_CONNECTION after this before returning */
 #define GET_CONNECTION(conn, ret)                                       \
     testConnPtr privconn;                                               \
                                                                         \
-    privconn = (testConnPtr)conn->privateData;
+    pthread_mutex_lock(&driverLock);                                    \
+    privconn = (testConnPtr)conn->privateData;                          \
+    pthread_mutex_lock(&privconn->lock);                                \
+    pthread_mutex_unlock(&driverLock);
+
+/* To be called after GET_CONNECTIOn */
+#define RELEASE_CONNECTION()                    \
+    pthread_mutex_unlock(&privconn->lock)
+
+/* To be called after GET_NETWORK */
+#define RELEASE_NETWORK()                       \
+    pthread_mutex_unlock(&privconn->lock)
+
+/* To be called after GET_DOMAIN */
+#define RELEASE_DOMAIN()                        \
+    pthread_mutex_unlock(&privconn->lock)
 
 
 static void
@@ -230,6 +278,7 @@ static int testLoadDomain(virConnectPtr 
 
     if (gettimeofday(&tv, NULL) < 0) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("getting time of
day"));
+        RELEASE_CONNECTION();
         return (-1);
     }
 
@@ -327,7 +376,7 @@ static int testLoadDomain(virConnectPtr 
         }
     }
     if (handle < 0)
-        return (-1);
+        goto error;
 
     privconn->domains[handle].active = 1;
     privconn->domains[handle].id = domid;
@@ -351,11 +400,13 @@ static int testLoadDomain(virConnectPtr 
     privconn->domains[handle].onPoweroff = onPoweroff;
     privconn->domains[handle].onCrash = onCrash;
 
+    RELEASE_CONNECTION();
     return (handle);
 
  error:
     if (name)
         free(name);
+    RELEASE_CONNECTION();
     return (-1);
 }
 
@@ -488,7 +539,7 @@ static int testLoadNetwork(virConnectPtr
         }
     }
     if (handle < 0)
-        return (-1);
+        goto error;
 
     privconn->networks[handle].active = 1;
     privconn->networks[handle].running = 1;
@@ -522,6 +573,8 @@ static int testLoadNetwork(virConnectPtr
     strncpy(privconn->networks[handle].dhcpEnd, dhcpend,
sizeof(privconn->networks[handle].dhcpEnd)-1);
    
privconn->networks[handle].dhcpEnd[sizeof(privconn->networks[handle].dhcpEnd)-1] =
'\0';
     free(dhcpend);
+
+    RELEASE_CONNECTION();
     return (handle);
 
  error:
@@ -535,6 +588,7 @@ static int testLoadNetwork(virConnectPtr
         free(dhcpend);
     if (name)
         free(name);
+    RELEASE_CONNECTION();
     return (-1);
 }
 
@@ -669,7 +723,7 @@ static int testOpenFromFile(virConnectPt
     char *str;
     xmlDocPtr xml = NULL;
     xmlNodePtr root = NULL;
-    xmlNodePtr *domains, *networks = NULL;
+    xmlNodePtr *domains = NULL, *networks = NULL;
     xmlXPathContextPtr ctxt = NULL;
     virNodeInfoPtr nodeInfo;
     testConnPtr privconn = malloc(sizeof(testConn));
@@ -851,9 +905,8 @@ static int testOpenFromFile(virConnectPt
     return VIR_DRV_OPEN_ERROR;
 }
 
-static int getDomainIndex(virDomainPtr domain) {
+static int getDomainIndex(testConnPtr privconn, virDomainPtr domain) {
     int i;
-    GET_CONNECTION(domain->conn, -1);
 
     for (i = 0 ; i < MAX_DOMAINS ; i++) {
         if (domain->id >= 0) {
@@ -867,9 +920,8 @@ static int getDomainIndex(virDomainPtr d
     return (-1);
 }
 
-static int getNetworkIndex(virNetworkPtr network) {
+static int getNetworkIndex(testConnPtr privconn, virNetworkPtr network) {
     int i;
-    GET_CONNECTION(network->conn, -1);
 
     for (i = 0 ; i < MAX_NETWORKS ; i++) {
         if (STREQ(network->name, privconn->networks[i].name))
@@ -956,8 +1008,10 @@ static char * testGetURI (virConnectPtr 
 
     if (asprintf (&uri, "test://%s", privconn->path) == -1) {
         testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno));
+        RELEASE_CONNECTION();
         return NULL;
     }
+    RELEASE_CONNECTION();
     return uri;
 }
 
@@ -972,6 +1026,7 @@ static int testNodeGetInfo(virConnectPtr
 {
     GET_CONNECTION(conn, -1);
     memcpy(info, &privconn->nodeInfo, sizeof(virNodeInfo));
+    RELEASE_CONNECTION();
     return (0);
 }
 
@@ -1022,6 +1077,7 @@ static int testNumOfDomains(virConnectPt
             continue;
         numActive++;
     }
+    RELEASE_CONNECTION();
     return (numActive);
 }
 
@@ -1035,22 +1091,30 @@ testDomainCreateLinux(virConnectPtr conn
 
     if (xmlDesc == NULL) {
         testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+        RELEASE_CONNECTION();
         return (NULL);
     }
 
     if (privconn->numDomains == MAX_DOMAINS) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many
domains"));
+        RELEASE_CONNECTION();
         return (NULL);
     }
 
     domid = privconn->nextDomID++;
-    if ((handle = testLoadDomainFromDoc(conn, domid, xmlDesc)) < 0)
+    if ((handle = testLoadDomainFromDoc(conn, domid, xmlDesc)) < 0) {
+        RELEASE_CONNECTION();
         return (NULL);
+    }
     privconn->domains[handle].config = 0;
 
     dom = virGetDomain(conn, privconn->domains[handle].name,
privconn->domains[handle].uuid);
-    if (dom == NULL) return NULL;
+    if (dom == NULL) {
+        RELEASE_CONNECTION();
+        return NULL;
+    }
     privconn->numDomains++;
+    RELEASE_CONNECTION();
     return (dom);
 }
 
@@ -1072,12 +1136,17 @@ static virDomainPtr testLookupDomainByID
 
     if (idx < 0) {
         testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+        RELEASE_CONNECTION();
         return(NULL);
     }
 
     dom = virGetDomain(conn, privconn->domains[idx].name,
privconn->domains[idx].uuid);
-    if (dom == NULL) return NULL;
+    if (dom == NULL) {
+        RELEASE_CONNECTION();
+        return NULL;
+    }
     dom->id = id;
+    RELEASE_CONNECTION();
     return (dom);
 }
 
@@ -1098,13 +1167,18 @@ static virDomainPtr testLookupDomainByUU
 
     if (idx < 0) {
         testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+        RELEASE_CONNECTION();
         return NULL;
     }
 
     dom = virGetDomain(conn, privconn->domains[idx].name,
privconn->domains[idx].uuid);
-    if (dom == NULL) return NULL;
+    if (dom == NULL) {
+        RELEASE_CONNECTION();
+        return NULL;
+    }
     dom->id = privconn->domains[idx].id;
 
+    RELEASE_CONNECTION();
     return dom;
 }
 
@@ -1125,13 +1199,18 @@ static virDomainPtr testLookupDomainByNa
 
     if (idx < 0) {
         testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+        RELEASE_CONNECTION();
         return NULL;
     }
 
     dom = virGetDomain(conn, privconn->domains[idx].name,
privconn->domains[idx].uuid);
-    if (dom == NULL) return NULL;
+    if (dom == NULL) {
+        RELEASE_CONNECTION();
+        return NULL;
+    }
     dom->id = privconn->domains[idx].id;
 
+    RELEASE_CONNECTION();
     return dom;
 }
 
@@ -1148,6 +1227,7 @@ static int testListDomains (virConnectPt
             ids[n++] = privconn->domains[i].id;
         }
     }
+    RELEASE_CONNECTION();
     return (n);
 }
 
@@ -1162,6 +1242,7 @@ static int testDestroyDomain (virDomainP
     } else {
         privdom->active = 0;
     }
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1171,10 +1252,12 @@ static int testResumeDomain (virDomainPt
 
     if (privdom->info.state != VIR_DOMAIN_PAUSED) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
paused");
+        RELEASE_DOMAIN();
         return -1;
     }
 
     privdom->info.state = VIR_DOMAIN_RUNNING;
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1185,10 +1268,12 @@ static int testPauseDomain (virDomainPtr
     if (privdom->info.state == VIR_DOMAIN_SHUTOFF ||
         privdom->info.state == VIR_DOMAIN_PAUSED) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
running");
+        RELEASE_DOMAIN();
         return -1;
     }
 
     privdom->info.state = VIR_DOMAIN_PAUSED;
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1198,13 +1283,14 @@ static int testShutdownDomain (virDomain
 
     if (privdom->info.state == VIR_DOMAIN_SHUTOFF) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
running");
+        RELEASE_DOMAIN();
         return -1;
     }
 
     privdom->info.state = VIR_DOMAIN_SHUTOFF;
     domain->id = -1;
     privdom->id = -1;
-
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1245,6 +1331,7 @@ static int testRebootDomain (virDomainPt
         break;
     }
 
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1256,6 +1343,7 @@ static int testGetDomainInfo (virDomainP
 
     if (gettimeofday(&tv, NULL) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, _("getting
time of day"));
+        RELEASE_DOMAIN();
         return (-1);
     }
 
@@ -1266,10 +1354,12 @@ static int testGetDomainInfo (virDomainP
         privdom->info.cpuTime = ((tv.tv_sec * 1000ll * 1000ll  * 1000ll) + (tv.tv_usec
* 1000ll));
     }
     memcpy(info, &privdom->info, sizeof(virDomainInfo));
+
+    RELEASE_DOMAIN();
     return (0);
 }
 
-static char *testDomainDumpXML(virDomainPtr domain, int flags);
+static char *testDomainDumpXMLLocked(virDomainPtr domain, testDomPtr privdom, int
flags);
 
 #define TEST_SAVE_MAGIC "TestGuestMagic"
 
@@ -1280,11 +1370,12 @@ static int testDomainSave(virDomainPtr d
     int fd, len;
     GET_DOMAIN(domain, -1);
 
-    xml = testDomainDumpXML(domain, 0);
+    xml = testDomainDumpXMLLocked(domain, privdom, 0);
 
     if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot save domain");
+        RELEASE_DOMAIN();
         return (-1);
     }
     len = strlen(xml);
@@ -1292,24 +1383,28 @@ static int testDomainSave(virDomainPtr d
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write header");
         close(fd);
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (write(fd, (char*)&len, sizeof(len)) != sizeof(len)) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write metadata length");
         close(fd);
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (write(fd, xml, len) != len) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write metadata");
         close(fd);
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (close(fd) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot save domain data");
         close(fd);
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (privdom->config) {
@@ -1319,8 +1414,10 @@ static int testDomainSave(virDomainPtr d
     } else {
         privdom->active = 0;
     }
+    RELEASE_DOMAIN();
     return 0;
 }
+
 
 static int testDomainRestore(virConnectPtr conn,
                              const char *path)
@@ -1333,42 +1430,49 @@ static int testDomainRestore(virConnectP
     if ((fd = open(path, O_RDONLY)) < 0) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot read domain image");
+        RELEASE_CONNECTION();
         return (-1);
     }
     if (read(fd, magic, sizeof(magic)) != sizeof(magic)) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                   "incomplete save header");
         close(fd);
+        RELEASE_CONNECTION();
         return (-1);
     }
     if (memcmp(magic, TEST_SAVE_MAGIC, sizeof(magic))) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                   "mismatched header magic");
         close(fd);
+        RELEASE_CONNECTION();
         return (-1);
     }
     if (read(fd, (char*)&len, sizeof(len)) != sizeof(len)) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                   "failed to read metadata length");
         close(fd);
+        RELEASE_CONNECTION();
         return (-1);
     }
     if (len < 1 || len > 8192) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                   "length of metadata out of range");
         close(fd);
+        RELEASE_CONNECTION();
         return (-1);
     }
     xml = malloc(len+1);
     if (!xml) {
         testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "xml");
         close(fd);
+        RELEASE_CONNECTION();
         return (-1);
     }
     if (read(fd, xml, len) != len) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                   "incomplete metdata");
         close(fd);
+        RELEASE_CONNECTION();
         return (-1);
     }
     xml[len] = '\0';
@@ -1376,6 +1480,7 @@ static int testDomainRestore(virConnectP
     domid = privconn->nextDomID++;
     ret = testLoadDomainFromDoc(conn, domid, xml);
     free(xml);
+    RELEASE_CONNECTION();
     return ret < 0 ? -1 : 0;
 }
 
@@ -1389,18 +1494,21 @@ static int testDomainCoreDump(virDomainP
     if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot save domain core");
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (write(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) != sizeof(TEST_SAVE_MAGIC))
{
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot write header");
         close(fd);
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (close(fd) < 0) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   "cannot save domain data");
         close(fd);
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (privdom->config) {
@@ -1410,6 +1518,7 @@ static int testDomainCoreDump(virDomainP
     } else {
         privdom->active = 0;
     }
+    RELEASE_DOMAIN();
     return 0;
 }
 
@@ -1418,9 +1527,12 @@ static char *testGetOSType(virDomainPtr 
 }
 
 static unsigned long testGetMaxMemory(virDomainPtr domain) {
+    unsigned long ret;
     GET_DOMAIN(domain, -1);
 
-    return privdom->info.maxMem;
+    ret = privdom->info.maxMem;
+    RELEASE_DOMAIN();
+    return ret;
 }
 
 static int testSetMaxMemory(virDomainPtr domain,
@@ -1430,6 +1542,7 @@ static int testSetMaxMemory(virDomainPtr
 
     /* XXX validate not over host memory wrt to other domains */
     privdom->info.maxMem = memory;
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1440,10 +1553,12 @@ static int testSetMemory(virDomainPtr do
 
     if (memory > privdom->info.maxMem) {
         testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+        RELEASE_DOMAIN();
         return (-1);
     }
 
     privdom->info.memory = memory;
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1454,20 +1569,21 @@ static int testSetVcpus(virDomainPtr dom
     /* We allow more cpus in guest than host */
     if (nrCpus > 32) {
         testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+        RELEASE_DOMAIN();
         return (-1);
     }
 
     privdom->info.nrVirtCpu = nrCpus;
+    RELEASE_DOMAIN();
     return (0);
 }
 
-static char *testDomainDumpXML(virDomainPtr domain, int flags ATTRIBUTE_UNUSED)
+static char *testDomainDumpXMLLocked(virDomainPtr domain, testDomPtr privdom, int flags
ATTRIBUTE_UNUSED)
 {
     virBufferPtr buf;
     char *xml;
     unsigned char *uuid;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
-    GET_DOMAIN(domain, NULL);
 
     if (!(buf = virBufferNew(4000))) {
         testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__);
@@ -1489,8 +1605,17 @@ static char *testDomainDumpXML(virDomain
 
     xml = buf->content;
     free(buf);
-
     return (xml);
+}
+
+static char *testDomainDumpXML(virDomainPtr domain, int flags)
+{
+    char *xml;
+    GET_DOMAIN(domain, NULL);
+
+    xml = testDomainDumpXMLLocked(domain, privdom, flags);
+    RELEASE_DOMAIN();
+    return xml;
 }
 
 static int testNumOfDefinedDomains(virConnectPtr conn) {
@@ -1503,6 +1628,7 @@ static int testNumOfDefinedDomains(virCo
             continue;
         numInactive++;
     }
+    RELEASE_CONNECTION();
     return (numInactive);
 }
 
@@ -1518,6 +1644,7 @@ static int testListDefinedDomains(virCon
             names[n++] = strdup(privconn->domains[i].name);
         }
     }
+    RELEASE_CONNECTION();
     return (n);
 }
 
@@ -1525,12 +1652,14 @@ static virDomainPtr testDomainDefineXML(
                                         const char *doc) {
     int handle;
     xmlDocPtr xml;
+    virDomainPtr ret;
     GET_CONNECTION(conn, NULL);
 
     if (!(xml = xmlReadDoc(BAD_CAST doc, "domain.xml", NULL,
                            XML_PARSE_NOENT | XML_PARSE_NONET |
                            XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
         testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain"));
+        RELEASE_CONNECTION();
         return (NULL);
     }
 
@@ -1539,10 +1668,14 @@ static virDomainPtr testDomainDefineXML(
 
     xmlFreeDoc(xml);
 
-    if (handle < 0)
+    if (handle < 0) {
+        RELEASE_CONNECTION();
         return (NULL);
-
-    return virGetDomain(conn, privconn->domains[handle].name,
privconn->domains[handle].uuid);
+    }
+
+    ret = virGetDomain(conn, privconn->domains[handle].name,
privconn->domains[handle].uuid);
+    RELEASE_CONNECTION();
+    return ret;
 }
 
 static int testDomainCreate(virDomainPtr domain) {
@@ -1551,12 +1684,14 @@ static int testDomainCreate(virDomainPtr
     if (privdom->info.state != VIR_DOMAIN_SHUTOFF) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   _("Domain is already running"));
+        RELEASE_DOMAIN();
         return (-1);
     }
 
     domain->id = privdom->id = privconn->nextDomID++;
     privdom->info.state = VIR_DOMAIN_RUNNING;
 
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1566,11 +1701,13 @@ static int testDomainUndefine(virDomainP
     if (privdom->info.state != VIR_DOMAIN_SHUTOFF) {
         testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
                   _("Domain is still running"));
+        RELEASE_DOMAIN();
         return (-1);
     }
 
     privdom->active = 0;
 
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1579,6 +1716,7 @@ static int testDomainGetAutostart(virDom
 {
     GET_DOMAIN(domain, -1);
     *autostart = privdom->autostart;
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1588,6 +1726,7 @@ static int testDomainSetAutostart(virDom
 {
     GET_DOMAIN(domain, -1);
     privdom->autostart = autostart ? 1 : 0;
+    RELEASE_DOMAIN();
     return (0);
 }
 
@@ -1611,11 +1750,13 @@ static int testDomainGetSchedulerParams(
     GET_DOMAIN(domain, -1);
     if (*nparams != 1) {
         testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+        RELEASE_DOMAIN();
         return (-1);
     }
     strcpy(params[0].field, "weight");
     params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT;
     params[0].value.ui = privdom->weight;
+    RELEASE_DOMAIN();
     return 0;
 }
 
@@ -1627,17 +1768,21 @@ static int testDomainSetSchedulerParams(
     GET_DOMAIN(domain, -1);
     if (nparams != 1) {
         testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (STRNEQ(params[0].field, "weight")) {
         testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"field");
+        RELEASE_DOMAIN();
         return (-1);
     }
     if (params[0].type != VIR_DOMAIN_SCHED_FIELD_UINT) {
         testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, "type");
+        RELEASE_DOMAIN();
         return (-1);
     }
     privdom->weight = params[0].value.ui;
+    RELEASE_DOMAIN();
     return 0;
 }
 
@@ -1662,6 +1807,7 @@ static virNetworkPtr testLookupNetworkBy
                                            const unsigned char *uuid)
 {
     int i, idx = -1;
+    virNetworkPtr ret;
     GET_CONNECTION(conn, NULL);
 
     for (i = 0 ; i < MAX_NETWORKS ; i++) {
@@ -1677,13 +1823,16 @@ static virNetworkPtr testLookupNetworkBy
         return NULL;
     }
 
-    return virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+    ret = virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+    RELEASE_CONNECTION();
+    return ret;
 }
 
 static virNetworkPtr testLookupNetworkByName(virConnectPtr conn,
                                            const char *name)
 {
     int i, idx = -1;
+    virNetworkPtr ret;
     GET_CONNECTION(conn, NULL);
 
     for (i = 0 ; i < MAX_NETWORKS ; i++) {
@@ -1696,10 +1845,13 @@ static virNetworkPtr testLookupNetworkBy
 
     if (idx < 0) {
         testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL);
+        RELEASE_CONNECTION();
         return NULL;
     }
 
-    return virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+    ret = virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+    RELEASE_CONNECTION();
+    return ret;
 }
 
 
@@ -1713,6 +1865,7 @@ static int testNumNetworks(virConnectPtr
             continue;
         numInactive++;
     }
+    RELEASE_CONNECTION();
     return (numInactive);
 }
 
@@ -1726,6 +1879,7 @@ static int testListNetworks(virConnectPt
             names[n++] = strdup(privconn->networks[i].name);
         }
     }
+    RELEASE_CONNECTION();
     return (n);
 }
 
@@ -1739,6 +1893,7 @@ static int testNumDefinedNetworks(virCon
             continue;
         numInactive++;
     }
+    RELEASE_CONNECTION();
     return (numInactive);
 }
 
@@ -1752,6 +1907,7 @@ static int testListDefinedNetworks(virCo
             names[n++] = strdup(privconn->networks[i].name);
         }
     }
+    RELEASE_CONNECTION();
     return (n);
 }
 
@@ -1762,21 +1918,29 @@ static virNetworkPtr testNetworkCreate(v
 
     if (xml == NULL) {
         testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+        RELEASE_CONNECTION();
         return (NULL);
     }
 
     if (privconn->numNetworks == MAX_NETWORKS) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many
networks"));
+        RELEASE_CONNECTION();
         return (NULL);
     }
 
-    if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0)
+    if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) {
+        RELEASE_CONNECTION();
         return (NULL);
+    }
     privconn->networks[handle].config = 0;
 
     net = virGetNetwork(conn, privconn->networks[handle].name,
privconn->networks[handle].uuid);
-    if (net == NULL) return NULL;
+    if (net == NULL) {
+        RELEASE_CONNECTION();
+        return NULL;
+    }
     privconn->numNetworks++;
+    RELEASE_CONNECTION();
     return (net);
 }
 
@@ -1787,21 +1951,29 @@ static virNetworkPtr testNetworkDefine(v
 
     if (xml == NULL) {
         testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+        RELEASE_CONNECTION();
         return (NULL);
     }
 
     if (privconn->numNetworks == MAX_NETWORKS) {
         testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many
networks"));
+        RELEASE_CONNECTION();
         return (NULL);
     }
 
-    if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0)
+    if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) {
+        RELEASE_CONNECTION();
         return (NULL);
+    }
 
     net = virGetNetwork(conn, privconn->networks[handle].name,
privconn->networks[handle].uuid);
     privconn->networks[handle].config = 1;
-    if (net == NULL) return NULL;
+    if (net == NULL) {
+        RELEASE_CONNECTION();
+        return NULL;
+    }
     privconn->numNetworks++;
+    RELEASE_CONNECTION();
     return (net);
 }
 
@@ -1811,11 +1983,13 @@ static int testNetworkUndefine(virNetwor
     if (privnet->running) {
         testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
                   _("Network is still running"));
+        RELEASE_NETWORK();
         return (-1);
     }
 
     privnet->active = 0;
 
+    RELEASE_NETWORK();
     return (0);
 }
 
@@ -1825,11 +1999,13 @@ static int testNetworkStart(virNetworkPt
     if (privnet->running) {
         testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
                   _("Network is already running"));
+        RELEASE_NETWORK();
         return (-1);
     }
 
     privnet->running = 1;
 
+    RELEASE_NETWORK();
     return (0);
 }
 
@@ -1841,6 +2017,7 @@ static int testNetworkDestroy(virNetwork
     } else {
         privnet->active = 0;
     }
+    RELEASE_NETWORK();
     return (0);
 }
 
@@ -1853,6 +2030,7 @@ static char *testNetworkDumpXML(virNetwo
 
     if (!(buf = virBufferNew(4000))) {
         testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, __FUNCTION__);
+        RELEASE_NETWORK();
         return (NULL);
     }
 
@@ -1882,6 +2060,7 @@ static char *testNetworkDumpXML(virNetwo
     xml = buf->content;
     free(buf);
 
+    RELEASE_NETWORK();
     return (xml);
 }
 
@@ -1891,8 +2070,10 @@ static char *testNetworkGetBridgeName(vi
     bridge = strdup(privnet->bridge);
     if (!bridge) {
         testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY,
"network");
+        RELEASE_NETWORK();
         return NULL;
     }
+    RELEASE_NETWORK();
     return bridge;
 }
 
@@ -1900,6 +2081,7 @@ static int testNetworkGetAutostart(virNe
                                    int *autostart) {
     GET_NETWORK(network, -1);
     *autostart = privnet->autostart;
+    RELEASE_NETWORK();
     return (0);
 }
 
@@ -1907,6 +2089,7 @@ static int testNetworkSetAutostart(virNe
                                    int autostart) {
     GET_NETWORK(network, -1);
     privnet->autostart = autostart ? 1 : 0;
+    RELEASE_NETWORK();
     return (0);
 }
 

-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [Libvir] RFC: PATCH 4/5: make test driver thread safe