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

Re: [libvirt] PATCH: 14/25: Xen thread safety



This patch makes the various Xen drivers threadsafe by adding a
mutex lock on the xenUnifiedPrivatePtr object. The XenD driver
does not really need much locking, since it usually just calls
out to XenD. Likewise the Xen driver just makes hypercalls. Locks
are needed for libxenstore access, and the xm/inotify drivers
since they have shared state

 src/proxy_internal.c  |  173 ++++++++++++++++-----------------
 src/xen_inotify.c     |   25 +++-
 src/xen_internal.c    |   29 +++--
 src/xen_unified.c     |  176 +++++++++++++++++++++-------------
 src/xen_unified.h     |   36 +++++--
 src/xend_internal.c   |   37 ++++++-
 src/xm_internal.c     |  256 +++++++++++++++++++++++++++++++++-----------------
 src/xs_internal.c     |  130 +++++++++++++++++++------
 src/xs_internal.h     |    7 -
 tests/sexpr2xmltest.c |   19 +++
 10 files changed, 575 insertions(+), 313 deletions(-)

Daniel

diff --git a/src/proxy_internal.c b/src/proxy_internal.c
--- a/src/proxy_internal.c
+++ b/src/proxy_internal.c
@@ -33,7 +33,7 @@
 
 #define STANDALONE
 
-static int debug = 0;
+#define VIR_FROM_THIS VIR_FROM_PROXY
 
 static int xenProxyClose(virConnectPtr conn);
 static virDrvOpenStatus xenProxyOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags);
@@ -149,19 +149,18 @@ virProxyForkServer(void)
     const char *proxyarg[2];
 
     if (!proxyPath) {
-        fprintf(stderr, "failed to find libvirt_proxy\n");
+        VIR_WARN0("failed to find libvirt_proxy\n");
         return(-1);
     }
 
-    if (debug)
-        fprintf(stderr, "Asking to launch %s\n", proxyPath);
+    VIR_DEBUG("Asking to launch %s\n", proxyPath);
 
     proxyarg[0] = proxyPath;
     proxyarg[1] = NULL;
 
     if (virExec(NULL, proxyarg, NULL, NULL,
                 &pid, -1, NULL, NULL, VIR_EXEC_DAEMON) < 0)
-        fprintf(stderr, "Failed to fork libvirt_proxy\n");
+        VIR_ERROR0("Failed to fork libvirt_proxy\n");
 
     /*
      * do a waitpid on the intermediate process to avoid zombies.
@@ -226,32 +225,33 @@ retry:
         return (-1);
     }
 
-    if (debug > 0)
-        fprintf(stderr, "connected to unix socket %s via %d\n", path, fd);
+    DEBUG("connected to unix socket %s via %d\n", path, fd);
 
     return (fd);
 }
 
 /**
- * virProxyCloseClientSocket:
- * @fd: the file descriptor for the socket
+ * virProxyCloseSocket:
+ * @priv: the Xen proxy data
  *
- * Close the socket from that client
+ * Close the socket from that client. The caller must
+ * hold the lock on 'priv' before calling
  *
  * Returns 0 in case of success and -1 in case of error
  */
 static int
-virProxyCloseClientSocket(int fd) {
+virProxyCloseSocket(xenUnifiedPrivatePtr priv) {
     int ret;
 
-    if (fd < 0)
+    if (priv->proxy < 0)
         return(-1);
 
-    ret = close(fd);
+    ret = close(priv->proxy);
     if (ret != 0)
-        fprintf(stderr, _("Failed to close socket %d\n"), fd);
-    else if (debug > 0)
-        fprintf(stderr, "Closed socket %d\n", fd);
+        VIR_WARN(_("Failed to close socket %d\n"), priv->proxy);
+    else
+        VIR_DEBUG("Closed socket %d\n", priv->proxy);
+    priv->proxy = -1;
     return(ret);
 }
 
@@ -260,14 +260,13 @@ virProxyCloseClientSocket(int fd) {
  * @fd: the socket
  * @buffer: the target memory area
  * @len: the length in bytes
- * @quiet: quiet access
  *
  * Process a read from a client socket
  *
  * Returns the number of byte read or -1 in case of error.
  */
 static int
-virProxyReadClientSocket(int fd, char *buffer, int len, int quiet) {
+virProxyReadClientSocket(int fd, char *buffer, int len) {
     int ret;
 
     if ((fd < 0) || (buffer == NULL) || (len < 0))
@@ -277,18 +276,15 @@ retry:
     ret = read(fd, buffer, len);
     if (ret < 0) {
         if (errno == EINTR) {
-            if (debug > 0)
-                fprintf(stderr, "read socket %d interrupted\n", fd);
+            VIR_DEBUG("read socket %d interrupted\n", fd);
             goto retry;
         }
-        if (!quiet)
-            fprintf(stderr, _("Failed to read socket %d\n"), fd);
+        VIR_WARN("Failed to read socket %d\n", fd);
         return(-1);
     }
 
-    if (debug)
-        fprintf(stderr, "read %d bytes from socket %d\n",
-                ret, fd);
+    VIR_DEBUG("read %d bytes from socket %d\n",
+              ret, fd);
     return(ret);
 }
 
@@ -309,12 +305,11 @@ virProxyWriteClientSocket(int fd, const 
 
     ret = safewrite(fd, data, len);
     if (ret < 0) {
-        fprintf(stderr, _("Failed to write to socket %d\n"), fd);
+        VIR_WARN(_("Failed to write to socket %d\n"), fd);
         return(-1);
     }
-    if (debug)
-        fprintf(stderr, "wrote %d bytes to socket %d\n",
-                len, fd);
+    VIR_DEBUG("wrote %d bytes to socket %d\n",
+              len, fd);
 
     return(0);
 }
@@ -347,12 +342,9 @@ xenProxyClose(virConnectPtr conn)
         return -1;
     }
 
-    /* Fail silently. */
-    if (priv->proxy == -1)
-        return -1;
-
-    virProxyCloseClientSocket (priv->proxy);
-    priv->proxy = -1;
+    xenUnifiedLock(priv);
+    virProxyCloseSocket (priv);
+    xenUnifiedUnlock(priv);
 
     return 0;
 }
@@ -376,9 +368,11 @@ xenProxyCommand(virConnectPtr conn, virP
         return -1;
     }
 
+    xenUnifiedLock(priv);
+
     /* Fail silently. */
     if (priv->proxy == -1)
-        return -1;
+        goto error;
 
     /*
      * normal communication serial numbers are in 0..4095
@@ -390,62 +384,69 @@ xenProxyCommand(virConnectPtr conn, virP
     request->serial = serial;
     ret  = virProxyWriteClientSocket(priv->proxy, (const char *) request,
                                      request->len);
-    if (ret < 0)
-        return(-1);
+    if (ret < 0) {
+        if (!quiet)
+            virReportSystemError(conn, errno, "%s",
+                                 _("failed to write proxy request"));
+        goto error;
+    }
 retry:
     if (answer == NULL) {
         /* read in situ */
         ret  = virProxyReadClientSocket(priv->proxy, (char *) request,
-                                        sizeof(virProxyPacket), quiet);
-        if (ret < 0)
-            return(-1);
+                                        sizeof(virProxyPacket));
+        if (ret < 0) {
+            if (!quiet)
+                virReportSystemError(conn, errno, "%s",
+                                     _("failed to read proxy reply"));
+            goto error;
+        }
         if (ret != sizeof(virProxyPacket)) {
-            fprintf(stderr,
-                    _("Communication error with proxy: got %d bytes of %d\n"),
-                    ret, (int) sizeof(virProxyPacket));
-            xenProxyClose(conn);
-            return(-1);
+            virProxyError(conn, VIR_ERR_INTERNAL_ERROR,
+                          _("Communication error with proxy: got %d bytes of %d\n"),
+                          ret, (int) sizeof(virProxyPacket));
+            goto error;
         }
         res = request;
         if (res->len != sizeof(virProxyPacket)) {
-            fprintf(stderr,
-                    _("Communication error with proxy: expected %d bytes got %d\n"),
-                    (int) sizeof(virProxyPacket), res->len);
-            xenProxyClose(conn);
-            return(-1);
+            virProxyError(conn, VIR_ERR_INTERNAL_ERROR,
+                          _("Communication error with proxy: expected %d bytes got %d\n"),
+                          (int) sizeof(virProxyPacket), res->len);
+            goto error;
         }
     } else {
         /* read in packet provided */
         ret  = virProxyReadClientSocket(priv->proxy, (char *) answer,
-                                        sizeof(virProxyPacket), quiet);
-        if (ret < 0)
-            return(-1);
+                                        sizeof(virProxyPacket));
+        if (ret < 0) {
+            if (!quiet)
+                virReportSystemError(conn, errno, "%s",
+                                     _("failed to read proxy reply"));
+            goto error;
+        }
         if (ret != sizeof(virProxyPacket)) {
-            fprintf(stderr,
-                    _("Communication error with proxy: got %d bytes of %d\n"),
-                    ret, (int) sizeof(virProxyPacket));
-            xenProxyClose(conn);
-            return(-1);
+            virProxyError(conn, VIR_ERR_INTERNAL_ERROR,
+                          _("Communication error with proxy: got %d bytes of %d\n"),
+                          ret, (int) sizeof(virProxyPacket));
+            goto error;
         }
         res = (virProxyPacketPtr) answer;
         if ((res->len < sizeof(virProxyPacket)) ||
             (res->len > sizeof(virProxyFullPacket))) {
-            fprintf(stderr,
-                    _("Communication error with proxy: got %d bytes packet\n"),
-                    res->len);
-            xenProxyClose(conn);
-            return(-1);
+            virProxyError(conn, VIR_ERR_INTERNAL_ERROR,
+                          _("Communication error with proxy: got %d bytes packet\n"),
+                          res->len);
+            goto error;
         }
         if (res->len > sizeof(virProxyPacket)) {
             ret  = virProxyReadClientSocket(priv->proxy,
                                    (char *) &(answer->extra.arg[0]),
-                                            res->len - ret, quiet);
+                                            res->len - ret);
             if (ret != (int) (res->len - sizeof(virProxyPacket))) {
-                fprintf(stderr,
-                        _("Communication error with proxy: got %d bytes of %d\n"),
-                        ret, (int) sizeof(virProxyPacket));
-                xenProxyClose(conn);
-                return(-1);
+                virProxyError(conn, VIR_ERR_INTERNAL_ERROR,
+                              _("Communication error with proxy: got %d bytes of %d\n"),
+                              ret, (int) sizeof(virProxyPacket));
+                goto error;
             }
         }
     }
