From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Add some simple wrappers around the fcntl() discretionary file locking capability. * src/util/util.c, src/util/util.h, src/libvirt_private.syms: Add virFileLock and virFileUnlock APIs --- src/libvirt_private.syms | 2 + src/util/virfile.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 3 ++ 3 files changed, 88 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 830222b..261f3e0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1039,6 +1039,7 @@ virFileFindMountPoint; virFileHasSuffix; virFileIsExecutable; virFileLinkPointsTo; +virFileLock; virFileMakePath; virFileMatchesNameSuffix; virFileOpenAs; @@ -1051,6 +1052,7 @@ virFileReadPidPath; virFileResolveLink; virFileSanitizePath; virFileStripSuffix; +virFileUnlock; virFileWaitForDevices; virFileWriteStr; virFindFileInPath; diff --git a/src/util/virfile.c b/src/util/virfile.c index 8b32518..1346c1a 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -251,3 +251,86 @@ virFileDirectFdFree(virFileDirectFdPtr dfd) virCommandFree(dfd->cmd); VIR_FREE(dfd); } + + +#ifndef WIN32 +/** + * virFileLock: + * @fd: file descriptor to acquire the lock on + * @shared: type of lock to acquire + * @start: byte offset to start lock + * @len: length of lock (0 to acquire entire remaining file from @start) + * + * Attempt to acquire a lock on the file @fd. If @shared + * is true, then the a shared lock will be acquired, + * otherwise an exclusive lock will be acquired. If + * the lock cannot be acquired, an error will be + * returned. This will not wait to acquire the lock if + * another process already holds it. + * + * The lock will be released will @fd is closed. The lock + * will also be released if *any* other open file descriptor + * pointing to the same underlying file is closed. As such + * this function should not be relied on in multi-threaded + * apps where other threads can be opening/closing arbitrary + * files. + * + * Returns: 0 on success, or -errno otherwise + */ +int virFileLock(int fd, bool shared, off_t start, off_t len) +{ + struct flock fl = { + .l_type = shared ? F_RDLCK : F_WRLCK, + .l_whence = SEEK_SET, + .l_start = start, + .l_len = len, + }; + + if (fcntl(fd, F_SETLK, &fl) < 0) + return -errno; + + return 0; +} + + +/** + * virFileUnlock: + * @fd: file descriptor to release the lock on + * @start: byte offset to start unlock + * @len: length of lock (0 to release entire remaining file from @start) + * + * Release a lock previously acquired with virFileUnlock(). + * NB the lock will also be released if any open file descriptor + * pointing to the same file as @fd is closed + * + * Returns 0 on succcess, or -errno on error + */ +int virFileUnlock(int fd, off_t start, off_t len) +{ + struct flock fl = { + .l_type = F_UNLCK, + .l_whence = SEEK_SET, + .l_start = start, + .l_len = len, + }; + + if (fcntl(fd, F_SETLK, &fl) < 0) + return -errno; + + return 0; +} +#else +int virFileLock(int fd ATTRIBUTE_UNUSED, + bool shared ATTRIBUTE_UNUSED, + off_t start ATTRIBUTE_UNUSED, + off_t len ATTRIBUTE_UNUSED) +{ + return -ENOSYS; +} +int virFileUnlock(int fd ATTRIBUTE_UNUSED, + off_t start ATTRIBUTE_UNUSED, + off_t len ATTRIBUTE_UNUSED) +{ + return -ENOSYS; +} +#endif diff --git a/src/util/virfile.h b/src/util/virfile.h index 0906568..e025614 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -65,4 +65,7 @@ int virFileDirectFdClose(virFileDirectFdPtr dfd); void virFileDirectFdFree(virFileDirectFdPtr dfd); +int virFileLock(int fd, bool shared, off_t start, off_t len); +int virFileUnlock(int fd, off_t start, off_t len); + #endif /* __VIR_FILES_H */ -- 1.7.6

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The functions for manipulating pidfiles are in util/util.{c,h}. We will shortly be adding some further pidfile related functions. To avoid further growing util.c, this moves the pidfile related functions into a dedicated virpidfile.{c,h}. The functions are also all renamed to have 'virPidFile' as their name prefix * util/util.h, util/util.c: Remove all pidfile code * util/virpidfile.c, util/virpidfile.h: Add new APIs for pidfile handling. * lxc/lxc_controller.c, lxc/lxc_driver.c, network/bridge_driver.c, qemu/qemu_process.c: Add virpidfile.h include and adapt for API renames --- src/Makefile.am | 1 + src/libvirt_private.syms | 14 ++- src/lxc/lxc_controller.c | 5 +- src/lxc/lxc_driver.c | 5 +- src/network/bridge_driver.c | 19 ++-- src/qemu/qemu_process.c | 5 +- src/util/command.c | 3 +- src/util/util.c | 152 --------------------------------- src/util/util.h | 15 --- src/util/virpidfile.c | 199 +++++++++++++++++++++++++++++++++++++++++++ src/util/virpidfile.h | 50 +++++++++++ 11 files changed, 281 insertions(+), 187 deletions(-) create mode 100644 src/util/virpidfile.c create mode 100644 src/util/virpidfile.h diff --git a/src/Makefile.am b/src/Makefile.am index 009ff25..cf7c003 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -84,6 +84,7 @@ UTIL_SOURCES = \ util/util.c util/util.h \ util/viraudit.c util/viraudit.h \ util/virfile.c util/virfile.h \ + util/virpidfile.c util/virpidfile.h \ util/xml.c util/xml.h \ util/virterror.c util/virterror_internal.h \ util/virkeycode.c util/virkeycode.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 261f3e0..7a96c1e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1033,7 +1033,6 @@ virEventAddHandle; virEventRemoveHandle; virFileAbsPath; virFileBuildPath; -virFileDeletePid; virFileExists; virFileFindMountPoint; virFileHasSuffix; @@ -1044,11 +1043,8 @@ virFileMakePath; virFileMatchesNameSuffix; virFileOpenAs; virFileOpenTty; -virFilePid; virFileReadAll; virFileReadLimFD; -virFileReadPid; -virFileReadPidPath; virFileResolveLink; virFileSanitizePath; virFileStripSuffix; @@ -1123,6 +1119,16 @@ virFileFclose; virFileFdopen; +# virpidfile.h +virPidFileBuildPath; +virPidFileRead; +virPidFileReadPath; +virPidFileWrite; +virPidFileWritePath; +virPidFileDelete; +virPidFileDeletePath; + + # virterror_internal.h virDispatchError; virErrorMsg; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 45b4c70..5028251 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -56,6 +56,7 @@ #include "memory.h" #include "util.h" #include "virfile.h" +#include "virpidfile.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -1136,7 +1137,7 @@ int main(int argc, char *argv[]) goto cleanup; if (pid > 0) { - if ((rc = virFileWritePid(LXC_STATE_DIR, name, pid)) < 0) { + if ((rc = virPidFileWrite(LXC_STATE_DIR, name, pid)) < 0) { virReportSystemError(-rc, _("Unable to write pid file '%s/%s.pid'"), LXC_STATE_DIR, name); @@ -1179,7 +1180,7 @@ int main(int argc, char *argv[]) cleanup: if (def) - virFileDeletePid(LXC_STATE_DIR, def->name); + virPidFileDelete(LXC_STATE_DIR, def->name); lxcControllerCleanupInterfaces(nveths, veths); if (sockpath) unlink(sockpath); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 2d94309..bb560b6 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -50,6 +50,7 @@ #include "stats_linux.h" #include "hooks.h" #include "virfile.h" +#include "virpidfile.h" #include "fdstream.h" #include "domain_audit.h" #include "domain_nwfilter.h" @@ -1030,7 +1031,7 @@ static void lxcVmCleanup(lxc_driver_t *driver, virEventRemoveHandle(priv->monitorWatch); VIR_FORCE_CLOSE(priv->monitor); - virFileDeletePid(driver->stateDir, vm->def->name); + virPidFileDelete(driver->stateDir, vm->def->name); virDomainDeleteConfig(driver->stateDir, NULL, vm); virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); @@ -1612,7 +1613,7 @@ static int lxcVmStart(virConnectPtr conn, goto cleanup; /* And get its pid */ - if ((r = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) < 0) { + if ((r = virPidFileRead(driver->stateDir, vm->def->name, &vm->pid)) < 0) { virReportSystemError(-r, _("Failed to read pid file %s/%s.pid"), driver->stateDir, vm->def->name); diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c90db63..8b27320 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -49,6 +49,7 @@ #include "network_conf.h" #include "driver.h" #include "buf.h" +#include "virpidfile.h" #include "util.h" #include "command.h" #include "memory.h" @@ -218,7 +219,7 @@ networkFindActiveConfigs(struct network_driver *driver) { if (obj->def->ips && (obj->def->nips > 0)) { char *pidpath, *radvdpidbase; - if (virFileReadPid(NETWORK_PID_DIR, obj->def->name, + if (virPidFileRead(NETWORK_PID_DIR, obj->def->name, &obj->dnsmasqPid) == 0) { /* Check that it's still alive */ if (kill(obj->dnsmasqPid, 0) != 0) @@ -236,7 +237,7 @@ networkFindActiveConfigs(struct network_driver *driver) { virReportOOMError(); goto cleanup; } - if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase, + if (virPidFileRead(NETWORK_PID_DIR, radvdpidbase, &obj->radvdPid) == 0) { /* Check that it's still alive */ if (kill(obj->radvdPid, 0) != 0) @@ -728,7 +729,7 @@ networkStartDhcpDaemon(virNetworkObjPtr network) goto cleanup; } - if (!(pidfile = virFilePid(NETWORK_PID_DIR, network->def->name))) { + if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, network->def->name))) { virReportOOMError(); goto cleanup; } @@ -765,7 +766,7 @@ networkStartDhcpDaemon(virNetworkObjPtr network) * pid */ - ret = virFileReadPid(NETWORK_PID_DIR, network->def->name, + ret = virPidFileRead(NETWORK_PID_DIR, network->def->name, &network->dnsmasqPid); if (ret < 0) goto cleanup; @@ -818,7 +819,7 @@ networkStartRadvd(virNetworkObjPtr network) virReportOOMError(); goto cleanup; } - if (!(pidfile = virFilePid(NETWORK_PID_DIR, radvdpidbase))) { + if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, radvdpidbase))) { virReportOOMError(); goto cleanup; } @@ -885,7 +886,7 @@ networkStartRadvd(virNetworkObjPtr network) * a dummy pidfile name - virCommand will create the pidfile we * want to use (this is necessary because radvd's internal * daemonization and pidfile creation causes a race, and the - * virFileReadPid() below will fail if we use them). + * virPidFileRead() below will fail if we use them). * Unfortunately, it isn't possible to tell radvd to not create * its own pidfile, so we just let it do so, with a slightly * different name. Unused, but harmless. @@ -901,7 +902,7 @@ networkStartRadvd(virNetworkObjPtr network) if (virCommandRun(cmd, NULL) < 0) goto cleanup; - if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase, + if (virPidFileRead(NETWORK_PID_DIR, radvdpidbase, &network->radvdPid) < 0) goto cleanup; @@ -1919,7 +1920,7 @@ static int networkShutdownNetworkVirtual(struct network_driver *driver, if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) { virReportOOMError(); } else { - virFileDeletePid(NETWORK_PID_DIR, radvdpidbase); + virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); VIR_FREE(radvdpidbase); } } @@ -2486,7 +2487,7 @@ static int networkUndefine(virNetworkPtr net) { virReportOOMError(); goto cleanup; } - virFileDeletePid(NETWORK_PID_DIR, radvdpidbase); + virPidFileDelete(NETWORK_PID_DIR, radvdpidbase); VIR_FREE(radvdpidbase); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 30c8b28..6f54b30 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -50,6 +50,7 @@ #include "memory.h" #include "hooks.h" #include "virfile.h" +#include "virpidfile.h" #include "util.h" #include "c-ctype.h" #include "nodeinfo.h" @@ -2771,7 +2772,7 @@ int qemuProcessStart(virConnectPtr conn, priv->gotShutdown = false; VIR_FREE(priv->pidfile); - if (!(priv->pidfile = virFilePid(driver->stateDir, vm->def->name))) { + if (!(priv->pidfile = virPidFileBuildPath(driver->stateDir, vm->def->name))) { virReportSystemError(errno, "%s", _("Failed to build pidfile path.")); goto cleanup; @@ -2880,7 +2881,7 @@ int qemuProcessStart(virConnectPtr conn, /* wait for qemu process to show up */ if (ret == 0) { - if (virFileReadPidPath(priv->pidfile, &vm->pid) < 0) { + if (virPidFileReadPath(priv->pidfile, &vm->pid) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("Domain %s didn't show up"), vm->def->name); ret = -1; diff --git a/src/util/command.c b/src/util/command.c index 26fcb28..d390478 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -39,6 +39,7 @@ #include "util.h" #include "logging.h" #include "virfile.h" +#include "virpidfile.h" #include "buf.h" #include "ignore-value.h" #include "verify.h" @@ -493,7 +494,7 @@ virExecWithHook(const char *const*argv, } if (pid > 0) { - if (pidfile && (virFileWritePidPath(pidfile,pid) < 0)) { + if (pidfile && (virPidFileWritePath(pidfile,pid) < 0)) { kill(pid, SIGTERM); usleep(500*1000); kill(pid, SIGTERM); diff --git a/src/util/util.c b/src/util/util.c index 2e2a6a0..e3b216f 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1151,158 +1151,6 @@ int virFileOpenTtyAt(const char *ptmx ATTRIBUTE_UNUSED, } #endif -char* virFilePid(const char *dir, const char* name) -{ - char *pidfile; - if (virAsprintf(&pidfile, "%s/%s.pid", dir, name) < 0) - return NULL; - return pidfile; -} - -int virFileWritePid(const char *dir, - const char *name, - pid_t pid) -{ - int rc; - char *pidfile = NULL; - - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } - - if (virFileMakePath(dir) < 0) { - rc = -errno; - goto cleanup; - } - - if (!(pidfile = virFilePid(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } - - rc = virFileWritePidPath(pidfile, pid); - -cleanup: - VIR_FREE(pidfile); - return rc; -} - -int virFileWritePidPath(const char *pidfile, - pid_t pid) -{ - int rc; - int fd; - FILE *file = NULL; - - if ((fd = open(pidfile, - O_WRONLY | O_CREAT | O_TRUNC, - S_IRUSR | S_IWUSR)) < 0) { - rc = -errno; - goto cleanup; - } - - if (!(file = VIR_FDOPEN(fd, "w"))) { - rc = -errno; - VIR_FORCE_CLOSE(fd); - goto cleanup; - } - - if (fprintf(file, "%d", pid) < 0) { - rc = -errno; - goto cleanup; - } - - rc = 0; - -cleanup: - if (VIR_FCLOSE(file) < 0) - rc = -errno; - - return rc; -} - - -int virFileReadPidPath(const char *path, - pid_t *pid) -{ - FILE *file; - int rc; - - *pid = 0; - - if (!(file = fopen(path, "r"))) { - rc = -errno; - goto cleanup; - } - - if (fscanf(file, "%d", pid) != 1) { - rc = -EINVAL; - VIR_FORCE_FCLOSE(file); - goto cleanup; - } - - if (VIR_FCLOSE(file) < 0) { - rc = -errno; - goto cleanup; - } - - rc = 0; - - cleanup: - return rc; -} - - -int virFileReadPid(const char *dir, - const char *name, - pid_t *pid) -{ - int rc; - char *pidfile = NULL; - *pid = 0; - - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } - - if (!(pidfile = virFilePid(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } - - rc = virFileReadPidPath(pidfile, pid); - - cleanup: - VIR_FREE(pidfile); - return rc; -} - -int virFileDeletePid(const char *dir, - const char *name) -{ - int rc = 0; - char *pidfile = NULL; - - if (name == NULL || dir == NULL) { - rc = -EINVAL; - goto cleanup; - } - - if (!(pidfile = virFilePid(dir, name))) { - rc = -ENOMEM; - goto cleanup; - } - - if (unlink(pidfile) < 0 && errno != ENOENT) - rc = -errno; - -cleanup: - VIR_FREE(pidfile); - return rc; -} - /* * Creates an absolute path for a potentially relative path. diff --git a/src/util/util.h b/src/util/util.h index af8b15d..e76da9c 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -120,21 +120,6 @@ int virFileOpenTtyAt(const char *ptmx, char **ttyName, int rawmode); -char* virFilePid(const char *dir, - const char *name); - -int virFileWritePidPath(const char *path, - pid_t pid) ATTRIBUTE_RETURN_CHECK; -int virFileWritePid(const char *dir, - const char *name, - pid_t pid) ATTRIBUTE_RETURN_CHECK; -int virFileReadPidPath(const char *path, - pid_t *pid) ATTRIBUTE_RETURN_CHECK; -int virFileReadPid(const char *dir, - const char *name, - pid_t *pid) ATTRIBUTE_RETURN_CHECK; -int virFileDeletePid(const char *dir, - const char *name); char *virArgvToString(const char *const *argv); diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c new file mode 100644 index 0000000..25c3272 --- /dev/null +++ b/src/util/virpidfile.c @@ -0,0 +1,199 @@ +/* + * virpidfile.c: manipulation of pidfiles + * + * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2006, 2007 Binary Karma + * Copyright (C) 2006 Shuveb Hussain + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> + +#include <fcntl.h> + +#include "virpidfile.h" +#include "virfile.h" +#include "memory.h" +#include "util.h" + + +char *virPidFileBuildPath(const char *dir, const char* name) +{ + char *pidfile; + + if (virAsprintf(&pidfile, "%s/%s.pid", dir, name) < 0) + return NULL; + + return pidfile; +} + + +int virPidFileWritePath(const char *pidfile, + pid_t pid) +{ + int rc; + int fd; + FILE *file = NULL; + + if ((fd = open(pidfile, + O_WRONLY | O_CREAT | O_TRUNC, + S_IRUSR | S_IWUSR)) < 0) { + rc = -errno; + goto cleanup; + } + + if (!(file = VIR_FDOPEN(fd, "w"))) { + rc = -errno; + VIR_FORCE_CLOSE(fd); + goto cleanup; + } + + if (fprintf(file, "%d", pid) < 0) { + rc = -errno; + goto cleanup; + } + + rc = 0; + +cleanup: + if (VIR_FCLOSE(file) < 0) + rc = -errno; + + return rc; +} + + +int virPidFileWrite(const char *dir, + const char *name, + pid_t pid) +{ + int rc; + char *pidfile = NULL; + + if (name == NULL || dir == NULL) { + rc = -EINVAL; + goto cleanup; + } + + if (virFileMakePath(dir) < 0) { + rc = -errno; + goto cleanup; + } + + if (!(pidfile = virPidFileBuildPath(dir, name))) { + rc = -ENOMEM; + goto cleanup; + } + + rc = virPidFileWritePath(pidfile, pid); + +cleanup: + VIR_FREE(pidfile); + return rc; +} + + +int virPidFileReadPath(const char *path, + pid_t *pid) +{ + FILE *file; + int rc; + + *pid = 0; + + if (!(file = fopen(path, "r"))) { + rc = -errno; + goto cleanup; + } + + if (fscanf(file, "%d", pid) != 1) { + rc = -EINVAL; + VIR_FORCE_FCLOSE(file); + goto cleanup; + } + + if (VIR_FCLOSE(file) < 0) { + rc = -errno; + goto cleanup; + } + + rc = 0; + + cleanup: + return rc; +} + + +int virPidFileRead(const char *dir, + const char *name, + pid_t *pid) +{ + int rc; + char *pidfile = NULL; + *pid = 0; + + if (name == NULL || dir == NULL) { + rc = -EINVAL; + goto cleanup; + } + + if (!(pidfile = virPidFileBuildPath(dir, name))) { + rc = -ENOMEM; + goto cleanup; + } + + rc = virPidFileReadPath(pidfile, pid); + + cleanup: + VIR_FREE(pidfile); + return rc; +} + + +int virPidFileDeletePath(const char *pidfile) +{ + int rc = 0; + + if (unlink(pidfile) < 0 && errno != ENOENT) + rc = -errno; + + return rc; +} + + +int virPidFileDelete(const char *dir, + const char *name) +{ + int rc = 0; + char *pidfile = NULL; + + if (name == NULL || dir == NULL) { + rc = -EINVAL; + goto cleanup; + } + + if (!(pidfile = virPidFileBuildPath(dir, name))) { + rc = -ENOMEM; + goto cleanup; + } + + rc = virPidFileDeletePath(pidfile); + +cleanup: + VIR_FREE(pidfile); + return rc; +} diff --git a/src/util/virpidfile.h b/src/util/virpidfile.h new file mode 100644 index 0000000..e28a3c1 --- /dev/null +++ b/src/util/virpidfile.h @@ -0,0 +1,50 @@ +/* + * virpidfile.h: manipulation of pidfiles + * + * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2006, 2007 Binary Karma + * Copyright (C) 2006 Shuveb Hussain + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef __VIR_PIDFILE_H__ +# define __VIR_PIDFILE_H__ + +# include <sys/types.h> +# include "internal.h" + +char *virPidFileBuildPath(const char *dir, + const char *name); + +int virPidFileWritePath(const char *path, + pid_t pid) ATTRIBUTE_RETURN_CHECK; +int virPidFileWrite(const char *dir, + const char *name, + pid_t pid) ATTRIBUTE_RETURN_CHECK; + +int virPidFileReadPath(const char *path, + pid_t *pid) ATTRIBUTE_RETURN_CHECK; +int virPidFileRead(const char *dir, + const char *name, + pid_t *pid) ATTRIBUTE_RETURN_CHECK; + +int virPidFileDeletePath(const char *path); +int virPidFileDelete(const char *dir, + const char *name); + + +#endif /* __VIR_PIDFILE_H__ */ -- 1.7.6

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; In some cases the caller of virPidFileRead might like extra checks to determine whether the pid just read is really the one they are expecting. This adds virPidFileReadIfValid which will check whether the pid is still alive with kill(0, -1), and (on linux only) will look at /proc/$PID/path * libvirt_private.syms, util/virpidfile.c, util/virpidfile.h: Add virPidFileReadIfValid and virPidFileReadPathIfValid * network/bridge_driver.c: Use new APIs to check PID validity --- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 35 ++++--------------------- src/util/virpidfile.c | 58 +++++++++++++++++++++++++++++++++++++++++++ src/util/virpidfile.h | 8 ++++++ 4 files changed, 74 insertions(+), 29 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7a96c1e..fee3520 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1122,7 +1122,9 @@ virFileFdopen; # virpidfile.h virPidFileBuildPath; virPidFileRead; +virPidFileReadIfAlive; virPidFileReadPath; +virPidFileReadPathIfAlive; virPidFileWrite; virPidFileWritePath; virPidFileDelete; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 8b27320..cea8771 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -217,40 +217,17 @@ networkFindActiveConfigs(struct network_driver *driver) { /* Try and read dnsmasq/radvd pids if any */ if (obj->def->ips && (obj->def->nips > 0)) { - char *pidpath, *radvdpidbase; - - if (virPidFileRead(NETWORK_PID_DIR, obj->def->name, - &obj->dnsmasqPid) == 0) { - /* Check that it's still alive */ - if (kill(obj->dnsmasqPid, 0) != 0) - obj->dnsmasqPid = -1; - if (virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid) < 0) { - virReportOOMError(); - goto cleanup; - } - if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0) - obj->dnsmasqPid = -1; - VIR_FREE(pidpath); - } + char *radvdpidbase; + + ignore_value(virPidFileReadIfAlive(NETWORK_PID_DIR, obj->def->name, + &obj->dnsmasqPid, DNSMASQ)); if (!(radvdpidbase = networkRadvdPidfileBasename(obj->def->name))) { virReportOOMError(); goto cleanup; } - if (virPidFileRead(NETWORK_PID_DIR, radvdpidbase, - &obj->radvdPid) == 0) { - /* Check that it's still alive */ - if (kill(obj->radvdPid, 0) != 0) - obj->radvdPid = -1; - if (virAsprintf(&pidpath, "/proc/%d/exe", obj->radvdPid) < 0) { - virReportOOMError(); - VIR_FREE(radvdpidbase); - goto cleanup; - } - if (virFileLinkPointsTo(pidpath, RADVD) == 0) - obj->radvdPid = -1; - VIR_FREE(pidpath); - } + ignore_value(virPidFileReadIfAlive(NETWORK_PID_DIR, radvdpidbase, + &obj->radvdPid, RADVD)); VIR_FREE(radvdpidbase); } } diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 25c3272..dc92868 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -24,6 +24,7 @@ #include <config.h> #include <fcntl.h> +#include <signal.h> #include "virpidfile.h" #include "virfile.h" @@ -164,6 +165,63 @@ int virPidFileRead(const char *dir, } +int virPidFileReadPathIfAlive(const char *path, + pid_t *pid, + const char *binpath) +{ + int rc; + char *procpath = NULL; + + rc = virPidFileReadPath(path, pid); + if (rc < 0) + return rc; + + /* Check that it's still alive */ + if (kill(*pid, 0) < 0) { + *pid = -1; + return 0; + } + + if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0) { + *pid = -1; + return 0; + } +#ifdef __linux__ + if (virFileLinkPointsTo(procpath, binpath) == 0) + *pid = -1; +#endif + VIR_FREE(procpath); + + return 0; +} + + +int virPidFileReadIfAlive(const char *dir, + const char *name, + pid_t *pid, + const char *binpath) +{ + int rc = 0; + char *pidfile = NULL; + + if (name == NULL || dir == NULL) { + rc = -EINVAL; + goto cleanup; + } + + if (!(pidfile = virPidFileBuildPath(dir, name))) { + rc = -ENOMEM; + goto cleanup; + } + + rc = virPidFileReadPathIfAlive(pidfile, pid, binpath); + +cleanup: + VIR_FREE(pidfile); + return rc; +} + + int virPidFileDeletePath(const char *pidfile) { int rc = 0; diff --git a/src/util/virpidfile.h b/src/util/virpidfile.h index e28a3c1..6659f1c 100644 --- a/src/util/virpidfile.h +++ b/src/util/virpidfile.h @@ -42,6 +42,14 @@ int virPidFileRead(const char *dir, const char *name, pid_t *pid) ATTRIBUTE_RETURN_CHECK; +int virPidFileReadPathIfAlive(const char *path, + pid_t *pid, + const char *binpath) ATTRIBUTE_RETURN_CHECK; +int virPidFileReadIfAlive(const char *dir, + const char *name, + pid_t *pid, + const char *binpath) ATTRIBUTE_RETURN_CHECK; + int virPidFileDeletePath(const char *path); int virPidFileDelete(const char *dir, const char *name); -- 1.7.6

Eh, I forgot to add some more notes... ... Indentation. And anyway, what about implementing the second half of this function as follows: #ifdef __linux__ if (binpath) { char *procpath = NULL; if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0 || !virFileLinkPointsTo(procpath, binpath)) *pid = -1; VIR_FREE(procpath); } #endif Building procpath makes little sense on non-linux and checking binpath first allows callers to use *IfAlive for checking pid liveliness without binpath checks (in case any caller wants to do so) and allows us to move procpath declaration into the same ifdef block :-) I think we should also document that both *IfAlive functions return -errno if the pid file cannot be read or doesn't contain integer value. If it contains a number but such process is not running or doesn't correspond to binpath, the return value is 0 but pid is -1. Jirka

On Wed, Aug 10, 2011 at 16:37:27 +0100, Daniel P. Berrange wrote: Hmm, why not virReportSystemError(errno, ...) everywhere instead of VIR_ERROR and virStrerror? Also I think we should call virSetCloseExec on the file descriptor for a little bit of extra safety. ... ACK with the comments addressed. Jirka