Make xen driver threadsafe

ChangeLog

+Wed Jan 21 18:10:12 GMT 2009 Daniel P. Berrange <berrange@redhat.com>
+
+	Make Xen driver threadsafe
+	* src/proxy_internal.c, src/xen_inotify.c, src/xen_internal.c,
+	src/xen_unified.c, src/xen_unified.h, src/xend_internal.c,
+	src/xm_internal.c, src/xs_internal.c, src/xs_internal.h: Add
+	mutex locking of shared state
+
 Wed Jan 21 10:48:12 IST 2009 Mark McLoughlin <markmc@redhat.com>
 
 	If you un-install libvirt and re-install it, you get a warning

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 char *data, int len) {
 
     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, virProxyPacketPtr request,
         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, virProxyPacketPtr request,
     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, unsigned long *hvVer)
     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, int *ids, int maxids)
     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(virConnectPtr conn, int id)
     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 domain, virDomainInfoPtr info)
     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, int id)
     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, const unsigned char *uuid)
 
     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, const char *name)
     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, virNodeInfoPtr info) {
     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 conn)
     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 domain, int flags ATTRIBUTE_UNUSED)
     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 domain)
     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)) {

src/xen_inotify.c

@@ -298,25 +298,27 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUSED,
         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;
@@ -340,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,
@@ -363,6 +365,9 @@ reread:
         }
 
     }
+
+cleanup:
+    xenUnifiedUnlock(priv);
 }
 
 /**
@@ -458,7 +463,7 @@ xenInotifyOpen(virConnectPtr conn ATTRIBUTE_UNUSED,
         DEBUG0("Failed to add inotify handle, disabling events");
     }
 
-    conn->refs++;
+    virConnectRef(conn);
     return 0;
 }
 

src/xen_internal.c

@@ -1337,9 +1337,14 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
 {
 #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",
@@ -2756,8 +2761,10 @@ xenHypervisorLookupDomainByID(virConnectPtr conn,
     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));
@@ -2824,7 +2831,10 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn,
     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);
@@ -3057,7 +3067,6 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
     xen_op_v2_sys op_sys;
     int i, j, ret;
     xenUnifiedPrivatePtr priv;
-    int nbNodeCells;
 
     if (conn == NULL) {
         virXenErrorFunc (conn, VIR_ERR_INVALID_ARG, __FUNCTION__,
@@ -3065,14 +3074,15 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
         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;
@@ -3087,7 +3097,6 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
         return -1;
     }
 
-    priv = (xenUnifiedPrivatePtr) conn->privateData;
     if (priv->handle < 0) {
         virXenErrorFunc (conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__,
                         "priv->handle invalid", 0);
@@ -3097,7 +3106,7 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
     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) {

src/xen_unified.c

@@ -56,7 +56,7 @@ xenUnifiedDomainGetVcpus (virDomainPtr dom,
                           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[XEN_UNIFIED_NR_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,43 +82,20 @@ 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;
-}
 
-/**
- * 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);
-}
+    priv = conn->privateData;
 
-/**
- * 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);
+    priv->nbNodeCells = nodeInfo.nodes;
+    priv->nbNodeCpus = nodeInfo.cpus;
 }
 
+
 /**
  * xenDomainUsedCpus:
  * @dom: the domain
@@ -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, virConnectAuthPtr auth, int flags)
         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, virConnectAuthPtr auth, int flags)
         }
     }
 
+    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 conn, virNodeInfoPtr info)
 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 dom, int flags)
     } 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);
@@ -1362,11 +1347,14 @@ xenUnifiedDomainEventRegister (virConnectPtr conn,
                                void *opaque,
                                void (*freefunc)(void *))
 {
+    GET_PRIVATE (conn);
+
     int ret;
+    xenUnifiedLock(priv);
 
-    GET_PRIVATE (conn);
     if (priv->xsWatch == -1) {
         xenUnifiedError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+        xenUnifiedUnlock(priv);
         return -1;
     }
 
@@ -1376,6 +1364,7 @@ xenUnifiedDomainEventRegister (virConnectPtr conn,
     if (ret == 0)
         conn->refs++;
 
+    xenUnifiedUnlock(priv);
     return (ret);
 }
 
@@ -1385,16 +1374,25 @@ xenUnifiedDomainEventDeregister (virConnectPtr conn,
 {
     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);
 
     if (ret == 0)
         virUnrefConnect(conn);
+
+    xenUnifiedUnlock(priv);
     return ret;
 }
 
@@ -1585,21 +1583,66 @@ xenUnifiedRemoveDomainInfo(xenUnifiedDomainInfoListPtr list,
     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);
 }

src/xen_unified.h

@@ -131,6 +131,12 @@ typedef xenUnifiedDomainInfoList *xenUnifiedDomainInfoListPtr;
  * 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 (xenUnifiedPrivatePtr priv,
                                     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__ */