@@ -454,17 +455,22 @@ retry:
      */
     if ((res == NULL) || (res->version != PROXY_PROTO_VERSION) ||
         (res->len < sizeof(virProxyPacket))) {
-        fprintf(stderr, "%s",
-                _("Communication error with proxy: malformed packet\n"));
-        xenProxyClose(conn);
-        return(-1);
+        virProxyError(conn, VIR_ERR_INTERNAL_ERROR,
+                      _("Communication error with proxy: malformed packet\n"));
+        goto error;
     }
     if (res->serial != serial) {
-        TODO /* Asynchronous communication */
-        fprintf(stderr, _("got asynchronous packet number %d\n"), res->serial);
+        VIR_WARN(_("got asynchronous packet number %d\n"), res->serial);
         goto retry;
     }
-    return(0);
+
+    xenUnifiedUnlock(priv);
+    return 0;
+
+error:
+    virProxyCloseSocket(priv);
+    xenUnifiedUnlock(priv);
+    return -1;
 }
 
 /**
@@ -507,7 +513,6 @@ xenProxyOpen(virConnectPtr conn,
     ret = xenProxyCommand(conn, &req, NULL, 1);
     if ((ret < 0) || (req.command != VIR_PROXY_NONE)) {
             virProxyError(NULL, VIR_ERR_OPERATION_FAILED, __FUNCTION__);
-        xenProxyClose(conn);
         return(-1);
     }
     return(0);
@@ -549,7 +554,6 @@ xenProxyGetVersion(virConnectPtr conn, u
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, NULL, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(-1);
     }
     *hvVer = req.data.larg;
@@ -587,7 +591,6 @@ xenProxyListDomains(virConnectPtr conn, 
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, &ans, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(-1);
     }
     nb = ans.data.arg;
@@ -627,7 +630,6 @@ xenProxyNumOfDomains(virConnectPtr conn)
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, NULL, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(-1);
     }
     return(req.data.arg);
@@ -659,7 +661,6 @@ xenProxyDomainGetDomMaxMemory(virConnect
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, NULL, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(0);
     }
     return(req.data.larg);
@@ -724,7 +725,6 @@ xenProxyDomainGetInfo(virDomainPtr domai
     req.len = sizeof(req);
     ret = xenProxyCommand(domain->conn, &req, &ans, 0);
     if (ret < 0) {
-        xenProxyClose(domain->conn);
         return(-1);
     }
     if (ans.len != sizeof(virProxyPacket) + sizeof(virDomainInfo)) {
@@ -769,7 +769,6 @@ xenProxyLookupByID(virConnectPtr conn, i
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, &ans, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(NULL);
     }
     if (ans.data.arg == -1) {
@@ -814,7 +813,6 @@ xenProxyLookupByUUID(virConnectPtr conn,
 
     ret = xenProxyCommand(conn, (virProxyPacketPtr) &req, &req, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(NULL);
     }
     if (req.data.arg == -1) {
@@ -861,7 +859,6 @@ xenProxyLookupByName(virConnectPtr conn,
     strcpy(&req.extra.str[0], name);
     ret = xenProxyCommand(conn, (virProxyPacketPtr) &req, &req, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(NULL);
     }
     if (req.data.arg == -1) {
@@ -901,7 +898,6 @@ xenProxyNodeGetInfo(virConnectPtr conn, 
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, &ans, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return(-1);
     }
     if (ans.data.arg == -1) {
@@ -941,7 +937,6 @@ xenProxyGetCapabilities (virConnectPtr c
     req.len = sizeof(req);
     ret = xenProxyCommand(conn, &req, &ans, 0);
     if (ret < 0) {
-        xenProxyClose(conn);
         return NULL;
     }
     if (ans.data.arg == -1)
@@ -995,7 +990,6 @@ xenProxyDomainDumpXML(virDomainPtr domai
     req.len = sizeof(req);
     ret = xenProxyCommand(domain->conn, &req, &ans, 0);
     if (ret < 0) {
-        xenProxyClose(domain->conn);
         return(NULL);
     }
     if (ans.len <= sizeof(virProxyPacket)) {
@@ -1044,7 +1038,6 @@ xenProxyDomainGetOSType(virDomainPtr dom
     req.len = sizeof(req);
     ret = xenProxyCommand(domain->conn, &req, &ans, 0);
     if (ret < 0) {
-        xenProxyClose(domain->conn);
         return(NULL);
     }
     if ((ans.len == sizeof(virProxyPacket)) && (ans.data.arg < 0)) {
diff --git a/src/xen_inotify.c b/src/xen_inotify.c
--- a/src/xen_inotify.c
+++ b/src/xen_inotify.c
@@ -92,11 +92,13 @@ struct xenUnifiedDriver xenInotifyDriver
 };
 
 static int
-xenInotifyXenCacheLookup(const char *filename,
+xenInotifyXenCacheLookup(virConnectPtr conn,
+                         const char *filename,
                          char **name, unsigned char *uuid) {
+    xenUnifiedPrivatePtr priv = conn->privateData;
     xenXMConfCachePtr entry;
 
-    if (!(entry = virHashLookup(xenXMGetConfigCache(), filename))) {
+    if (!(entry = virHashLookup(priv->configCache, filename))) {
         DEBUG("No config found for %s", filename);
         return -1;
     }
@@ -172,7 +174,7 @@ xenInotifyDomainLookup(virConnectPtr con
                        char **name, unsigned char *uuid) {
     xenUnifiedPrivatePtr priv = conn->privateData;
     if (priv->useXenConfigCache)
-        return xenInotifyXenCacheLookup(filename, name, uuid);
+        return xenInotifyXenCacheLookup(conn, filename, name, uuid);
     else
         return xenInotifyXendDomainsDirLookup(conn, filename, name, uuid);
 }
@@ -296,25 +298,27 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUS
         return;
     }
 
+    xenUnifiedLock(priv);
+
 reread:
     got = read(fd, buf, sizeof(buf));
     if (got == -1) {
         if (errno == EINTR)
             goto reread;
-        return;
+        goto cleanup;
     }
 
     tmp = buf;
     while (got) {
         if (got < sizeof(struct inotify_event))
-            return; /* bad */
+            goto cleanup; /* bad */
 
         e = (struct inotify_event *)tmp;
         tmp += sizeof(struct inotify_event);
         got -= sizeof(struct inotify_event);
 
         if (got < e->len)
-            return;
+            goto cleanup;
 
         tmp += e->len;
         got -= e->len;
@@ -338,14 +342,14 @@ reread:
             if (xenInotifyRemoveDomainConfigInfo(conn, fname) < 0 ) {
                 virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
                                    "%s", _("Error adding file to config cache"));
-                return;
+                goto cleanup;
             }
         } else if (e->mask & ( IN_CREATE | IN_CLOSE_WRITE | IN_MOVED_TO) ) {
             virDomainEventPtr event;
             if (xenInotifyAddDomainConfigInfo(conn, fname) < 0 ) {
                 virXenInotifyError(NULL, VIR_ERR_INTERNAL_ERROR,
                                    "%s", _("Error adding file to config cache"));
-                return;
+                goto cleanup;
             }
 
             event = xenInotifyDomainEventFromFile(conn, fname,
@@ -361,6 +365,9 @@ reread:
         }
 
     }
