From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> In daemons using pidfiles to protect against concurrent execution there is a possibility that a crash may leave a stale pidfile on disk, which then prevents later restart of the daemon. To avoid this problem, introduce a pair of APIs which make use of virFileLock to ensure crash-safe & race condition-safe pidfile acquisition & releae * src/libvirt_private.syms, src/util/virpidfile.c, src/util/virpidfile.h: Add virPidFileAcquire and virPidFileRelease --- po/POTFILES.in | 1 + src/libvirt_private.syms | 4 + src/util/virpidfile.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virpidfile.h | 12 ++++ 4 files changed, 159 insertions(+), 0 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 7bb08b0..b200f89 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -123,6 +123,7 @@ src/util/sysinfo.c src/util/util.c src/util/viraudit.c src/util/virfile.c +src/util/virpidfile.c src/util/virterror.c src/util/xml.c src/vbox/vbox_MSCOMGlue.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fee3520..64b91ee 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1120,11 +1120,15 @@ virFileFdopen; # virpidfile.h +virPidFileAcquire; +virPidFileAcquirePath; virPidFileBuildPath; virPidFileRead; virPidFileReadIfAlive; virPidFileReadPath; virPidFileReadPathIfAlive; +virPidFileRelease; +virPidFileReleasePath; virPidFileWrite; virPidFileWritePath; virPidFileDelete; diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index dc92868..005c95d 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -25,11 +25,15 @@ #include <fcntl.h> #include <signal.h> +#include <sys/stat.h> #include "virpidfile.h" #include "virfile.h" #include "memory.h" #include "util.h" +#include "intprops.h" +#include "logging.h" +#include "virterror_internal.h" char *virPidFileBuildPath(const char *dir, const char* name) @@ -255,3 +259,141 @@ cleanup: VIR_FREE(pidfile); return rc; } + + + +int virPidFileAcquirePath(const char *path, + pid_t pid) +{ + int fd = -1; + char ebuf[1024]; + char pidstr[INT_BUFSIZE_BOUND(pid)]; + verify(sizeof(pid_t) <= sizeof(unsigned int)); + + if (path[0] == '\0') + return 0; + + while (1) { + struct stat a, b; + if ((fd = open(path, O_WRONLY|O_CREAT, 0644)) < 0) { + VIR_ERROR(_("Failed to open pid file '%s' : %s"), + path, virStrerror(errno, ebuf, sizeof ebuf)); + return -1; + } + + if (fstat(fd, &b) < 0) { + VIR_ERROR(_("Unable to check status of pid file '%s': %s"), + path, virStrerror(errno, ebuf, sizeof ebuf)); + VIR_FORCE_CLOSE(fd); + return -1; + } + + if (virFileLock(fd, false, 0, 1) < 0) { + VIR_ERROR(_("Failed to acquire pid file '%s' : %s"), + path, virStrerror(errno, ebuf, sizeof ebuf)); + VIR_FORCE_CLOSE(fd); + return -1; + } + + /* Now make sure the pidfile we locked is the same + * one that now exists on the filesystem + */ + if (stat(path, &a) < 0) { + VIR_DEBUG("Pid file '%s' disappeared: %s", + path, virStrerror(errno, ebuf, sizeof ebuf)); + VIR_FORCE_CLOSE(fd); + /* Someone else must be racing with us, so try agin */ + continue; + } + + if (a.st_ino == b.st_ino) + break; + + VIR_DEBUG("Pid file '%s' was recreated", path); + VIR_FORCE_CLOSE(fd); + /* Someone else must be racing with us, so try agin */ + } + + snprintf(pidstr, sizeof(pidstr), "%u", (unsigned int)pid); + + if (safewrite(fd, pidstr, strlen(pidstr)) < 0) { + VIR_ERROR(_("Failed to write to pid file '%s' : %s"), + path, virStrerror(errno, ebuf, sizeof ebuf)); + VIR_FORCE_CLOSE(fd); + } + + return fd; +} + + +int virPidFileAcquire(const char *dir, + const char *name, + pid_t pid) +{ + 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 = virPidFileAcquirePath(pidfile, pid); + +cleanup: + VIR_FREE(pidfile); + return rc; +} + + +int virPidFileReleasePath(const char *path, + int fd) +{ + int rc = 0; + /* + * We need to unlink before closing the FD to avoid + * a race, but Win32 won't let you unlink an open + * file handle. So on that platform we do the reverse + * and just have to live with the possible race. + */ +#ifdef WIN32 + VIR_FORCE_CLOSE(fd); + if (unlink(path) < 0 && errno != ENOENT) + rc = -errno; +#else + if (unlink(path) < 0 && errno != ENOENT) + rc = -errno; + VIR_FORCE_CLOSE(fd); +#endif + return rc; +} + + +int virPidFileRelease(const char *dir, + const char *name, + int fd) +{ + 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 = virPidFileReleasePath(pidfile, fd); + +cleanup: + VIR_FREE(pidfile); + return rc; +} diff --git a/src/util/virpidfile.h b/src/util/virpidfile.h index 6659f1c..dc44fc9 100644 --- a/src/util/virpidfile.h +++ b/src/util/virpidfile.h @@ -55,4 +55,16 @@ int virPidFileDelete(const char *dir, const char *name); +int virPidFileAcquirePath(const char *path, + pid_t pid) ATTRIBUTE_RETURN_CHECK; +int virPidFileAcquire(const char *dir, + const char *name, + pid_t pid) ATTRIBUTE_RETURN_CHECK; + +int virPidFileReleasePath(const char *path, + int fd); +int virPidFileRelease(const char *dir, + const char *name, + int fd); + #endif /* __VIR_PIDFILE_H__ */ -- 1.7.6 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list