src/xend_internal.c

@@ -1941,18 +1941,25 @@ xenDaemonParseSxprGraphicsOld(virConnectPtr conn,
                               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;
 
@@ -2017,6 +2024,9 @@ xenDaemonParseSxprGraphicsNew(virConnectPtr conn,
                               virDomainDefPtr def,
                               const struct sexpr *root)
 {
+#ifndef PROXY
+    xenUnifiedPrivatePtr priv = conn->privateData;
+#endif
     virDomainGraphicsDefPtr graphics = NULL;
     const struct sexpr *cur, *node;
     const char *tmp;
@@ -2048,16 +2058,21 @@ xenDaemonParseSxprGraphicsNew(virConnectPtr conn,
                     !(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;
@@ -2114,6 +2129,9 @@ xenDaemonParseSxpr(virConnectPtr conn,
                    int xendConfigVersion,
                    const char *cpus)
 {
+#ifndef PROXY
+    xenUnifiedPrivatePtr priv = conn->privateData;
+#endif
     const char *tmp;
     virDomainDefPtr def;
     int hvm = 0;
@@ -2333,7 +2351,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")) {
@@ -5442,14 +5462,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;
 
@@ -5465,8 +5488,10 @@ virDomainXMLDevID(virDomainPtr domain,
 
         strcpy(class, "vif");
 
+        xenUnifiedLock(priv);
         xref = xenStoreDomainGetNetworkID(domain->conn, domain->id,
                                           mac);
+        xenUnifiedUnlock(priv);
         if (xref == NULL)
             return -1;
 

src/xm_internal.c

@@ -347,6 +347,11 @@ xenXMConfigSaveFile(virConnectPtr conn, const char *filename, virDomainDefPtr de
     return ret;
 }
 
+
+/*
+ * Caller must hold the lock on 'conn->privateData' before
+ * calling this funtion
+ */
 int
 xenXMConfigCacheRemoveFile(virConnectPtr conn,
                            const char *filename)
@@ -367,6 +372,10 @@ xenXMConfigCacheRemoveFile(virConnectPtr conn,
 }
 
 
+/*
+ * Caller must hold the lock on 'conn->privateData' before
+ * calling this funtion
+ */
 int
 xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
 {
@@ -460,10 +469,14 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
 }
 
 /* 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;
@@ -621,12 +634,13 @@ int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) {
         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;
@@ -635,8 +649,12 @@ int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) {
     info->state = VIR_DOMAIN_SHUTOFF;
     info->cpuTime = 0;
 
+    xenUnifiedUnlock(priv);
     return (0);
 
+error:
+    xenUnifiedUnlock(priv);
+    return -1;
 }
 
 #define MAX_VFB 1024
@@ -1293,6 +1311,7 @@ char *xenXMDomainDumpXML(virDomainPtr domain, int flags) {
     xenUnifiedPrivatePtr priv;
     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,
@@ -1303,14 +1322,19 @@ char *xenXMDomainDumpXML(virDomainPtr domain, int flags) {
         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;
+
+    ret = virDomainDefFormat(domain->conn, entry->def, flags);
 
-    return virDomainDefFormat(domain->conn, entry->def, flags);
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 
@@ -1321,6 +1345,7 @@ int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) {
     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,
@@ -1335,12 +1360,13 @@ int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) {
         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)
@@ -1350,9 +1376,12 @@ int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) {
      * 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;
 }
 
 /*
@@ -1362,6 +1391,7 @@ int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) {
     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,
@@ -1374,12 +1404,13 @@ int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) {
         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)
@@ -1389,9 +1420,12 @@ int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) {
      * 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;
 }
 
 /*
@@ -1401,24 +1435,30 @@ unsigned long xenXMDomainGetMaxMemory(virDomainPtr domain) {
     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;
+
+    ret = entry->def->maxmem;
 
-    return entry->def->maxmem;
+cleanup:
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /*
@@ -1428,6 +1468,7 @@ int xenXMDomainSetVcpus(virDomainPtr domain, unsigned int vcpus) {
     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,
@@ -1440,12 +1481,13 @@ int xenXMDomainSetVcpus(virDomainPtr domain, unsigned int vcpus) {
         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;
 
@@ -1453,9 +1495,12 @@ int xenXMDomainSetVcpus(virDomainPtr domain, unsigned int vcpus) {
      * 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;
 }
 
 /**
@@ -1501,15 +1546,16 @@ int xenXMDomainPinVcpu(virDomainPtr domain,
     }
 
     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 */
@@ -1527,7 +1573,7 @@ int xenXMDomainPinVcpu(virDomainPtr domain,
 
     if (virBufferError(&mapbuf)) {
         virReportOOMError(domain->conn);
-        return -1;
+        goto cleanup;
     }
 
     mapstr = virBufferContentAndReset(&mapbuf);
@@ -1554,6 +1600,7 @@ int xenXMDomainPinVcpu(virDomainPtr domain,
  cleanup:
     VIR_FREE(mapstr);
     VIR_FREE(cpuset);
+    xenUnifiedUnlock(priv);
     return (ret);
 }
 
@@ -1564,7 +1611,7 @@ virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname) {
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
-    virDomainPtr ret;
+    virDomainPtr ret = NULL;
 
     if (!VIR_IS_CONNECT(conn)) {
         xenXMError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -1576,27 +1623,28 @@ virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname) {
     }
 
     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);
 }
 
@@ -1621,7 +1669,7 @@ virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn,
                                      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__);
@@ -1633,24 +1681,25 @@ virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn,
     }
 
     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);
 }
 
@@ -1660,7 +1709,7 @@ virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn,
  */
 int xenXMDomainCreate(virDomainPtr domain) {
     char *sexpr;
-    int ret;
+    int ret = -1;
     xenUnifiedPrivatePtr priv;
     const char *filename;
     xenXMConfCachePtr entry;
@@ -1670,43 +1719,45 @@ int xenXMDomainCreate(virDomainPtr domain) {
     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);
 }
 
@@ -2290,14 +2341,20 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
     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))) {
+        xenUnifiedLock(priv);
         return (NULL);
+    }
 
     if (virHashLookup(priv->nameConfigMap, def->name)) {
         /* domain exists, we will overwrite it */
@@ -2375,16 +2432,16 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
         goto error;
     }
 