+
+cleanup:
+    xenUnifiedUnlock(priv);
 }
 
 /**
@@ -456,7 +463,7 @@ xenInotifyOpen(virConnectPtr conn ATTRIB
         DEBUG0("Failed to add inotify handle, disabling events");
     }
 
-    conn->refs++;
+    virConnectRef(conn);
     return 0;
 }
 
diff --git a/src/xen_internal.c b/src/xen_internal.c
--- a/src/xen_internal.c
+++ b/src/xen_internal.c
@@ -1330,9 +1330,14 @@ xenHypervisorDomainBlockStats (virDomain
 {
 #ifdef __linux__
     xenUnifiedPrivatePtr priv;
+    int ret;
 
     priv = (xenUnifiedPrivatePtr) dom->conn->privateData;
-    return xenLinuxDomainBlockStats (priv, dom, path, stats);
+    xenUnifiedLock(priv);
+    /* Need to lock because it hits the xenstore handle :-( */
+    ret = xenLinuxDomainBlockStats (priv, dom, path, stats);
+    xenUnifiedUnlock(priv);
+    return ret;
 #else
     virXenErrorFunc (dom->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__,
                      "block statistics not supported on this platform",
@@ -2627,8 +2632,10 @@ xenHypervisorLookupDomainByID(virConnect
     if (XEN_GETDOMAININFO_DOMAIN(dominfo) != id)
         return (NULL);
 
-
-    if (!(name = xenStoreDomainGetName(conn, id)))
+    xenUnifiedLock(priv);
+    name = xenStoreDomainGetName(conn, id);
+    xenUnifiedUnlock(priv);
+    if (!name)
         return (NULL);
 
     ret = virGetDomain(conn, name, XEN_GETDOMAININFO_UUID(dominfo));
@@ -2695,7 +2702,10 @@ xenHypervisorLookupDomainByUUID(virConne
     if (id == -1)
         return (NULL);
 
-    if (!(name = xenStoreDomainGetName(conn, id)))
+    xenUnifiedLock(priv);
+    name = xenStoreDomainGetName(conn, id);
+    xenUnifiedUnlock(priv);
+    if (!name)
         return (NULL);
 
     ret = virGetDomain(conn, name, uuid);
@@ -2928,7 +2938,6 @@ xenHypervisorNodeGetCellsFreeMemory(virC
     xen_op_v2_sys op_sys;
     int i, j, ret;
     xenUnifiedPrivatePtr priv;
-    int nbNodeCells;
 
     if (conn == NULL) {
         virXenErrorFunc (conn, VIR_ERR_INVALID_ARG, __FUNCTION__,
@@ -2936,14 +2945,15 @@ xenHypervisorNodeGetCellsFreeMemory(virC
         return -1;
     }
 
-    nbNodeCells = xenNbCells(conn);
-    if (nbNodeCells < 0) {
+    priv = conn->privateData;
+
+    if (priv->nbNodeCells < 0) {
         virXenErrorFunc (conn, VIR_ERR_XEN_CALL, __FUNCTION__,
                          "cannot determine actual number of cells",0);
         return(-1);
     }
 
-    if ((maxCells < 1) || (startCell >= nbNodeCells)) {
+    if ((maxCells < 1) || (startCell >= priv->nbNodeCells)) {
         virXenErrorFunc (conn, VIR_ERR_INVALID_ARG, __FUNCTION__,
                         "invalid argument", 0);
         return -1;
@@ -2958,7 +2968,6 @@ xenHypervisorNodeGetCellsFreeMemory(virC
         return -1;
     }
 
-    priv = (xenUnifiedPrivatePtr) conn->privateData;
     if (priv->handle < 0) {
         virXenErrorFunc (conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
                         "priv->handle invalid", 0);
@@ -2968,7 +2977,7 @@ xenHypervisorNodeGetCellsFreeMemory(virC
     memset(&op_sys, 0, sizeof(op_sys));
     op_sys.cmd = XEN_V2_OP_GETAVAILHEAP;
 
-    for (i = startCell, j = 0;(i < nbNodeCells) && (j < maxCells);i++,j++) {
+    for (i = startCell, j = 0;(i < priv->nbNodeCells) && (j < maxCells);i++,j++) {
         op_sys.u.availheap.node = i;
         ret = xenHypervisorDoV2Sys(priv->handle, &op_sys);
         if (ret < 0) {
diff --git a/src/xen_unified.c b/src/xen_unified.c
--- a/src/xen_unified.c
+++ b/src/xen_unified.c
@@ -56,7 +56,7 @@ xenUnifiedDomainGetVcpus (virDomainPtr d
                           unsigned char *cpumaps, int maplen);
 
 /* The five Xen drivers below us. */
-static struct xenUnifiedDriver *drivers[XEN_UNIFIED_NR_DRIVERS] = {
+static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = {
     [XEN_UNIFIED_HYPERVISOR_OFFSET] = &xenHypervisorDriver,
     [XEN_UNIFIED_PROXY_OFFSET] = &xenProxyDriver,
     [XEN_UNIFIED_XEND_OFFSET] = &xenDaemonDriver,
@@ -71,14 +71,6 @@ static struct xenUnifiedDriver *drivers[
         virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__,           \
                                __FUNCTION__, __LINE__, fmt)
 
-/*
- * Helper functions currently used in the NUMA code
- * Those variables should not be accessed directly but through helper
- * functions xenNbCells() and xenNbCpu() available to all Xen backends
- */
-static int nbNodeCells = -1;
-static int nbNodeCpus = -1;
-
 /**
  * xenNumaInit:
  * @conn: pointer to the hypervisor connection
@@ -90,42 +82,19 @@ static int nbNodeCpus = -1;
 static void
 xenNumaInit(virConnectPtr conn) {
     virNodeInfo nodeInfo;
+    xenUnifiedPrivatePtr priv;
     int ret;
 
     ret = xenUnifiedNodeGetInfo(conn, &nodeInfo);
     if (ret < 0)
         return;
-    nbNodeCells = nodeInfo.nodes;
-    nbNodeCpus = nodeInfo.cpus;
+
+    priv = conn->privateData;
+
+    priv->nbNodeCells = nodeInfo.nodes;
+    priv->nbNodeCpus = nodeInfo.cpus;
 }
 
-/**
- * xenNbCells:
- * @conn: pointer to the hypervisor connection
- *
- * Number of NUMA cells present in the actual Node
- *
- * Returns the number of NUMA cells available on that Node
- */
-int xenNbCells(virConnectPtr conn) {
-    if (nbNodeCells < 0)
-        xenNumaInit(conn);
-    return(nbNodeCells);
-}
-
-/**
- * xenNbCpus:
- * @conn: pointer to the hypervisor connection
- *
- * Number of CPUs present in the actual Node
- *
- * Returns the number of CPUs available on that Node
- */
-int xenNbCpus(virConnectPtr conn) {
-    if (nbNodeCpus < 0)
-        xenNumaInit(conn);
-    return(nbNodeCpus);
-}
 
 /**
  * xenDomainUsedCpus:
@@ -141,7 +110,7 @@ char *
 xenDomainUsedCpus(virDomainPtr dom)
 {
     char *res = NULL;
-    int nb_cpu, ncpus;
+    int ncpus;
     int nb_vcpu;
     char *cpulist = NULL;
     unsigned char *cpumap = NULL;
@@ -150,12 +119,14 @@ xenDomainUsedCpus(virDomainPtr dom)
     int n, m;
     virVcpuInfoPtr cpuinfo = NULL;
     virNodeInfo nodeinfo;
+    xenUnifiedPrivatePtr priv;
 
     if (!VIR_IS_CONNECTED_DOMAIN(dom))
         return (NULL);
 
-    nb_cpu = xenNbCpus(dom->conn);
-    if (nb_cpu <= 0)
+    priv = dom->conn->privateData;
+
+    if (priv->nbNodeCpus <= 0)
         return(NULL);
     nb_vcpu = xenUnifiedDomainGetMaxVcpus(dom);
     if (nb_vcpu <= 0)
@@ -163,7 +134,7 @@ xenDomainUsedCpus(virDomainPtr dom)
     if (xenUnifiedNodeGetInfo(dom->conn, &nodeinfo) < 0)
         return(NULL);
 
-    if (VIR_ALLOC_N(cpulist, nb_cpu) < 0)
+    if (VIR_ALLOC_N(cpulist, priv->nbNodeCpus) < 0)
         goto done;
     if (VIR_ALLOC_N(cpuinfo, nb_vcpu) < 0)
         goto done;
@@ -175,19 +146,19 @@ xenDomainUsedCpus(virDomainPtr dom)
     if ((ncpus = xenUnifiedDomainGetVcpus(dom, cpuinfo, nb_vcpu,
                                           cpumap, cpumaplen)) >= 0) {
         for (n = 0 ; n < ncpus ; n++) {
-            for (m = 0 ; m < nb_cpu; m++) {
+            for (m = 0 ; m < priv->nbNodeCpus; m++) {
                 if ((cpulist[m] == 0) &&
                     (VIR_CPU_USABLE(cpumap, cpumaplen, n, m))) {
                     cpulist[m] = 1;
                     nb++;
                     /* if all CPU are used just return NULL */
-                    if (nb == nb_cpu)
+                    if (nb == priv->nbNodeCpus)
                         goto done;
 
                 }
             }
         }
-        res = virDomainCpuSetFormat(dom->conn, cpulist, nb_cpu);
+        res = virDomainCpuSetFormat(dom->conn, cpulist, priv->nbNodeCpus);
     }
 
 done:
@@ -267,13 +238,22 @@ xenUnifiedOpen (virConnectPtr conn, virC
         xenUnifiedError (NULL, VIR_ERR_NO_MEMORY, "allocating private data");
         return VIR_DRV_OPEN_ERROR;
     }
-    conn->privateData = priv;
+    if (virMutexInit(&priv->lock) < 0) {
+        xenUnifiedError (NULL, VIR_ERR_INTERNAL_ERROR,
+                         _("cannot initialise mutex"));
+        VIR_FREE(priv);
+        return VIR_DRV_OPEN_ERROR;
+    }
 
     /* Allocate callback list */
     if (VIR_ALLOC(cbList) < 0) {
         xenUnifiedError (NULL, VIR_ERR_NO_MEMORY, "allocating callback list");
+        virMutexDestroy(&priv->lock);
+        VIR_FREE(priv);
         return VIR_DRV_OPEN_ERROR;
     }
+    conn->privateData = priv;
+
     priv->domainEventCallbacks = cbList;
 
     priv->handle = -1;
@@ -344,6 +324,8 @@ xenUnifiedOpen (virConnectPtr conn, virC
         }
     }
 
+    xenNumaInit(conn);
+
     if (!(priv->caps = xenHypervisorMakeCapabilities(conn))) {
         DEBUG0("Failed to make capabilities");
         goto fail;
@@ -368,7 +350,9 @@ clean:
     DEBUG0("Failed to activate a mandatory sub-driver");
     for (i = 0 ; i < XEN_UNIFIED_NR_DRIVERS ; i++)
         if (priv->opened[i]) drivers[i]->close(conn);
+    virMutexDestroy(&priv->lock);
     VIR_FREE(priv);
+    conn->privateData = NULL;
     return ret;
 }
 
@@ -388,7 +372,8 @@ xenUnifiedClose (virConnectPtr conn)
         if (priv->opened[i] && drivers[i]->close)
             (void) drivers[i]->close (conn);
 
-    free (conn->privateData);
+    virMutexDestroy(&priv->lock);
+    VIR_FREE(conn->privateData);
     conn->privateData = NULL;
 
     return 0;
@@ -497,17 +482,15 @@ xenUnifiedNodeGetInfo (virConnectPtr con
 static char *
 xenUnifiedGetCapabilities (virConnectPtr conn)
 {
-    GET_PRIVATE(conn);
-    int i;
-    char *ret;
+    xenUnifiedPrivatePtr priv = conn->privateData;
+    char *xml;
 
-    for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
-        if (priv->opened[i] && drivers[i]->getCapabilities) {
-            ret = drivers[i]->getCapabilities (conn);
-            if (ret) return ret;
-        }
+    if (!(xml = virCapabilitiesFormatXML(priv->caps))) {
+        virReportOOMError(conn);
+        return NULL;
+    }
 
-    return NULL;
+    return xml;
 }
 
 static int
@@ -1019,7 +1002,9 @@ xenUnifiedDomainDumpXML (virDomainPtr do
     } else {
         if (priv->opened[XEN_UNIFIED_XEND_OFFSET]) {
             char *cpus, *res;
+            xenUnifiedLock(priv);
             cpus = xenDomainUsedCpus(dom);
+            xenUnifiedUnlock(priv);
             res = xenDaemonDomainDumpXML(dom, flags, cpus);
             VIR_FREE(cpus);
             return(res);
@@ -1363,14 +1348,20 @@ xenUnifiedDomainEventRegister (virConnec
                                void (*freefunc)(void *))
 {
     GET_PRIVATE (conn);
+    int ret;
+
+    xenUnifiedLock(priv);
     if (priv->xsWatch == -1) {
         xenUnifiedError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+        xenUnifiedUnlock(priv);
         return -1;
     }
 
-    conn->refs++;
-    return virDomainEventCallbackListAdd(conn, priv->domainEventCallbacks,
-                                         callback, opaque, freefunc);
+    virConnectRef(conn);
+    ret = virDomainEventCallbackListAdd(conn, priv->domainEventCallbacks,
+                                        callback, opaque, freefunc);
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 static int
@@ -1379,14 +1370,22 @@ xenUnifiedDomainEventDeregister (virConn
 {
     int ret;
     GET_PRIVATE (conn);
+
+    xenUnifiedLock(priv);
     if (priv->xsWatch == -1) {
         xenUnifiedError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+        xenUnifiedUnlock(priv);
         return -1;
     }
 
-    ret = virDomainEventCallbackListRemove(conn, priv->domainEventCallbacks,
-                                            callback);
+    if (priv->domainEventDispatching)
+        ret = virDomainEventCallbackListMarkDelete(conn, priv->domainEventCallbacks,
+                                                   callback);
+    else
+        ret = virDomainEventCallbackListRemove(conn, priv->domainEventCallbacks,
+                                               callback);
     virUnrefConnect(conn);
+    xenUnifiedUnlock(priv);
     return ret;
 }
 
@@ -1573,21 +1572,66 @@ xenUnifiedRemoveDomainInfo(xenUnifiedDom
     return -1;
 }
 
+static void
+xenUnifiedDomainEventDispatchFunc(virConnectPtr conn,
+                                  virDomainEventPtr event,
+                                  virConnectDomainEventCallback cb,
+                                  void *cbopaque,
+                                  void *opaque)
+{
+    xenUnifiedPrivatePtr priv = opaque;
+
+    /*
+     * Release the lock while the callback is running so that
+     * we're re-entrant safe for callback work - the callback
+     * may want to invoke other virt functions & we have already
+     * protected the one piece of state we have - the callback
+     * list
+     */
+    xenUnifiedUnlock(priv);
+    virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL);
+    xenUnifiedLock(priv);
+}
+
 /**
  * xenUnifiedDomainEventDispatch:
+ * @priv: the connection to dispatch events on
+ * @event: the event to dispatch
  *
  * Dispatch domain events to registered callbacks
  *
+ * The caller must hold the lock in 'priv' before invoking
+ *
  */
 void xenUnifiedDomainEventDispatch (xenUnifiedPrivatePtr priv,
                                     virDomainEventPtr event)
 {
-    if (!priv || !priv->domainEventCallbacks)
+    if (!priv)
         return;
 
-    virDomainEventDispatch(event,
-                           priv->domainEventCallbacks,
-                           virDomainEventDispatchDefaultFunc,
-                           NULL);
+    priv->domainEventDispatching = 1;
+
+    if (priv->domainEventCallbacks) {
+        virDomainEventDispatch(event,
+                               priv->domainEventCallbacks,
+                               xenUnifiedDomainEventDispatchFunc,
+                               priv);
+
+        /* Purge any deleted callbacks */
+        virDomainEventCallbackListPurgeMarked(priv->domainEventCallbacks);
+    }
+
     virDomainEventFree(event);
+
+    priv->domainEventDispatching = 0;
 }
+
+void xenUnifiedLock(xenUnifiedPrivatePtr priv)
+{
+    virMutexLock(&priv->lock);
+}
+
+void xenUnifiedUnlock(xenUnifiedPrivatePtr priv)
+{
+    virMutexUnlock(&priv->lock);
+}
diff --git a/src/xen_unified.h b/src/xen_unified.h
--- a/src/xen_unified.h
+++ b/src/xen_unified.h
@@ -131,6 +131,12 @@ typedef xenUnifiedDomainInfoList *xenUni
  * low-level drivers access parts of this structure.
  */
 struct _xenUnifiedPrivate {
+    virMutex lock;
+
+    /* These initial vars are initialized in Open method
+     * and readonly thereafter, so can be used without
+     * holding the lock
+     */
     virCapsPtr caps;
     int handle;			/* Xen hypervisor handle */
 
@@ -144,25 +150,36 @@ struct _xenUnifiedPrivate {
     struct sockaddr_un addr_un; /* the unix address */
     struct sockaddr_in addr_in; /* the inet address */
 
-    struct xs_handle *xshandle; /* handle to talk to the xenstore */
-
-    int proxy;                  /* fd of proxy. */
-
     /* Keep track of the drivers which opened.  We keep a yes/no flag
      * here for each driver, corresponding to the array drivers in
      * xen_unified.c.
      */
     int opened[XEN_UNIFIED_NR_DRIVERS];
 
+
+    /*
+     * Everything from this point onwards must be protected
+     * by the lock when used
+     */
+
+    struct xs_handle *xshandle; /* handle to talk to the xenstore */
+
+    int proxy;                  /* fd of proxy. */
+
+
     /* A list of xenstore watches */
     xenStoreWatchListPtr xsWatchList;
     int xsWatch;
     /* A list of active domain name/uuids */
     xenUnifiedDomainInfoListPtr activeDomainList;
 
+    /* NUMA topology info cache */
+    int nbNodeCells;
+    int nbNodeCpus;
 
     /* An list of callbacks */
     virDomainEventCallbackListPtr domainEventCallbacks;
+    int domainEventDispatching;
 
     /* Location of config files, either /etc
      * or /var/lib/xen */
@@ -188,9 +205,6 @@ struct _xenUnifiedPrivate {
 
 typedef struct _xenUnifiedPrivate *xenUnifiedPrivatePtr;
 
-
-int xenNbCells(virConnectPtr conn);
-int xenNbCpus(virConnectPtr conn);
 char *xenDomainUsedCpus(virDomainPtr dom);
 
 void xenUnifiedDomainInfoListFree(xenUnifiedDomainInfoListPtr info);
@@ -204,4 +218,12 @@ void xenUnifiedDomainEventDispatch (xenU
                                     virDomainEventPtr event);
 unsigned long xenUnifiedVersion(void);
 
+#ifndef PROXY
+void xenUnifiedLock(xenUnifiedPrivatePtr priv);
+void xenUnifiedUnlock(xenUnifiedPrivatePtr priv);
+#else
+#define xenUnifiedLock(p) do {} while(0)
+#define xenUnifiedUnlock(p) do {} while(0)
+#endif
+
 #endif /* __VIR_XEN_UNIFIED_H__ */
diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -1964,18 +1964,25 @@ xenDaemonParseSxprGraphicsOld(virConnect
                               int hvm,
                               int xendConfigVersion)
 {
+#ifndef PROXY
+    xenUnifiedPrivatePtr priv = conn->privateData;
+#endif
     const char *tmp;
     virDomainGraphicsDefPtr graphics = NULL;
 
     if ((tmp = sexpr_fmt_node(root, "domain/image/%s/vnc", hvm ? "hvm" : "linux")) &&
         tmp[0] == '1') {
         /* Graphics device (HVM, or old (pre-3.0.4) style PV VNC config) */
-        int port = xenStoreDomainGetVNCPort(conn, def->id);
+        int port;
         const char *listenAddr = sexpr_fmt_node(root, "domain/image/%s/vnclisten", hvm ? "hvm" : "linux");
         const char *vncPasswd = sexpr_fmt_node(root, "domain/image/%s/vncpasswd", hvm ? "hvm" : "linux");
         const char *keymap = sexpr_fmt_node(root, "domain/image/%s/keymap", hvm ? "hvm" : "linux");
         const char *unused = sexpr_fmt_node(root, "domain/image/%s/vncunused", hvm ? "hvm" : "linux");
 
+        xenUnifiedLock(priv);
+        port = xenStoreDomainGetVNCPort(conn, def->id);
+        xenUnifiedUnlock(priv);
+
         if (VIR_ALLOC(graphics) < 0)
             goto no_memory;
 
@@ -2040,6 +2047,9 @@ xenDaemonParseSxprGraphicsNew(virConnect
                               virDomainDefPtr def,
                               const struct sexpr *root)
 {
+#ifndef PROXY
+    xenUnifiedPrivatePtr priv = conn->privateData;
+#endif
     virDomainGraphicsDefPtr graphics = NULL;
     const struct sexpr *cur, *node;
     const char *tmp;
@@ -2071,16 +2081,21 @@ xenDaemonParseSxprGraphicsNew(virConnect
                     !(graphics->data.sdl.xauth = strdup(xauth)))
                     goto no_memory;
             } else {
-                int port = xenStoreDomainGetVNCPort(conn, def->id);
-                if (port == -1) {
-                    // Didn't find port entry in xenstore
-                    port = sexpr_int(node, "device/vfb/vncdisplay");
-                }
+                int port;
                 const char *listenAddr = sexpr_node(node, "device/vfb/vnclisten");
                 const char *vncPasswd = sexpr_node(node, "device/vfb/vncpasswd");;
                 const char *keymap = sexpr_node(node, "device/vfb/keymap");
                 const char *unused = sexpr_node(node, "device/vfb/vncunused");
 
+                xenUnifiedLock(priv);
+                port = xenStoreDomainGetVNCPort(conn, def->id);
+                xenUnifiedUnlock(priv);
+
+                if (port == -1) {
+                    // Didn't find port entry in xenstore
+                    port = sexpr_int(node, "device/vfb/vncdisplay");
+                }
+
                 if ((unused && STREQ(unused, "1")) || port == -1) {
                     graphics->data.vnc.autoport = 1;
                     port = -1;
@@ -2137,6 +2152,9 @@ xenDaemonParseSxpr(virConnectPtr conn,
                    int xendConfigVersion,
                    const char *cpus)
 {
+#ifndef PROXY
+    xenUnifiedPrivatePtr priv = conn->privateData;
+#endif
     const char *tmp;
     virDomainDefPtr def;
     int hvm = 0;
@@ -2356,7 +2374,9 @@ xenDaemonParseSxpr(virConnectPtr conn,
         goto error;
 
     /* Character device config */
+    xenUnifiedLock(priv);
     tty = xenStoreDomainGetConsolePath(conn, def->id);
+    xenUnifiedUnlock(priv);
     if (hvm) {
         tmp = sexpr_node(root, "domain/image/hvm/serial");
         if (tmp && STRNEQ(tmp, "none")) {
@@ -5464,14 +5484,17 @@ virDomainXMLDevID(virDomainPtr domain,
                   char *ref,
                   int ref_len)
 {
+    xenUnifiedPrivatePtr priv = domain->conn->privateData;
     char *xref;
 
     if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
         strcpy(class, "vbd");
         if (dev->data.disk->dst == NULL)
             return -1;
+        xenUnifiedLock(priv);
         xref = xenStoreDomainGetDiskID(domain->conn, domain->id,
                                        dev->data.disk->dst);
+        xenUnifiedUnlock(priv);
         if (xref == NULL)
             return -1;
 
@@ -5487,8 +5510,10 @@ virDomainXMLDevID(virDomainPtr domain,
 
         strcpy(class, "vif");
 
+        xenUnifiedLock(priv);
         xref = xenStoreDomainGetNetworkID(domain->conn, domain->id,
                                           mac);
+        xenUnifiedUnlock(priv);
         if (xref == NULL)
             return -1;
 
diff --git a/src/xm_internal.c b/src/xm_internal.c
--- a/src/xm_internal.c
+++ b/src/xm_internal.c
@@ -339,6 +339,11 @@ xenXMConfigSaveFile(virConnectPtr conn, 
     return ret;
 }
 
+
+/*
+ * Caller must hold the lock on 'conn->privateData' before
+ * calling this funtion
+ */
 int
 xenXMConfigCacheRemoveFile(virConnectPtr conn,
                            const char *filename)
@@ -359,6 +364,10 @@ xenXMConfigCacheRemoveFile(virConnectPtr
 }
 
 
+/*
+ * Caller must hold the lock on 'conn->privateData' before
+ * calling this funtion
+ */
 int
 xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
 {
@@ -452,10 +461,14 @@ xenXMConfigCacheAddFile(virConnectPtr co
 }
 
 /* This method is called by various methods to scan /etc/xen
-   (or whatever directory was set by  LIBVIRT_XM_CONFIG_DIR
-   environment variable) and process any domain configs. It
-   has rate-limited so never rescans more frequently than
-   once every X seconds */
+ * (or whatever directory was set by  LIBVIRT_XM_CONFIG_DIR
+ * environment variable) and process any domain configs. It
+ * has rate-limited so never rescans more frequently than
+ * once every X seconds
+ *
+ * Caller must hold the lock on 'conn->privateData' before
+ * calling this funtion
+ */
 int xenXMConfigCacheRefresh (virConnectPtr conn) {
     xenUnifiedPrivatePtr priv = conn->privateData;
     DIR *dh;
@@ -613,12 +626,13 @@ int xenXMDomainGetInfo(virDomainPtr doma
         return (-1);
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (-1);
+        goto error;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (-1);
+        goto error;
 
     memset(info, 0, sizeof(virDomainInfo));
     info->maxMem = entry->def->maxmem;
@@ -627,8 +641,12 @@ int xenXMDomainGetInfo(virDomainPtr doma
     info->state = VIR_DOMAIN_SHUTOFF;
     info->cpuTime = 0;
 
+    xenUnifiedUnlock(priv);
     return (0);
 
+error:
+    xenUnifiedUnlock(priv);
+    return -1;
 }
 
 #define MAX_VFB 1024
@@ -1285,6 +1303,7 @@ char *xenXMDomainDumpXML(virDomainPtr do
     xenUnifiedPrivatePtr priv = domain->conn->privateData;
     const char *filename;
     xenXMConfCachePtr entry;
+    char *ret = NULL;
 
     if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
@@ -1295,14 +1314,19 @@ char *xenXMDomainDumpXML(virDomainPtr do
         return (NULL);
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (NULL);
+        goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (NULL);
+        goto cleanup;
 
-    return virDomainDefFormat(domain->conn, entry->def, flags);
+    ret = virDomainDefFormat(domain->conn, entry->def, flags);
+
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 
@@ -1313,6 +1337,7 @@ int xenXMDomainSetMemory(virDomainPtr do
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
+    int ret = -1;
 
     if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
@@ -1327,12 +1352,13 @@ int xenXMDomainSetMemory(virDomainPtr do
         return (-1);
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (-1);
+        goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (-1);
+        goto cleanup;
 
     entry->def->memory = memory;
     if (entry->def->memory > entry->def->maxmem)
@@ -1342,9 +1368,12 @@ int xenXMDomainSetMemory(virDomainPtr do
      * in-memory representation of the config file. I say not!
      */
     if (xenXMConfigSaveFile(domain->conn, entry->filename, entry->def) < 0)
-        return (-1);
+        goto cleanup;
+    ret = 0;
 
-    return (0);
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /*
@@ -1354,6 +1383,7 @@ int xenXMDomainSetMaxMemory(virDomainPtr
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
+    int ret = -1;
 
     if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
@@ -1366,12 +1396,13 @@ int xenXMDomainSetMaxMemory(virDomainPtr
         return (-1);
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (-1);
+        goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (-1);
+        goto cleanup;
 
     entry->def->maxmem = memory;
     if (entry->def->memory > entry->def->maxmem)
@@ -1381,9 +1412,12 @@ int xenXMDomainSetMaxMemory(virDomainPtr
      * in-memory representation of the config file. I say not!
      */
     if (xenXMConfigSaveFile(domain->conn, entry->filename, entry->def) < 0)
-        return (-1);
+        goto cleanup;
+    ret = 0;
 
-    return (0);
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /*
@@ -1393,24 +1427,30 @@ unsigned long xenXMDomainGetMaxMemory(vi
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
+    unsigned long ret = 0;
 
     if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
                    __FUNCTION__);
-        return (-1);
+        return (0);
     }
     if (domain->id != -1)
-        return (-1);
+        return (0);
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (-1);
+        goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (-1);
+        goto cleanup;
 
-    return entry->def->maxmem;
+    ret = entry->def->maxmem;
+
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /*
@@ -1420,6 +1460,7 @@ int xenXMDomainSetVcpus(virDomainPtr dom
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
+    int ret = -1;
 
     if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
@@ -1432,12 +1473,13 @@ int xenXMDomainSetVcpus(virDomainPtr dom
         return (-1);
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (-1);
+        goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (-1);
+        goto cleanup;
 
     entry->def->vcpus = vcpus;
 
@@ -1445,9 +1487,12 @@ int xenXMDomainSetVcpus(virDomainPtr dom
      * in-memory representation of the config file. I say not!
      */
     if (xenXMConfigSaveFile(domain->conn, entry->filename, entry->def) < 0)
-        return (-1);
+        goto cleanup;
+    ret = 0;
 
-    return (0);
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /**
@@ -1493,15 +1538,16 @@ int xenXMDomainPinVcpu(virDomainPtr doma
     }
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name))) {
         xenXMError (domain->conn, VIR_ERR_INTERNAL_ERROR, "%s", _("virHashLookup"));
-        return -1;
+        goto cleanup;
     }
     if (!(entry = virHashLookup(priv->configCache, filename))) {
         xenXMError (domain->conn, VIR_ERR_INTERNAL_ERROR,
                     "%s", _("can't retrieve config file for domain"));
-        return -1;
+        goto cleanup;
     }
 
     /* from bit map, build character string of mapped CPU numbers */
@@ -1519,7 +1565,7 @@ int xenXMDomainPinVcpu(virDomainPtr doma
 
     if (virBufferError(&mapbuf)) {
         virReportOOMError(domain->conn);
-        return -1;
+        goto cleanup;
     }
 
     mapstr = virBufferContentAndReset(&mapbuf);
@@ -1546,6 +1592,7 @@ int xenXMDomainPinVcpu(virDomainPtr doma
  cleanup:
     VIR_FREE(mapstr);
     VIR_FREE(cpuset);
+    xenUnifiedUnlock(priv);
     return (ret);
 }
 
@@ -1556,7 +1603,7 @@ virDomainPtr xenXMDomainLookupByName(vir
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
-    virDomainPtr ret;
+    virDomainPtr ret = NULL;
 
     if (!VIR_IS_CONNECT(conn)) {
         xenXMError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1568,27 +1615,28 @@ virDomainPtr xenXMDomainLookupByName(vir
     }
 
     priv = conn->privateData;
+    xenUnifiedLock(priv);
 
 #ifndef WITH_XEN_INOTIFY
     if (xenXMConfigCacheRefresh (conn) < 0)
-        return (NULL);
+        goto cleanup;
 #endif
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domname)))
-        return (NULL);
+        goto cleanup;
 
-    if (!(entry = virHashLookup(priv->configCache, filename))) {
-        return (NULL);
-    }
+    if (!(entry = virHashLookup(priv->configCache, filename)))
+        goto cleanup;
 
-    if (!(ret = virGetDomain(conn, domname, entry->def->uuid))) {
-        return (NULL);
-    }
+    if (!(ret = virGetDomain(conn, domname, entry->def->uuid)))
+        goto cleanup;
 
     /* Ensure its marked inactive, because may be cached
        handle to a previously active domain */
     ret->id = -1;
 
+cleanup:
+    xenUnifiedUnlock(priv);
     return (ret);
 }
 
@@ -1613,7 +1661,7 @@ virDomainPtr xenXMDomainLookupByUUID(vir
                                      const unsigned char *uuid) {
     xenUnifiedPrivatePtr priv;
     xenXMConfCachePtr entry;
-    virDomainPtr ret;
+    virDomainPtr ret = NULL;
 
     if (!VIR_IS_CONNECT(conn)) {
         xenXMError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1625,24 +1673,25 @@ virDomainPtr xenXMDomainLookupByUUID(vir
     }
 
     priv = conn->privateData;
+    xenUnifiedLock(priv);
 
 #ifndef WITH_XEN_INOTIFY
     if (xenXMConfigCacheRefresh (conn) < 0)
-        return (NULL);
+        goto cleanup;
 #endif
 
-    if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, (const void *)uuid))) {
-        return (NULL);
-    }
+    if (!(entry = virHashSearch(priv->configCache, xenXMDomainSearchForUUID, (const void *)uuid)))
+        goto cleanup;
 
-    if (!(ret = virGetDomain(conn, entry->def->name, uuid))) {
-        return (NULL);
-    }
+    if (!(ret = virGetDomain(conn, entry->def->name, uuid)))
+        goto cleanup;
 
     /* Ensure its marked inactive, because may be cached
        handle to a previously active domain */
     ret->id = -1;
 
+cleanup:
+    xenUnifiedUnlock(priv);
     return (ret);
 }
 
@@ -1652,7 +1701,7 @@ virDomainPtr xenXMDomainLookupByUUID(vir
  */
 int xenXMDomainCreate(virDomainPtr domain) {
     char *sexpr;
-    int ret;
+    int ret = -1;
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
@@ -1662,43 +1711,45 @@ int xenXMDomainCreate(virDomainPtr domai
     if (domain->id != -1)
         return (-1);
 
+    xenUnifiedLock(priv);
+
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (-1);
+        goto error;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (-1);
+        goto error;
 
     if (!(sexpr = xenDaemonFormatSxpr(domain->conn, entry->def, priv->xendConfigVersion))) {
         xenXMError(domain->conn, VIR_ERR_XML_ERROR,
                    "%s", _("failed to build sexpr"));
-        return (-1);
+        goto error;
     }
 
     ret = xenDaemonDomainCreateXML(domain->conn, sexpr);
     VIR_FREE(sexpr);
-    if (ret != 0) {
-        return (-1);
-    }
+    if (ret != 0)
+        goto error;
 
     if ((ret = xenDaemonDomainLookupByName_ids(domain->conn, domain->name,
-                                               entry->def->uuid)) < 0) {
-        return (-1);
-    }
+                                               entry->def->uuid)) < 0)
+        goto error;
     domain->id = ret;
 
     if ((ret = xend_wait_for_devices(domain->conn, domain->name)) < 0)
-        goto cleanup;
+        goto error;
 
     if ((ret = xenDaemonDomainResume(domain)) < 0)
-        goto cleanup;
+        goto error;
 
+    xenUnifiedUnlock(priv);
     return (0);
 
- cleanup:
+ error:
     if (domain->id != -1) {
         xenDaemonDomainDestroy(domain);
         domain->id = -1;
     }
+    xenUnifiedUnlock(priv);
     return (-1);
 }
 
@@ -2281,14 +2332,20 @@ virDomainPtr xenXMDomainDefineXML(virCon
     if (conn->flags & VIR_CONNECT_RO)
         return (NULL);
 
+    xenUnifiedLock(priv);
+
 #ifndef WITH_XEN_INOTIFY
-    if (xenXMConfigCacheRefresh (conn) < 0)
+    if (xenXMConfigCacheRefresh (conn) < 0) {
+        xenUnifiedUnlock(priv);
         return (NULL);
+    }
 #endif
 
     if (!(def = virDomainDefParseString(conn, priv->caps, xml,
-                                        VIR_DOMAIN_XML_INACTIVE)))
+                                        VIR_DOMAIN_XML_INACTIVE))) {
+        xenUnifiedUnlock(priv);
         return (NULL);
+    }
 
     if (virHashLookup(priv->nameConfigMap, def->name)) {
         /* domain exists, we will overwrite it */
@@ -2366,16 +2423,16 @@ virDomainPtr xenXMDomainDefineXML(virCon
         goto error;
     }
 
-    if (!(ret = virGetDomain(conn, def->name, def->uuid)))
-        return NULL;
+    if ((ret = virGetDomain(conn, def->name, def->uuid)))
+        ret->id = -1;
 
-    ret->id = -1;
-
+    xenUnifiedUnlock(priv);
     return (ret);
 
  error:
     VIR_FREE(entry);
     virDomainDefFree(def);
+    xenUnifiedUnlock(priv);
     return (NULL);
 }
 
@@ -2386,6 +2443,8 @@ int xenXMDomainUndefine(virDomainPtr dom
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
+    int ret = -1;
+
     if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
         xenXMError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
                    __FUNCTION__);
@@ -2398,25 +2457,30 @@ int xenXMDomainUndefine(virDomainPtr dom
         return (-1);
 
     priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
 
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return (-1);
+        goto cleanup;
 
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return (-1);
+        goto cleanup;
 
     if (unlink(entry->filename) < 0)
-        return (-1);
+        goto cleanup;
 
     /* Remove the name -> filename mapping */
     if (virHashRemoveEntry(priv->nameConfigMap, domain->name, NULL) < 0)
-        return(-1);
+        goto cleanup;
 
     /* Remove the config record itself */
     if (virHashRemoveEntry(priv->configCache, entry->filename, xenXMConfigFree) < 0)
-        return (-1);
+        goto cleanup;
 
-    return (0);
+    ret = 0;
+
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 struct xenXMListIteratorContext {
@@ -2450,6 +2514,7 @@ static void xenXMListIterator(const void
 int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) {
     xenUnifiedPrivatePtr priv;
     struct xenXMListIteratorContext ctx;
+    int ret = -1;
 
     if (!VIR_IS_CONNECT(conn)) {
         xenXMError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -2457,10 +2522,11 @@ int xenXMListDefinedDomains(virConnectPt
     }
 
     priv = conn->privateData;
+    xenUnifiedLock(priv);
 
 #ifndef WITH_XEN_INOTIFY
     if (xenXMConfigCacheRefresh (conn) < 0)
-        return (-1);
+        goto cleanup;
 #endif
 
     if (maxnames > virHashSize(priv->configCache))
@@ -2472,7 +2538,13 @@ int xenXMListDefinedDomains(virConnectPt
     ctx.names = names;
 
     virHashForEach(priv->nameConfigMap, xenXMListIterator, &ctx);
-    return (ctx.count);
+    ret = ctx.count;
+
+#ifndef WITH_XEN_INOTIFY
+cleanup:
+#endif
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /*
@@ -2481,6 +2553,7 @@ int xenXMListDefinedDomains(virConnectPt
  */
 int xenXMNumOfDefinedDomains(virConnectPtr conn) {
     xenUnifiedPrivatePtr priv;
+    int ret = -1;
 
     if (!VIR_IS_CONNECT(conn)) {
         xenXMError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -2488,13 +2561,20 @@ int xenXMNumOfDefinedDomains(virConnectP
     }
 
     priv = conn->privateData;
+    xenUnifiedLock(priv);
 
 #ifndef WITH_XEN_INOTIFY
     if (xenXMConfigCacheRefresh (conn) < 0)
-        return (-1);
+        goto cleanup;
 #endif
 
-    return virHashSize(priv->nameConfigMap);
+    ret = virHashSize(priv->nameConfigMap);
+
+#ifndef WITH_XEN_INOTIFY
+cleanup:
+#endif
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 
@@ -2523,24 +2603,25 @@ xenXMDomainAttachDevice(virDomainPtr dom
         return -1;
     }
 
-    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
-
     if (domain->conn->flags & VIR_CONNECT_RO)
         return -1;
     if (domain->id != -1)
         return -1;
 
+    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
+    xenUnifiedLock(priv);
+
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return -1;
+        goto cleanup;
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return -1;
+        goto cleanup;
     def = entry->def;
 
     if (!(dev = virDomainDeviceDefParse(domain->conn,
                                         priv->caps,
                                         entry->def,
                                         xml, VIR_DOMAIN_XML_INACTIVE)))
-        return -1;
+        goto cleanup;
 
     switch (dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
@@ -2583,7 +2664,7 @@ xenXMDomainAttachDevice(virDomainPtr dom
 
  cleanup:
     virDomainDeviceDefFree(dev);
-
+    xenUnifiedUnlock(priv);
     return ret;
 }
 
@@ -2613,23 +2694,25 @@ xenXMDomainDetachDevice(virDomainPtr dom
         return -1;
     }
 
-    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
-
     if (domain->conn->flags & VIR_CONNECT_RO)
         return -1;
     if (domain->id != -1)
         return -1;
+
+    priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
+    xenUnifiedLock(priv);
+
     if (!(filename = virHashLookup(priv->nameConfigMap, domain->name)))
-        return -1;
+        goto cleanup;
     if (!(entry = virHashLookup(priv->configCache, filename)))
-        return -1;
+        goto cleanup;
     def = entry->def;
 
     if (!(dev = virDomainDeviceDefParse(domain->conn,
                                         priv->caps,
                                         entry->def,
                                         xml, VIR_DOMAIN_XML_INACTIVE)))
-        return -1;
+        goto cleanup;
 
     switch (dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
@@ -2681,6 +2764,7 @@ xenXMDomainDetachDevice(virDomainPtr dom
 
  cleanup:
     virDomainDeviceDefFree(dev);
+    xenUnifiedUnlock(priv);
     return (ret);
 }
 
diff --git a/src/xs_internal.c b/src/xs_internal.c
--- a/src/xs_internal.c
+++ b/src/xs_internal.c
@@ -37,16 +37,10 @@
 #include "xs_internal.h"
 #include "xen_internal.h" /* for xenHypervisorCheckID */
 
-#ifdef __linux__
-#define XEN_HYPERVISOR_SOCKET "/proc/xen/privcmd"
-#elif defined(__sun)
-#define XEN_HYPERVISOR_SOCKET "/dev/xen/privcmd"
-#else
-#error "unsupported platform"
-#endif
-
 #ifndef PROXY
 static char *xenStoreDomainGetOSType(virDomainPtr domain);
+static void xenStoreWatchEvent(int watch, int fd, int events, void *data);
+static void xenStoreWatchListFree(xenStoreWatchListPtr list);
 
 struct xenUnifiedDriver xenStoreDriver = {
     xenStoreOpen, /* open */
@@ -374,6 +368,7 @@ xenStoreClose(virConnectPtr conn)
 
     priv = (xenUnifiedPrivatePtr) conn->privateData;
 
+#ifndef PROXY
     if (xenStoreRemoveWatch(conn, "@introduceDomain", "introduceDomain") < 0) {
         DEBUG0("Warning, could not remove @introduceDomain watch");
         /* not fatal */
@@ -385,7 +380,6 @@ xenStoreClose(virConnectPtr conn)
     }
 
     xenStoreWatchListFree(priv->xsWatchList);
-#ifndef PROXY
     xenUnifiedDomainInfoListFree(priv->activeDomainList);
 #endif
     if (priv->xshandle == NULL)
@@ -516,17 +510,21 @@ xenStoreDomainGetMaxMemory(virDomainPtr 
 {
     char *tmp;
     unsigned long ret = 0;
+    xenUnifiedPrivatePtr priv;
 
     if (!VIR_IS_CONNECTED_DOMAIN(domain))
         return (ret);
     if (domain->id == -1)
         return(-1);
 
+    priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
     tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target");
     if (tmp != NULL) {
         ret = (unsigned long) atol(tmp);
-        free(tmp);
+        VIR_FREE(tmp);
     }
+    xenUnifiedUnlock(priv);
     return(ret);
 }
 
@@ -589,12 +587,14 @@ xenStoreListDomains(virConnectPtr conn, 
     }
 
     priv = (xenUnifiedPrivatePtr) conn->privateData;
+
+    xenUnifiedLock(priv);
     if (priv->xshandle == NULL)
-        return(-1);
+        goto error;
 
     idlist = xs_directory (priv->xshandle, 0, "/local/domain", &num);
     if (idlist == NULL)
-        return(-1);
+        goto error;
 
     for (ret = 0, i = 0; (i < num) && (ret < maxids); i++) {
         id = strtol(idlist[i], &endptr, 10);
@@ -609,7 +609,12 @@ xenStoreListDomains(virConnectPtr conn, 
         ids[ret++] = (int) id;
     }
     free(idlist);
+    xenUnifiedUnlock(priv);
     return(ret);
+
+error:
+    xenUnifiedUnlock(priv);
+    return -1;
 }
 
 /**
@@ -694,6 +699,9 @@ done:
 int
 xenStoreDomainShutdown(virDomainPtr domain)
 {
+    int ret;
+    xenUnifiedPrivatePtr priv;
+
     if ((domain == NULL) || (domain->conn == NULL)) {
         virXenStoreError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
                          __FUNCTION__);
@@ -705,7 +713,11 @@ xenStoreDomainShutdown(virDomainPtr doma
      * this is very hackish, the domU kernel probes for a special
      * node in the xenstore and launch the shutdown command if found.
      */
-    return(virDomainDoStoreWrite(domain, "control/shutdown", "poweroff"));
+    priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
+    ret = virDomainDoStoreWrite(domain, "control/shutdown", "poweroff");
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /**
@@ -722,6 +734,9 @@ xenStoreDomainShutdown(virDomainPtr doma
 int
 xenStoreDomainReboot(virDomainPtr domain, unsigned int flags ATTRIBUTE_UNUSED)
 {
+    int ret;
+    xenUnifiedPrivatePtr priv;
+
     if ((domain == NULL) || (domain->conn == NULL)) {
         virXenStoreError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
                          __FUNCTION__);
@@ -733,7 +748,11 @@ xenStoreDomainReboot(virDomainPtr domain
      * this is very hackish, the domU kernel probes for a special
      * node in the xenstore and launch the shutdown command if found.
      */
-    return(virDomainDoStoreWrite(domain, "control/shutdown", "reboot"));
+    priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
+    ret = virDomainDoStoreWrite(domain, "control/shutdown", "reboot");
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /*
@@ -757,8 +776,11 @@ xenStoreDomainGetOSType(virDomainPtr dom
 
     vm = virDomainGetVM(domain);
     if (vm) {
+        xenUnifiedPrivatePtr priv = domain->conn->privateData;
+        xenUnifiedLock(priv);
         str = virDomainGetVMInfo(domain, vm, "image/ostype");
-        free(vm);
+        xenUnifiedUnlock(priv);
+        VIR_FREE(vm);
     }
 
     return (str);
@@ -773,6 +795,9 @@ xenStoreDomainGetOSType(virDomainPtr dom
  * Return the port number on which the domain is listening for VNC
  * connections.
  *
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ *
  * Returns the port number, -1 in case of error
  */
 int             xenStoreDomainGetVNCPort(virConnectPtr conn, int domid) {
@@ -801,6 +826,9 @@ int             xenStoreDomainGetVNCPort
  * Returns the path to the serial console. It is the callers
  * responsibilty to free() the return string. Returns NULL
  * on error
+ *
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
  */
 char *          xenStoreDomainGetConsolePath(virConnectPtr conn, int domid) {
   return virDomainDoStoreQuery(conn, domid, "console/tty");
@@ -814,6 +842,9 @@ char *          xenStoreDomainGetConsole
  *
  * Get the type of domain operation system.
  *
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ *
  * Returns the new string or NULL in case of error, the string must be
  *         freed by the caller.
  */
@@ -858,6 +889,9 @@ xenStoreDomainGetOSTypeID(virConnectPtr 
  * Get the reference (i.e. the string number) for the device on that domain
  * which uses the given mac address
  *
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ *
  * Returns the new string or NULL in case of error, the string must be
  *         freed by the caller.
  */
@@ -910,6 +944,9 @@ xenStoreDomainGetNetworkID(virConnectPtr
  * Get the reference (i.e. the string number) for the device on that domain
  * which uses the given virtual block device name
  *
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ *
  * Returns the new string or NULL in case of error, the string must be
  *         freed by the caller.
  */
@@ -973,6 +1010,10 @@ xenStoreDomainGetDiskID(virConnectPtr co
     return (NULL);
 }
 
+/*
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ */
 char *xenStoreDomainGetName(virConnectPtr conn,
                             int id) {
     char prop[200];
@@ -989,6 +1030,10 @@ char *xenStoreDomainGetName(virConnectPt
 }
 
 #ifndef PROXY
+/*
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ */
 int xenStoreDomainGetUUID(virConnectPtr conn,
                           int id,
                           unsigned char *uuid) {
@@ -1015,9 +1060,9 @@ int xenStoreDomainGetUUID(virConnectPtr 
 
     return ret;
 }
-#endif //PROXY
 
-void xenStoreWatchListFree(xenStoreWatchListPtr list)
+static void
+xenStoreWatchListFree(xenStoreWatchListPtr list)
 {
     int i;
     for (i=0; i<list->count; i++) {
@@ -1028,6 +1073,10 @@ void xenStoreWatchListFree(xenStoreWatch
     VIR_FREE(list);
 }
 
+/*
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ */
 int xenStoreAddWatch(virConnectPtr conn,
                      const char *path,
                      const char *token,
@@ -1080,6 +1129,10 @@ int xenStoreAddWatch(virConnectPtr conn,
     return xs_watch(priv->xshandle, watch->path, watch->token);
 }
 
+/*
+ * The caller must hold the lock on the privateData
+ * associated with the 'conn' parameter.
+ */
 int xenStoreRemoveWatch(virConnectPtr conn,
                         const char *path,
                         const char *token)
@@ -1122,18 +1175,17 @@ int xenStoreRemoveWatch(virConnectPtr co
                 ; /* Failure to reduce memory allocation isn't fatal */
             }
             list->count--;
-#ifndef PROXY
             virUnrefConnect(conn);
-#endif
             return 0;
         }
     }
     return -1;
 }
 
-xenStoreWatchPtr xenStoreFindWatch(xenStoreWatchListPtr list,
-                                   const char *path,
-                                   const char *token)
+static xenStoreWatchPtr
+xenStoreFindWatch(xenStoreWatchListPtr list,
+                  const char *path,
+                  const char *token)
 {
     int i;
     for (i = 0 ; i < list->count ; i++)
@@ -1144,10 +1196,11 @@ xenStoreWatchPtr xenStoreFindWatch(xenSt
     return NULL;
 }
 
-void xenStoreWatchEvent(int watch ATTRIBUTE_UNUSED,
-                        int fd ATTRIBUTE_UNUSED,
-                        int events ATTRIBUTE_UNUSED,
-                        void *data)
+static void
+xenStoreWatchEvent(int watch ATTRIBUTE_UNUSED,
+                   int fd ATTRIBUTE_UNUSED,
+                   int events ATTRIBUTE_UNUSED,
+                   void *data)
 {
     char		 **event;
     char		 *path;
@@ -1158,11 +1211,15 @@ void xenStoreWatchEvent(int watch ATTRIB
     virConnectPtr        conn = data;
     xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
     if(!priv) return;
-    if(!priv->xshandle) return;
+
+    xenUnifiedLock(priv);
+
+    if(!priv->xshandle)
+        goto cleanup;
 
     event = xs_read_watch(priv->xshandle, &stringCount);
     if (!event)
-        return;
+        goto cleanup;
 
     path  = event[XS_WATCH_PATH];
     token = event[XS_WATCH_TOKEN];
@@ -1171,10 +1228,17 @@ void xenStoreWatchEvent(int watch ATTRIB
     if( sw )
         sw->cb(conn, path, token, sw->opaque);
     VIR_FREE(event);
+
+cleanup:
+    xenUnifiedUnlock(priv);
 }
 
-#ifndef PROXY
-/* The domain callback for the @introduceDomain watch */
+
+/*
+ * The domain callback for the @introduceDomain watch
+ *
+ * The lock on 'priv' is held when calling this
+ */
 int xenStoreDomainIntroduced(virConnectPtr conn,
                              const char *path ATTRIBUTE_UNUSED,
                              const char *token ATTRIBUTE_UNUSED,
@@ -1249,7 +1313,11 @@ retry:
     return 0;
 }
 
-/* The domain callback for the @destroyDomain watch */
+/*
+ * The domain callback for the @destroyDomain watch
+ *
+ * The lock on 'priv' is held when calling this
+ */
 int xenStoreDomainReleased(virConnectPtr conn,
                             const char *path  ATTRIBUTE_UNUSED,
                             const char *token ATTRIBUTE_UNUSED,
diff --git a/src/xs_internal.h b/src/xs_internal.h
--- a/src/xs_internal.h
+++ b/src/xs_internal.h
@@ -78,8 +78,6 @@ typedef struct _xenStoreWatchList xenSto
 typedef xenStoreWatchList *xenStoreWatchListPtr;
 
 
-void            xenStoreWatchListFree(xenStoreWatchListPtr head);
-
 int             xenStoreAddWatch(virConnectPtr conn,
                                  const char *path,
                                  const char *token,
@@ -88,11 +86,6 @@ int             xenStoreAddWatch(virConn
 int             xenStoreRemoveWatch(virConnectPtr conn,
                                     const char *path,
                                     const char *token);
-xenStoreWatchPtr xenStoreFindWatch(xenStoreWatchListPtr list,
-                                  const char *path,
-                                  const char *token);
-
-void xenStoreWatchEvent(int watch, int fd, int events, void *data);
 
 /* domain events */
 int xenStoreDomainIntroduced(virConnectPtr conn,
diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c
--- a/tests/sexpr2xmltest.c
+++ b/tests/sexpr2xmltest.c
@@ -5,7 +5,9 @@
 #include <unistd.h>
 
 #include "internal.h"
+#include "datatypes.h"
 #include "xml.h"
+#include "xen_unified.h"
 #include "xend_internal.h"
 #include "testutils.h"
 
@@ -23,6 +25,11 @@ static int testCompareFiles(const char *
   char *sexprPtr = &(sexprData[0]);
   int ret = -1;
   virDomainDefPtr def = NULL;
+  virConnectPtr conn;
+  struct _xenUnifiedPrivate priv;
+
+  conn = virGetConnect();
+  if (!conn) goto fail;
 
   if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0)
       goto fail;
@@ -30,7 +37,15 @@ static int testCompareFiles(const char *
   if (virtTestLoadFile(sexpr, &sexprPtr, MAX_FILE) < 0)
       goto fail;
 
-  if (!(def = xenDaemonParseSxprString(NULL, sexprData, xendConfigVersion)))
+  memset(&priv, 0, sizeof(priv));
+  /* Many puppies died to bring you this code. */
+  priv.xendConfigVersion = xendConfigVersion;
+  conn->privateData = &priv;
+
+  if (virMutexInit(&priv.lock) < 0)
+      goto fail;
+
+  if (!(def = xenDaemonParseSxprString(conn, sexprData, xendConfigVersion)))
       goto fail;
 
   if (!(gotxml = virDomainDefFormat(NULL, def, 0)))
@@ -46,6 +61,8 @@ static int testCompareFiles(const char *
  fail:
   free(gotxml);
   virDomainDefFree(def);
+  virUnrefConnect(conn);
+  virMutexDestroy(&priv.lock);
 
   return ret;
 }

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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