Introduce functions for checking whether a pidfile is valid

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 virPidFileReadIfAlive 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

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7a96c1e3981f2d267ee01dbdc501f4037afd71bc..fee35203081711422064c75acd88b860f237cfca 100644 (file)
--- 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 8b2732057f8036a3bc92fd2577d532a83d188764..cea8771774ea6f8d117d2907a58cd579593755b4 100644 (file)
--- 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 25c3272b383376a10900783b058dc11d660963fd..c0b12867db09d138f0382d76db098c78722be7c4 100644 (file)
--- 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,99 @@ int virPidFileRead(const char *dir,
 }
 
 
+
+/**
+ * virPidFileReadPathIfAlive:
+ * @path: path to pidfile
+ * @pid: variable to return pid in
+ * @binpath: path of executable associated with the pidfile
+ *
+ * This will attempt to read a pid from @path, and store it
+ * in @pid. The @pid will only be set, however, if the
+ * pid in @path is running, and its executable path
+ * resolves to @binpath. This adds protection against
+ * recycling of previously reaped pids.
+ *
+ * Returns -errno upon error, or zero on successful
+ * reading of the pidfile. If the PID was not still
+ * alive, zero will be returned, but @pid will be
+ * set to -1.
+ */
+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;
+}
+
+
+/**
+ * virPidFileReadIfAlive:
+ * @dir: directory containing pidfile
+ * @name: base filename of pidfile
+ * @pid: variable to return pid in
+ * @binpath: path of executable associated with the pidfile
+ *
+ * This will attempt to read a pid from the pidfile @name
+ * in directory @dir, and store it in @pid. The @pid will
+ * only be set, however, if the pid in @name is running,
+ * and its executable path resolves to @binpath. This adds
+ * protection against recycling of previously reaped pids.
+ *
+ * Returns -errno upon error, or zero on successful
+ * reading of the pidfile. If the PID was not still
+ * alive, zero will be returned, but @pid will be
+ * set to -1.
+ */
+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 e28a3c1689d2bd133f5e218fbb6ef845268fe5b5..6659f1c1a4d3829402658be3a0e6cd471f8d2af5 100644 (file)
--- 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);