qemu: Kill processes used for QMP caps probing

Since libvirt switched to QMP capabilities probing recently, it starts
QEMU process used for this probing with -daemonize, which means
virCommandAbort can no longer reach these processes. As a result of
that, restarting libvirtd will leave several new QEMU processes behind.
Let's use QEMU's -pidfile and use it to kill the process when QMP caps
probing is done.

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 20b350add7021a29c9898db6c0e9a25c2e631e55..759f5bb9686264518dc232a22e7ee6d76cd55d73 100644 (file)
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -29,6 +29,8 @@
 #include "virterror_internal.h"
 #include "util.h"
 #include "virfile.h"
+#include "virpidfile.h"
+#include "virprocess.h"
 #include "nodeinfo.h"
 #include "cpu/cpu.h"
 #include "domain_conf.h"
@@ -214,6 +216,7 @@ struct _qemuCapsCache {
     virMutex lock;
     virHashTablePtr binaries;
     char *libDir;
+    char *runDir;
 };
 
 
@@ -2180,7 +2183,8 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
 
 static int
 qemuCapsInitQMP(qemuCapsPtr caps,
-                const char *libDir)
+                const char *libDir,
+                const char *runDir)
 {
     int ret = -1;
     virCommandPtr cmd = NULL;
@@ -2191,7 +2195,11 @@ qemuCapsInitQMP(qemuCapsPtr caps,
     virDomainChrSourceDef config;
     char *monarg = NULL;
     char *monpath = NULL;
+    char *pidfile = NULL;
 
+    /* the ".sock" sufix is important to avoid a possible clash with a qemu
+     * domain called "capabilities"
+     */
     if (virAsprintf(&monpath, "%s/%s", libDir, "capabilities.monitor.sock") < 0) {
         virReportOOMError();
         goto cleanup;
@@ -2201,6 +2209,14 @@ qemuCapsInitQMP(qemuCapsPtr caps,
         goto cleanup;
     }
 
+    /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
+     * with a qemu domain called "capabilities"
+     */
+    if (virAsprintf(&pidfile, "%s/%s", runDir, "capabilities.pidfile") < 0) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
     memset(&config, 0, sizeof(config));
     config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
     config.data.nix.path = monpath;
@@ -2215,6 +2231,7 @@ qemuCapsInitQMP(qemuCapsPtr caps,
                                "-nographic",
                                "-M", "none",
                                "-qmp", monarg,
+                               "-pidfile", pidfile,
                                "-daemonize",
                                NULL);
     virCommandAddEnvPassCommon(cmd);
@@ -2300,12 +2317,32 @@ cleanup:
     virCommandFree(cmd);
     VIR_FREE(monarg);
     VIR_FREE(monpath);
+
+    if (pidfile) {
+        char ebuf[1024];
+        pid_t pid;
+        int rc;
+
+        if ((rc = virPidFileReadPath(pidfile, &pid)) < 0) {
+            VIR_DEBUG("Failed to read pidfile %s: %d",
+                      pidfile, virStrerror(-rc, ebuf, sizeof(ebuf)));
+        } else {
+            VIR_DEBUG("Killing QMP caps process %lld", (long long) pid);
+            if (virProcessKill(pid, SIGKILL) < 0 && errno != ESRCH)
+                VIR_ERROR(_("Failed to kill process %lld: %s"),
+                          (long long) pid,
+                          virStrerror(errno, ebuf, sizeof(ebuf)));
+        }
+        unlink(pidfile);
+        VIR_FREE(pidfile);
+    }
     return ret;
 }
 
 
 qemuCapsPtr qemuCapsNewForBinary(const char *binary,
-                                 const char *libDir)
+                                 const char *libDir,
+                                 const char *runDir)
 {
     qemuCapsPtr caps = qemuCapsNew();
     struct stat sb;
@@ -2333,7 +2370,7 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary,
         goto error;
     }
 
-    if ((rv = qemuCapsInitQMP(caps, libDir)) < 0)
+    if ((rv = qemuCapsInitQMP(caps, libDir, runDir)) < 0)
         goto error;
 
     if (!caps->usedQMP &&
@@ -2373,7 +2410,7 @@ qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED)
 
 
 qemuCapsCachePtr
-qemuCapsCacheNew(const char *libDir)
+qemuCapsCacheNew(const char *libDir, const char *runDir)
 {
     qemuCapsCachePtr cache;
 
@@ -2391,7 +2428,8 @@ qemuCapsCacheNew(const char *libDir)
 
     if (!(cache->binaries = virHashCreate(10, qemuCapsHashDataFree)))
         goto error;
-    if (!(cache->libDir = strdup(libDir))) {
+    if (!(cache->libDir = strdup(libDir)) ||
+        !(cache->runDir = strdup(runDir))) {
         virReportOOMError();
         goto error;
     }
@@ -2420,7 +2458,7 @@ qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary)
     if (!ret) {
         VIR_DEBUG("Creating capabilities for %s",
                   binary);
-        ret = qemuCapsNewForBinary(binary, cache->libDir);
+        ret = qemuCapsNewForBinary(binary, cache->libDir, cache->runDir);
         if (ret) {
             VIR_DEBUG("Caching capabilities %p for %s",
                       ret, binary);
@@ -2459,6 +2497,7 @@ qemuCapsCacheFree(qemuCapsCachePtr cache)
         return;
 
     VIR_FREE(cache->libDir);
+    VIR_FREE(cache->runDir);
     virHashFree(cache->binaries);
     virMutexDestroy(&cache->lock);
     VIR_FREE(cache);

diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 89f351c2085b3bdbf619ad36cc06343aba0ba1e5..5d343c15ad40a9d06cc7ce293467d2ef240077fc 100644 (file)
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -163,7 +163,8 @@ typedef qemuCapsCache *qemuCapsCachePtr;
 qemuCapsPtr qemuCapsNew(void);
 qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps);
 qemuCapsPtr qemuCapsNewForBinary(const char *binary,
-                                 const char *libDir);
+                                 const char *libDir,
+                                 const char *runDir);
 
 int qemuCapsProbeQMP(qemuCapsPtr caps,
                      qemuMonitorPtr mon);
@@ -201,7 +202,7 @@ int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps,
 bool qemuCapsIsValid(qemuCapsPtr caps);
 
 
-qemuCapsCachePtr qemuCapsCacheNew(const char *libDir);
+qemuCapsCachePtr qemuCapsCacheNew(const char *libDir, const char *runDir);
 qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary);
 qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary);
 void qemuCapsCacheFree(qemuCapsCachePtr cache);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 50f44ce70bebdcd1b16205ac72ff6055a443c133..97ad23e87691485f53aca306a00d05ff008e30e6 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -767,7 +767,9 @@ qemudStartup(int privileged) {
     if (qemuSecurityInit(qemu_driver) < 0)
         goto error;
 
-    if ((qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir)) == NULL)
+    qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir,
+                                              qemu_driver->stateDir);
+    if (!qemu_driver->capsCache)
         goto error;
 
     if ((qemu_driver->caps = qemuCreateCapabilities(qemu_driver)) == NULL)