-    if (!(ret = virGetDomain(conn, def->name, def->uuid)))
-        return NULL;
-
-    ret->id = -1;
+    if ((ret = virGetDomain(conn, def->name, def->uuid)))
+        ret->id = -1;
 
+    xenUnifiedUnlock(priv);
     return (ret);
 
  error:
     VIR_FREE(entry);
     virDomainDefFree(def);
+    xenUnifiedUnlock(priv);
     return (NULL);
 }
 
@@ -2395,6 +2452,8 @@ int xenXMDomainUndefine(virDomainPtr domain) {
     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__);
@@ -2407,25 +2466,30 @@ int xenXMDomainUndefine(virDomainPtr domain) {
         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 {
@@ -2459,6 +2523,7 @@ static void xenXMListIterator(const void *payload ATTRIBUTE_UNUSED, const char *
 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__);
@@ -2466,10 +2531,11 @@ int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames
     }
 
     priv = conn->privateData;
+    xenUnifiedLock(priv);
 
 #ifndef WITH_XEN_INOTIFY
     if (xenXMConfigCacheRefresh (conn) < 0)
-        return (-1);
+        goto cleanup;
 #endif
 
     if (maxnames > virHashSize(priv->configCache))
@@ -2481,7 +2547,13 @@ int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames
     ctx.names = names;
 
     virHashForEach(priv->nameConfigMap, xenXMListIterator, &ctx);
-    return (ctx.count);
+    ret = ctx.count;
+
+#ifndef WITH_XEN_INOTIFY
+cleanup:
+#endif
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /*
@@ -2490,6 +2562,7 @@ int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames
  */
 int xenXMNumOfDefinedDomains(virConnectPtr conn) {
     xenUnifiedPrivatePtr priv;
+    int ret = -1;
 
     if (!VIR_IS_CONNECT(conn)) {
         xenXMError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
@@ -2497,13 +2570,20 @@ int xenXMNumOfDefinedDomains(virConnectPtr conn) {
     }
 
     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;
 }
 
 
@@ -2532,24 +2612,25 @@ xenXMDomainAttachDevice(virDomainPtr domain, const char *xml) {
         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:
@@ -2592,7 +2673,7 @@ xenXMDomainAttachDevice(virDomainPtr domain, const char *xml) {
 
  cleanup:
     virDomainDeviceDefFree(dev);
-
+    xenUnifiedUnlock(priv);
     return ret;
 }
 
@@ -2622,23 +2703,26 @@ xenXMDomainDetachDevice(virDomainPtr domain, const char *xml) {
         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:
@@ -2690,6 +2774,7 @@ xenXMDomainDetachDevice(virDomainPtr domain, const char *xml) {
 
  cleanup:
     virDomainDeviceDefFree(dev);
+    xenUnifiedUnlock(priv);
     return (ret);
 }
 

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 */
@@ -386,7 +381,6 @@ xenStoreClose(virConnectPtr conn)
 
     xenStoreWatchListFree(priv->xsWatchList);
     priv->xsWatchList = NULL;
-#ifndef PROXY
     xenUnifiedDomainInfoListFree(priv->activeDomainList);
     priv->activeDomainList = NULL;
 #endif
@@ -518,17 +512,21 @@ xenStoreDomainGetMaxMemory(virDomainPtr domain)
 {
     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);
 }
 
@@ -591,12 +589,14 @@ xenStoreListDomains(virConnectPtr conn, int *ids, int maxids)
     }
 
     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);
@@ -611,7 +611,12 @@ xenStoreListDomains(virConnectPtr conn, int *ids, int maxids)
         ids[ret++] = (int) id;
     }
     free(idlist);
