Add some APIs which use locking for crashsafe pidfile handling

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

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 7bb08b012f86d14e5786714576921278b0885bd4..b200f896f293a1888336f17ecbde19dbb324a90b 100644 (file)
--- 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 fee35203081711422064c75acd88b860f237cfca..64b91eea3bb1522d99e3719e142c0ad586f06fe2 100644 (file)
--- 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 c0b12867db09d138f0382d76db098c78722be7c4..38cc7e28c619defa81d55a47f596eeb6f9832cfd 100644 (file)
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -25,13 +25,19 @@
 
 #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"
 
 
+#define VIR_FROM_THIS VIR_FROM_NONE
+
 char *virPidFileBuildPath(const char *dir, const char* name)
 {
     char *pidfile;
@@ -291,3 +297,153 @@ cleanup:
     VIR_FREE(pidfile);
     return rc;
 }
+
+
+
+int virPidFileAcquirePath(const char *path,
+                          pid_t pid)
+{
+    int fd = -1;
+    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) {
+            virReportSystemError(errno,
+                                 _("Failed to open pid file '%s'"),
+                                 path);
+            return -1;
+        }
+
+        if (virSetCloseExec(fd) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to set close-on-exec flag '%s'"),
+                                 path);
+            VIR_FORCE_CLOSE(fd);
+            return -1;
+        }
+
+        if (fstat(fd, &b) < 0) {
+            virReportSystemError(errno,
+                                 _("Unable to check status of pid file '%s'"),
+                                 path);
+            VIR_FORCE_CLOSE(fd);
+            return -1;
+        }
+
+        if (virFileLock(fd, false, 0, 1) < 0) {
+            virReportSystemError(errno,
+                                 _("Failed to acquire pid file '%s'"),
+                                 path);
+            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) {
+            char ebuf[1024];
+            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) {
+        virReportSystemError(errno,
+                             _("Failed to write to pid file '%s'"),
+                             path);
+        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 6659f1c1a4d3829402658be3a0e6cd471f8d2af5..dc44fc96f157b62f35c94dde63641e347e05ada1 100644 (file)
--- 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__ */