diff --git a/ChangeLog b/ChangeLog index c6b7696f21..d19c01e8c6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +Thu Jan 29 23:01:22 GMT 2009 Daniel P. Berrange + + Misc Xen driver crash/bug fixes + * src/remote_internal.c: Re-factor startup of secondary driver + activation to fix missing initialization & crash. Fix memory + leak in error reporting + * src/xen_unified.c: Don't activate inotify driver if non-root + * src/xend_internal.c: Don't report errors when probing for + XenD TCP port if unprivileged, allow caller to do it. Fix bad + return values in open method + * src/xs_internal.c: Fix double free + Thu Jan 29 17:22:53 GMT 2009 John Levon * src/xend_internal.c: Fix xend XML generation when CPU pinning diff --git a/src/remote_internal.c b/src/remote_internal.c index 06f8a7f09f..f8740af686 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -892,6 +892,57 @@ doRemoteOpen (virConnectPtr conn, goto cleanup; } +static struct private_data * +remoteAllocPrivateData(virConnectPtr conn) +{ + struct private_data *priv; + if (VIR_ALLOC(priv) < 0) { + virReportOOMError(conn); + return NULL; + } + + if (virMutexInit(&priv->lock) < 0) { + error(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot initialize mutex")); + VIR_FREE(priv); + return NULL; + } + remoteDriverLock(priv); + priv->localUses = 1; + priv->watch = -1; + priv->sock = -1; + + return priv; +} + +static int +remoteOpenSecondaryDriver(virConnectPtr conn, + virConnectAuthPtr auth, + int flags, + struct private_data **priv) +{ + int ret; + int rflags = 0; + + if (!((*priv) = remoteAllocPrivateData(conn))) + return VIR_DRV_OPEN_ERROR; + + if (flags & VIR_CONNECT_RO) + rflags |= VIR_DRV_OPEN_REMOTE_RO; + rflags |= VIR_DRV_OPEN_REMOTE_UNIX; + + ret = doRemoteOpen(conn, *priv, auth, rflags); + if (ret != VIR_DRV_OPEN_SUCCESS) { + remoteDriverUnlock(*priv); + VIR_FREE(*priv); + } else { + (*priv)->localUses = 1; + remoteDriverUnlock(*priv); + } + + return ret; +} + static virDrvOpenStatus remoteOpen (virConnectPtr conn, virConnectAuthPtr auth, @@ -903,20 +954,8 @@ remoteOpen (virConnectPtr conn, if (inside_daemon) return VIR_DRV_OPEN_DECLINED; - if (VIR_ALLOC(priv) < 0) { - virReportOOMError (conn); + if (!(priv = remoteAllocPrivateData(conn))) return VIR_DRV_OPEN_ERROR; - } - - if (virMutexInit(&priv->lock) < 0) { - error(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot initialize mutex")); - VIR_FREE(priv); - return VIR_DRV_OPEN_ERROR; - } - remoteDriverLock(priv); - priv->localUses = 1; - priv->watch = -1; if (flags & VIR_CONNECT_RO) rflags |= VIR_DRV_OPEN_REMOTE_RO; @@ -971,7 +1010,6 @@ remoteOpen (virConnectPtr conn, #endif } - priv->sock = -1; ret = doRemoteOpen(conn, priv, auth, rflags); if (ret != VIR_DRV_OPEN_SUCCESS) { conn->privateData = NULL; @@ -3085,30 +3123,13 @@ remoteNetworkOpen (virConnectPtr conn, * which doesn't have its own impl of the network APIs. */ struct private_data *priv; - int ret, rflags = 0; - if (VIR_ALLOC(priv) < 0) { - virReportOOMError (conn); - return VIR_DRV_OPEN_ERROR; - } - if (virMutexInit(&priv->lock) < 0) { - error(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot initialize mutex")); - VIR_FREE(priv); - return VIR_DRV_OPEN_ERROR; - } - if (flags & VIR_CONNECT_RO) - rflags |= VIR_DRV_OPEN_REMOTE_RO; - rflags |= VIR_DRV_OPEN_REMOTE_UNIX; - - priv->sock = -1; - ret = doRemoteOpen(conn, priv, auth, rflags); - if (ret != VIR_DRV_OPEN_SUCCESS) { - conn->networkPrivateData = NULL; - VIR_FREE(priv); - } else { - priv->localUses = 1; + int ret; + ret = remoteOpenSecondaryDriver(conn, + auth, + flags, + &priv); + if (ret == VIR_DRV_OPEN_SUCCESS) conn->networkPrivateData = priv; - } return ret; } } @@ -3598,30 +3619,13 @@ remoteStorageOpen (virConnectPtr conn, * which doesn't have its own impl of the network APIs. */ struct private_data *priv; - int ret, rflags = 0; - if (VIR_ALLOC(priv) < 0) { - virReportOOMError (NULL); - return VIR_DRV_OPEN_ERROR; - } - if (virMutexInit(&priv->lock) < 0) { - error(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot initialize mutex")); - VIR_FREE(priv); - return VIR_DRV_OPEN_ERROR; - } - if (flags & VIR_CONNECT_RO) - rflags |= VIR_DRV_OPEN_REMOTE_RO; - rflags |= VIR_DRV_OPEN_REMOTE_UNIX; - - priv->sock = -1; - ret = doRemoteOpen(conn, priv, auth, rflags); - if (ret != VIR_DRV_OPEN_SUCCESS) { - conn->storagePrivateData = NULL; - VIR_FREE(priv); - } else { - priv->localUses = 1; + int ret; + ret = remoteOpenSecondaryDriver(conn, + auth, + flags, + &priv); + if (ret == VIR_DRV_OPEN_SUCCESS) conn->storagePrivateData = priv; - } return ret; } } @@ -4551,30 +4555,13 @@ remoteDevMonOpen(virConnectPtr conn, * which doesn't have its own impl of the network APIs. */ struct private_data *priv; - int ret, rflags = 0; - if (VIR_ALLOC(priv) < 0) { - virReportOOMError (NULL); - return VIR_DRV_OPEN_ERROR; - } - if (virMutexInit(&priv->lock) < 0) { - error(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot initialize mutex")); - VIR_FREE(priv); - return VIR_DRV_OPEN_ERROR; - } - if (flags & VIR_CONNECT_RO) - rflags |= VIR_DRV_OPEN_REMOTE_RO; - rflags |= VIR_DRV_OPEN_REMOTE_UNIX; - - priv->sock = -1; - ret = doRemoteOpen(conn, priv, auth, rflags); - if (ret != VIR_DRV_OPEN_SUCCESS) { - conn->devMonPrivateData = NULL; - VIR_FREE(priv); - } else { - priv->localUses = 1; + int ret; + ret = remoteOpenSecondaryDriver(conn, + auth, + flags, + &priv); + if (ret == VIR_DRV_OPEN_SUCCESS) conn->devMonPrivateData = priv; - } return ret; } } @@ -6429,6 +6416,7 @@ cleanup: thiscall->err.domain == VIR_FROM_REMOTE && thiscall->err.code == VIR_ERR_RPC && thiscall->err.level == VIR_ERR_ERROR && + thiscall->err.message && STRPREFIX(*thiscall->err.message, "unknown procedure")) { rv = -2; } else { @@ -6436,6 +6424,7 @@ cleanup: &thiscall->err); rv = -1; } + xdr_free((xdrproc_t)xdr_remote_error, (char *)&thiscall->err); } else { rv = 0; } diff --git a/src/xen_unified.c b/src/xen_unified.c index e70c8acd78..eefdb6c15c 100644 --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -355,11 +355,13 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, int flags) } #if WITH_XEN_INOTIFY - DEBUG0("Trying Xen inotify sub-driver"); - if (drivers[XEN_UNIFIED_INOTIFY_OFFSET]->open(conn, auth, flags) == - VIR_DRV_OPEN_SUCCESS) { - DEBUG0("Activated Xen inotify sub-driver"); - priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1; + if (xenHavePrivilege()) { + DEBUG0("Trying Xen inotify sub-driver"); + if (drivers[XEN_UNIFIED_INOTIFY_OFFSET]->open(conn, auth, flags) == + VIR_DRV_OPEN_SUCCESS) { + DEBUG0("Activated Xen inotify sub-driver"); + priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1; + } } #endif diff --git a/src/xend_internal.c b/src/xend_internal.c index a47214663a..aca22d25a6 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -840,9 +840,11 @@ xenDaemonOpen_tcp(virConnectPtr conn, const char *host, const char *port) freeaddrinfo (res); if (!priv->addrlen) { - virReportSystemError(conn, saved_errno, - _("unable to connect to '%s:%s'"), - host, port); + /* Don't raise error when unprivileged, since proxy takes over */ + if (xenHavePrivilege()) + virReportSystemError(conn, saved_errno, + _("unable to connect to '%s:%s'"), + host, port); return -1; } @@ -2733,7 +2735,6 @@ error: * @flags: combination of virDrvOpenFlag(s) * * Creates a localhost Xen Daemon connection - * Note: this doesn't try to check if the connection actually works * * Returns 0 in case of success, -1 in case of error. */ @@ -2742,7 +2743,8 @@ xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - int ret; + char *port = NULL; + int ret = VIR_DRV_OPEN_ERROR; /* Switch on the scheme, which we expect to be NULL (file), * "http" or "xen". @@ -2753,45 +2755,30 @@ xenDaemonOpen(virConnectPtr conn, virXendError(NULL, VIR_ERR_NO_CONNECT, __FUNCTION__); goto failed; } - ret = xenDaemonOpen_unix(conn, conn->uri->path); - if (ret < 0) - goto failed; - - ret = xend_detect_config_version(conn); - if (ret == -1) + if (xenDaemonOpen_unix(conn, conn->uri->path) < 0 || + xend_detect_config_version(conn) == -1) goto failed; } else if (STRCASEEQ (conn->uri->scheme, "xen")) { /* * try first to open the unix socket */ - ret = xenDaemonOpen_unix(conn, "/var/lib/xend/xend-socket"); - if (ret < 0) - goto try_http; - ret = xend_detect_config_version(conn); - if (ret != -1) + if (xenDaemonOpen_unix(conn, "/var/lib/xend/xend-socket") == 0 && + xend_detect_config_version(conn) != -1) goto done; - try_http: /* * try though http on port 8000 */ - ret = xenDaemonOpen_tcp(conn, "localhost", "8000"); - if (ret < 0) - goto failed; - ret = xend_detect_config_version(conn); - if (ret == -1) + if (xenDaemonOpen_tcp(conn, "localhost", "8000") < 0 || + xend_detect_config_version(conn) == -1) goto failed; } else if (STRCASEEQ (conn->uri->scheme, "http")) { - char *port; if (virAsprintf(&port, "%d", conn->uri->port) == -1) goto failed; - ret = xenDaemonOpen_tcp(conn, conn->uri->server, port); - VIR_FREE(port); - if (ret < 0) - goto failed; - ret = xend_detect_config_version(conn); - if (ret == -1) + + if (xenDaemonOpen_tcp(conn, conn->uri->server, port) < 0 || + xend_detect_config_version(conn) == -1) goto failed; } else { virXendError(NULL, VIR_ERR_NO_CONNECT, __FUNCTION__); @@ -2799,10 +2786,11 @@ xenDaemonOpen(virConnectPtr conn, } done: - return(ret); + ret = VIR_DRV_OPEN_SUCCESS; failed: - return(-1); + VIR_FREE(port); + return ret; } diff --git a/src/xs_internal.c b/src/xs_internal.c index c7087ed574..9e6b80f08a 100644 --- a/src/xs_internal.c +++ b/src/xs_internal.c @@ -599,8 +599,6 @@ xenStoreDoListDomains(xenUnifiedPrivatePtr priv, int *ids, int maxids) ids[ret++] = (int) id; } - free(idlist); - out: VIR_FREE (idlist); return ret;