+    xenUnifiedUnlock(priv);
     return(ret);
+
+error:
+    xenUnifiedUnlock(priv);
+    return -1;
 }
 
 /**
@@ -696,6 +701,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__);
@@ -707,7 +715,11 @@ xenStoreDomainShutdown(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", "poweroff"));
+    priv = domain->conn->privateData;
+    xenUnifiedLock(priv);
+    ret = virDomainDoStoreWrite(domain, "control/shutdown", "poweroff");
+    xenUnifiedUnlock(priv);
+    return ret;
 }
 
 /**
@@ -724,6 +736,9 @@ xenStoreDomainShutdown(virDomainPtr domain)
 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__);
@@ -735,7 +750,11 @@ xenStoreDomainReboot(virDomainPtr domain, unsigned int flags ATTRIBUTE_UNUSED)
      * 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;
 }
 
 /*
@@ -759,8 +778,11 @@ xenStoreDomainGetOSType(virDomainPtr domain) {
 
     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);
@@ -775,6 +797,9 @@ xenStoreDomainGetOSType(virDomainPtr domain) {
  * 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) {
@@ -803,6 +828,9 @@ int             xenStoreDomainGetVNCPort(virConnectPtr conn, int domid) {
  * 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");
@@ -816,6 +844,9 @@ char *          xenStoreDomainGetConsolePath(virConnectPtr conn, int domid) {
  *
  * 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.
  */
@@ -860,6 +891,9 @@ xenStoreDomainGetOSTypeID(virConnectPtr conn, int id) {
  * 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.
  */
@@ -912,6 +946,9 @@ xenStoreDomainGetNetworkID(virConnectPtr conn, int id, const char *mac) {
  * 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.
  */
@@ -975,6 +1012,10 @@ xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) {
     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];
@@ -991,6 +1032,10 @@ char *xenStoreDomainGetName(virConnectPtr conn,
 }
 
 #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) {
@@ -1017,9 +1062,9 @@ int xenStoreDomainGetUUID(virConnectPtr conn,
 
     return ret;
 }
-#endif //PROXY
 
-void xenStoreWatchListFree(xenStoreWatchListPtr list)
+static void
+xenStoreWatchListFree(xenStoreWatchListPtr list)
 {
     int i;
     for (i=0; i<list->count; i++) {
@@ -1030,6 +1075,10 @@ void xenStoreWatchListFree(xenStoreWatchListPtr list)
     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,
@@ -1082,6 +1131,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)
@@ -1124,18 +1177,17 @@ int xenStoreRemoveWatch(virConnectPtr conn,
                 ; /* 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++)
@@ -1146,10 +1198,11 @@ xenStoreWatchPtr xenStoreFindWatch(xenStoreWatchListPtr list,
     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;
@@ -1160,11 +1213,15 @@ void xenStoreWatchEvent(int watch ATTRIBUTE_UNUSED,
     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];
@@ -1173,10 +1230,17 @@ void xenStoreWatchEvent(int watch ATTRIBUTE_UNUSED,
     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,
@@ -1251,7 +1315,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,

src/xs_internal.h

@@ -78,8 +78,6 @@ typedef struct _xenStoreWatchList xenStoreWatchList;
 typedef xenStoreWatchList *xenStoreWatchListPtr;
 
 
-void            xenStoreWatchListFree(xenStoreWatchListPtr head);
-
 int             xenStoreAddWatch(virConnectPtr conn,
                                  const char *path,
                                  const char *token,
@@ -88,11 +86,6 @@ int             xenStoreAddWatch(virConnectPtr conn,
 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,