From: Daniel P. Berrange Date: Wed, 21 Jan 2009 18:11:14 +0000 (+0000) Subject: Make xen driver threadsafe X-Git-Tag: LIBVIRT_0_6_0~67 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e52d74e53614cd4f1b26a30b0c5e0cef4c082968;p=thirdparty%2Flibvirt.git Make xen driver threadsafe --- diff --git a/ChangeLog b/ChangeLog index bd13c5a56d..a717111c7e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +Wed Jan 21 18:10:12 GMT 2009 Daniel P. Berrange + + 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 If you un-install libvirt and re-install it, you get a warning diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 5909220a8b..4d8be3ae2d 100644 --- 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 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)) { diff --git a/src/xen_inotify.c b/src/xen_inotify.c index 2043c4e7d0..b3677c8596 100644 --- a/src/xen_inotify.c +++ b/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; } diff --git a/src/xen_internal.c b/src/xen_internal.c index c38cb27a75..3da5865d9d 100644 --- a/src/xen_internal.c +++ b/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) { diff --git a/src/xen_unified.c b/src/xen_unified.c index 201842d801..91718b7cd8 100644 --- a/src/xen_unified.c +++ b/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); } diff --git a/src/xen_unified.h b/src/xen_unified.h index e949156fac..569c7babc8 100644 --- a/src/xen_unified.h +++ b/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__ */ diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bf7658dbf..e24b1a6d92 100644 --- a/src/xend_internal.c +++ b/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; diff --git a/src/xm_internal.c b/src/xm_internal.c index b944b979cf..f7bd774206 100644 --- a/src/xm_internal.c +++ b/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); } diff --git a/src/xs_internal.c b/src/xs_internal.c index 14cb3078eb..3c6e8aa9a3 100644 --- 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 */ @@ -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; icount; 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, diff --git a/src/xs_internal.h b/src/xs_internal.h index f4b46d0ccf..29e680fb07 100644 --- a/src/xs_internal.h +++ b/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,