On Fri, Jan 16, 2009 at 12:25:02PM +0000, Daniel P. Berrange wrote:
On Fri, Jan 16, 2009 at 12:21:59PM +0000, Richard W.M. Jones wrote:
On Tue, Jan 13, 2009 at 05:46:37PM +0000, Daniel P. Berrange wrote:
+ char buf[1024];
sysconf (_SC_GETPW_R_SIZE_MAX)?
Looking at glibc's implementation of getpwuid (which uses getpwuid_r), I see that glibc dynamically reallocates the buffer as necessary to the correct size for the return value. The logic of this is fairly simple so maybe we should do the same?
From glibc:
buffer = malloc (/*some_initial_size*/);
while (buffer != NULL && (getpwuid_r (const char *name, &resbuf, buffer, buffer_size, &result) == ERANGE)) { char *new_buf; buffer_size *= 2; new_buf = (char *) realloc (buffer, buffer_size); if (new_buf == NULL) { free (buffer); errno = ENOMEM; } buffer = new_buf; }
Anyhow, +1 but I'd be happier if these functions were centralized in somewhere like src/utils.c.
That's a good idea - in all the cases where we currently use getpwuid all we actually want is the home directory path. So we could add a simple func:
char *virUserHomeDirectory(uid_t uid);
and hide all the horrific code in there.
Here's an update with that usage in it diff --git a/configure.in b/configure.in --- a/configure.in +++ b/configure.in @@ -75,7 +75,7 @@ dnl Availability of various common funct AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid]) dnl Availability of various not common threadsafe functions -AC_CHECK_FUNCS([strerror_r]) +AC_CHECK_FUNCS([strerror_r strtok_r getmntent_r getgrnam_r getpwuid_r]) dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/utsname.h sys/wait.h winsock2.h sched.h termios.h sys/poll.h syslog.h]) diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -76,9 +76,6 @@ proxyInitXen(void) { priv->handle = -1; priv->xendConfigVersion = -1; - priv->type = -1; - priv->len = -1; - priv->addr = NULL; priv->xshandle = NULL; priv->proxy = -1; diff --git a/qemud/qemud.c b/qemud/qemud.c --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -688,20 +688,18 @@ static int qemudInitPaths(struct qemud_s if (snprintf(server->logDir, PATH_MAX, "%s/log/libvirt/", LOCAL_STATE_DIR) >= PATH_MAX) goto snprintf_error; } else { - struct passwd *pw; + char *userdir = virGetUserDirectory(NULL, uid); - if (!(pw = getpwuid(uid))) { - VIR_ERROR(_("Failed to find user record for uid '%d': %s"), - uid, strerror(errno)); - return -1; + if (snprintf(sockname, maxlen, "@%s/.libvirt/libvirt-sock", userdir) >= maxlen) { + VIR_FREE(userdir); + goto snprintf_error; } - if (snprintf(sockname, maxlen, "@%s/.libvirt/libvirt-sock", pw->pw_dir) >= maxlen) + if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log", userdir) >= PATH_MAX) { + VIR_FREE(userdir); goto snprintf_error; - - if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log", pw->pw_dir) >= PATH_MAX) - goto snprintf_error; - + } + VIR_FREE(userdir); } /* !remote */ return 0; @@ -2433,8 +2431,9 @@ remoteReadConfigFile (struct qemud_serve if (getuid() != 0) { VIR_WARN0(_("Cannot set group when not running as root")); } else { - struct group *grp = getgrnam(unix_sock_group); - if (!grp) { + char buf[1024]; + struct group grpdata, *grp; + if (getgrnam_r(unix_sock_group, &grpdata, buf, sizeof(buf), &grp) != 0 || !grp) { VIR_ERROR(_("Failed to lookup group '%s'"), unix_sock_group); goto free_and_fail; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -311,6 +311,7 @@ virAsprintf; virRun; virSkipSpaces; virKillProcess; +virGetUserDirectory; # uuid.h diff --git a/src/lxc_container.c b/src/lxc_container.c --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -414,19 +414,20 @@ static int lxcContainerMountNewFS(virDom static int lxcContainerUnmountOldFS(void) { - struct mntent *mntent; + struct mntent mntent; char **mounts = NULL; int nmounts = 0; FILE *procmnt; int i; + char mntbuf[1024]; if (!(procmnt = setmntent("/proc/mounts", "r"))) { virReportSystemError(NULL, errno, "%s", _("failed to read /proc/mounts")); return -1; } - while ((mntent = getmntent(procmnt)) != NULL) { - if (!STRPREFIX(mntent->mnt_dir, "/.oldroot")) + while (getmntent_r(procmnt, &mntent, mntbuf, sizeof(mntbuf)) != NULL) { + if (!STRPREFIX(mntent.mnt_dir, "/.oldroot")) continue; if (VIR_REALLOC_N(mounts, nmounts+1) < 0) { @@ -434,7 +435,7 @@ static int lxcContainerUnmountOldFS(void lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return -1; } - if (!(mounts[nmounts++] = strdup(mntent->mnt_dir))) { + if (!(mounts[nmounts++] = strdup(mntent.mnt_dir))) { endmntent(procmnt); lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); return -1; diff --git a/src/network_driver.c b/src/network_driver.c --- a/src/network_driver.c +++ b/src/network_driver.c @@ -195,7 +195,6 @@ networkAutostartConfigs(struct network_d static int networkStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; int err; @@ -216,19 +215,22 @@ networkStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { - networkLog(NETWORK_ERR, _("Failed to find user record for uid '%d': %s\n"), - uid, strerror(errno)); + char *userdir = virGetUserDirectory(NULL, uid); + + if (!userdir) + goto error; + + if (virAsprintf(&driverState->logDir, + "%s/.libvirt/qemu/log", userdir) == -1) { + VIR_FREE(userdir); goto out_of_memory; } - if (virAsprintf(&driverState->logDir, - "%s/.libvirt/qemu/log", pw->pw_dir) == -1) - goto out_of_memory; - - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) { + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + VIR_FREE(userdir); goto out_of_memory; } + VIR_FREE(userdir); } /* Configuration paths are either ~/.libvirt/qemu/... (session) or diff --git a/src/openvz_driver.c b/src/openvz_driver.c --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -448,11 +448,12 @@ openvzGenerateContainerVethName(int veid if ( (ret = openvzReadConfigParam(veid, "NETIF", temp, sizeof(temp))) <= 0) { snprintf(temp, sizeof(temp), "eth0"); } else { + char *saveptr; char *s; int max = 0; /* get maximum interface number (actually, it is the last one) */ - for (s=strtok(temp, ";"); s; s=strtok(NULL, ";")) { + for (s=strtok_r(temp, ";", &saveptr); s; s=strtok_r(NULL, ";", &saveptr)) { int x; if (sscanf(s, "ifname=eth%d", &x) != 1) return NULL; diff --git a/src/qemu_driver.c b/src/qemu_driver.c --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -382,7 +382,6 @@ next: static int qemudStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; @@ -421,27 +420,30 @@ qemudStartup(void) { "%s/run/libvirt/qemu/", LOCAL_STATE_DIR) == -1) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { - qemudLog(QEMUD_ERR, _("Failed to find user record for uid '%d': %s\n"), - uid, strerror(errno)); - goto error; - } + char *userdir = virGetUserDirectory(NULL, uid); + if (!userdir) + goto error; if (virAsprintf(&qemu_driver->logDir, - "%s/.libvirt/qemu/log", pw->pw_dir) == -1) - goto out_of_memory; - - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) - goto out_of_memory; + "%s/.libvirt/qemu/log", userdir) == -1) { + VIR_FREE(userdir); + goto out_of_memory; + } + + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + VIR_FREE(userdir); + goto out_of_memory; + } + VIR_FREE(userdir); if (virAsprintf(&qemu_driver->stateDir, "%s/qemu/run", base) == -1) goto out_of_memory; } if (virFileMakePath(qemu_driver->stateDir) < 0) { - qemudLog(QEMUD_ERR, _("Failed to create state dir '%s': %s\n"), - qemu_driver->stateDir, strerror(errno)); - goto error; + qemudLog(QEMUD_ERR, _("Failed to create state dir '%s': %s\n"), + qemu_driver->stateDir, strerror(errno)); + goto error; } /* Configuration paths are either ~/.libvirt/qemu/... (session) or diff --git a/src/remote_internal.c b/src/remote_internal.c --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -605,19 +605,16 @@ doRemoteOpen (virConnectPtr conn, case trans_unix: { if (!sockname) { if (flags & VIR_DRV_OPEN_REMOTE_USER) { - struct passwd *pw; - uid_t uid = getuid(); - - if (!(pw = getpwuid(uid))) { - virReportSystemError(conn, errno, - _("unable to lookup user '%d'"), - uid); + char *userdir = virGetUserDirectory(conn, getuid()); + + if (!userdir) goto failed; + + if (virAsprintf(&sockname, "@%s" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0) { + VIR_FREE(userdir); + goto out_of_memory; } - - if (virAsprintf(&sockname, "@%s" LIBVIRTD_USER_UNIX_SOCKET, pw->pw_dir) < 0) - goto out_of_memory; - + VIR_FREE(userdir); } else { if (flags & VIR_DRV_OPEN_REMOTE_RO) sockname = strdup (LIBVIRTD_PRIV_UNIX_SOCKET_RO); diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -385,7 +385,8 @@ static int virStorageBackendFileSystemIsMounted(virConnectPtr conn, virStoragePoolObjPtr pool) { FILE *mtab; - struct mntent *ent; + struct mntent ent; + char buf[1024]; if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) { virReportSystemError(conn, errno, @@ -394,8 +395,8 @@ virStorageBackendFileSystemIsMounted(vir return -1; } - while ((ent = getmntent(mtab)) != NULL) { - if (STREQ(ent->mnt_dir, pool->def->target.path)) { + while ((getmntent_r(mtab, &ent, buf, sizeof(buf))) != NULL) { + if (STREQ(ent.mnt_dir, pool->def->target.path)) { fclose(mtab); return 1; } diff --git a/src/storage_driver.c b/src/storage_driver.c --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -108,7 +108,6 @@ storageDriverAutostart(virStorageDriverS static int storageDriverStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; @@ -125,16 +124,17 @@ storageDriverStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { - if (!(pw = getpwuid(uid))) { - storageLog("Failed to find user record for uid '%d': %s", - uid, strerror(errno)); + char *userdir = virGetUserDirectory(NULL, uid); + + if (!userdir) + goto error; + + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + storageLog("out of memory in virAsprintf"); + VIR_FREE(userdir); goto out_of_memory; } - - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) { - storageLog("out of memory in virAsprintf"); - goto out_of_memory; - } + VIR_FREE(userdir); } /* Configuration paths are either ~/.libvirt/storage/... (session) or diff --git a/src/uml_driver.c b/src/uml_driver.c --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -309,9 +309,9 @@ cleanup: static int umlStartup(void) { uid_t uid = geteuid(); - struct passwd *pw; char *base = NULL; char driverConf[PATH_MAX]; + char *userdir = NULL; if (VIR_ALLOC(uml_driver) < 0) return -1; @@ -325,11 +325,9 @@ umlStartup(void) { /* Don't have a dom0 so start from 1 */ uml_driver->nextvmid = 1; - if (!(pw = getpwuid(uid))) { - umlLog(VIR_LOG_ERROR, _("Failed to find user record for uid '%d': %s\n"), - uid, strerror(errno)); + userdir = virGetUserDirectory(NULL, uid); + if (!userdir) goto error; - } if (!uid) { if (virAsprintf(¨_driver->logDir, @@ -339,16 +337,17 @@ umlStartup(void) { if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL) goto out_of_memory; } else { + if (virAsprintf(¨_driver->logDir, - "%s/.libvirt/uml/log", pw->pw_dir) == -1) + "%s/.libvirt/uml/log", userdir) == -1) goto out_of_memory; - if (virAsprintf(&base, "%s/.libvirt", pw->pw_dir) == -1) + if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) goto out_of_memory; } if (virAsprintf(¨_driver->monitorDir, - "%s/.uml", pw->pw_dir) == -1) + "%s/.uml", userdir) == -1) goto out_of_memory; /* Configuration paths are either ~/.libvirt/uml/... (session) or @@ -403,6 +402,8 @@ umlStartup(void) { umlAutostartConfigs(uml_driver); umlDriverUnlock(uml_driver); + VIR_FREE(userdir); + return 0; out_of_memory: @@ -410,6 +411,7 @@ out_of_memory: "%s", _("umlStartup: out of memory\n")); error: + VIR_FREE(userdir); VIR_FREE(base); umlDriverUnlock(uml_driver); umlShutdown(); diff --git a/src/util.c b/src/util.c --- a/src/util.c +++ b/src/util.c @@ -49,6 +49,9 @@ #include <paths.h> #endif #include <netdb.h> +#ifdef HAVE_GETPWUID_R +#include <pwd.h> +#endif #include "virterror_internal.h" #include "logging.h" @@ -1431,3 +1434,37 @@ int virKillProcess(pid_t pid, int sig) return kill(pid, sig); #endif } + + +#ifdef HAVE_GETPWUID_R +char *virGetUserDirectory(virConnectPtr conn, + uid_t uid) +{ + char *strbuf; + char *ret; + struct passwd pwbuf; + struct passwd *pw; + size_t strbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); + + if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { + virReportOOMError(conn); + return NULL; + } + + if (getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw) != 0) { + virReportSystemError(conn, errno, + _("Failed to find user record for uid '%d'"), + uid); + VIR_FREE(strbuf); + return NULL; + } + + ret = strdup(pw->pw_dir); + + VIR_FREE(strbuf); + if (!ret) + virReportOOMError(conn); + + return NULL; +} +#endif diff --git a/src/util.h b/src/util.h --- a/src/util.h +++ b/src/util.h @@ -172,4 +172,9 @@ char *virGetHostname(void); int virKillProcess(pid_t pid, int sig); +#ifdef HAVE_GETPWUID_R +char *virGetUserDirectory(virConnectPtr conn, + uid_t uid); +#endif + #endif /* __VIR_UTIL_H__ */ diff --git a/src/xen_unified.c b/src/xen_unified.c --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -278,9 +278,6 @@ xenUnifiedOpen (virConnectPtr conn, virC priv->handle = -1; priv->xendConfigVersion = -1; - priv->type = -1; - priv->len = -1; - priv->addr = NULL; priv->xshandle = NULL; priv->proxy = -1; diff --git a/src/xen_unified.h b/src/xen_unified.h --- a/src/xen_unified.h +++ b/src/xen_unified.h @@ -136,13 +136,11 @@ struct _xenUnifiedPrivate { int xendConfigVersion; /* XenD config version */ - /* XXX This code is not IPv6 aware. */ /* connection to xend */ - int type; /* PF_UNIX or PF_INET */ - int len; /* length of addr */ - struct sockaddr *addr; /* type of address used */ - struct sockaddr_un addr_un; /* the unix address */ - struct sockaddr_in addr_in; /* the inet address */ + struct sockaddr_storage addr; + socklen_t addrlen; + int addrfamily; + int addrprotocol; struct xs_handle *xshandle; /* handle to talk to the xenstore */ diff --git a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -69,30 +69,6 @@ #define XEND_CONFIG_MIN_VERS_PVFB_NEWCONF 3 #endif -/** - * xend_connection_type: - * - * The connection to the Xen Daemon can be done either though a normal TCP - * socket or a local domain direct connection. - */ -enum xend_connection_type { - XEND_DOMAIN, - XEND_TCP, -}; - -/** - * xend: - * - * Structure associated to a connection to a Xen daemon - */ -struct xend { - int len; - int type; - struct sockaddr *addr; - struct sockaddr_un addr_un; - struct sockaddr_in addr_in; -}; - #ifndef PROXY static int @@ -140,7 +116,7 @@ do_connect(virConnectPtr xend) int no_slow_start = 1; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) xend->privateData; - s = socket(priv->type, SOCK_STREAM, 0); + s = socket(priv->addrfamily, SOCK_STREAM, priv->addrprotocol); if (s == -1) { virXendError(xend, VIR_ERR_INTERNAL_ERROR, "%s", _("failed to create a socket")); @@ -154,7 +130,7 @@ do_connect(virConnectPtr xend) sizeof(no_slow_start)); - if (connect(s, priv->addr, priv->len) == -1) { + if (connect(s, (struct sockaddr *)&priv->addr, priv->addrlen) == -1) { serrno = errno; close(s); errno = serrno; @@ -767,18 +743,16 @@ xenDaemonOpen_unix(virConnectPtr conn, c if ((conn == NULL) || (path == NULL)) return (-1); - addr = &priv->addr_un; + memset(&priv->addr, 0, sizeof(priv->addr)); + priv->addrfamily = AF_UNIX; + priv->addrprotocol = PF_UNIX; + priv->addrlen = sizeof(struct sockaddr_un); + + addr = (struct sockaddr_un *)&priv->addr; addr->sun_family = AF_UNIX; memset(addr->sun_path, 0, sizeof(addr->sun_path)); strncpy(addr->sun_path, path, sizeof(addr->sun_path)); - priv->len = sizeof(addr->sun_family) + strlen(addr->sun_path); - if ((unsigned int) priv->len > sizeof(addr->sun_path)) - priv->len = sizeof(addr->sun_path); - - priv->addr = (struct sockaddr *) addr; - priv->type = PF_UNIX; - return (0); } @@ -795,38 +769,71 @@ xenDaemonOpen_unix(virConnectPtr conn, c * Returns 0 in case of success, -1 in case of error. */ static int -xenDaemonOpen_tcp(virConnectPtr conn, const char *host, int port) -{ - struct in_addr ip; - struct hostent *pent; - xenUnifiedPrivatePtr priv; - - if ((conn == NULL) || (host == NULL) || (port <= 0)) - return (-1); - - priv = (xenUnifiedPrivatePtr) conn->privateData; - - pent = gethostbyname(host); - if (pent == NULL) { - if (inet_aton(host, &ip) == 0) { - virXendError(NULL, VIR_ERR_UNKNOWN_HOST, - _("gethostbyname failed: %s"), host); - errno = ESRCH; - return (-1); - } - } else { - memcpy(&ip, pent->h_addr_list[0], sizeof(ip)); - } - - priv->len = sizeof(struct sockaddr_in); - priv->addr = (struct sockaddr *) &priv->addr_in; - priv->type = PF_INET; - - priv->addr_in.sin_family = AF_INET; - priv->addr_in.sin_port = htons(port); - memcpy(&priv->addr_in.sin_addr, &ip, sizeof(ip)); - - return (0); +xenDaemonOpen_tcp(virConnectPtr conn, const char *host, const char *port) +{ + xenUnifiedPrivatePtr priv; + struct addrinfo *res, *r; + struct addrinfo hints; + int saved_errno = EINVAL; + int ret; + + if ((conn == NULL) || (host == NULL) || (port == NULL)) + return (-1); + + priv = (xenUnifiedPrivatePtr) conn->privateData; + + priv->addrlen = 0; + memset(&priv->addr, 0, sizeof(priv->addr)); + + // http://people.redhat.com/drepper/userapi-ipv6.html + memset (&hints, 0, sizeof hints); + hints.ai_socktype = SOCK_STREAM; + hints.ai_flags = AI_ADDRCONFIG; + + ret = getaddrinfo (host, port, &hints, &res); + if (ret != 0) { + virXendError(NULL, VIR_ERR_UNKNOWN_HOST, + _("unable to resolve hostname '%s': %s"), + host, gai_strerror (ret)); + return -1; + } + + /* Try to connect to each returned address in turn. */ + for (r = res; r; r = r->ai_next) { + int sock; + + sock = socket (r->ai_family, SOCK_STREAM, r->ai_protocol); + if (sock == -1) { + saved_errno = errno; + continue; + } + + if (connect (sock, r->ai_addr, r->ai_addrlen) == -1) { + saved_errno = errno; + close (sock); + continue; + } + + priv->addrlen = r->ai_addrlen; + priv->addrfamily = r->ai_family; + priv->addrprotocol = r->ai_protocol; + memcpy(&priv->addr, + r->ai_addr, + r->ai_addrlen); + close(sock); + break; + } + + freeaddrinfo (res); + + if (!priv->addrlen) { + virReportSystemError(conn, saved_errno, + _("unable to connect to '%s:%s'"), + host, port); + return -1; + } + + return 0; } @@ -2721,14 +2728,18 @@ xenDaemonOpen(virConnectPtr conn, /* * try though http on port 8000 */ - ret = xenDaemonOpen_tcp(conn, "localhost", 8000); + ret = xenDaemonOpen_tcp(conn, "localhost", "8000"); if (ret < 0) goto failed; ret = xend_detect_config_version(conn); if (ret == -1) goto failed; } else if (STRCASEEQ (conn->uri->scheme, "http")) { - ret = xenDaemonOpen_tcp(conn, conn->uri->server, conn->uri->port); + 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); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|