[libvirt] [PATCH v5 01/36] qemu_process: Move process code from qemu_capabilities to qemu_process

Chris Venteicher posted 36 patches 7 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH v5 01/36] qemu_process: Move process code from qemu_capabilities to qemu_process
Posted by Chris Venteicher 7 years, 2 months ago
Qemu process code in qemu_capabilities.c is moved to qemu_process.c in
order to make the code usable outside the original capabilities
usecases.

The moved code activates and manages Qemu processes without
establishing a guest domain.

** This patch is a straight cut/paste move between files. **

(Exception: I did change a function prototype in qemu_process.h to be one
 parameter per line.)

Then, subsequent patches modify the process code
to make function prefixes and variable names match qemu_process,
and make the code usable for more than the capabilities usecase.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
---
 src/qemu/qemu_capabilities.c | 218 +----------------------------------
 src/qemu/qemu_process.c      | 199 ++++++++++++++++++++++++++++++++
 src/qemu/qemu_process.h      |  30 +++++
 3 files changed, 230 insertions(+), 217 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 20a1a0c201..14b1ab5bec 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -47,6 +47,7 @@
 #define __QEMU_CAPSPRIV_H_ALLOW__
 #include "qemu_capspriv.h"
 #include "qemu_qapi.h"
+#include "qemu_process.h"
 
 #include <fcntl.h>
 #include <sys/stat.h>
@@ -3939,18 +3940,6 @@ virQEMUCapsIsValid(void *data,
 }
 
 
-static void virQEMUCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
-                                     virDomainObjPtr vm ATTRIBUTE_UNUSED,
-                                     void *opaque ATTRIBUTE_UNUSED)
-{
-}
-
-static qemuMonitorCallbacks callbacks = {
-    .eofNotify = virQEMUCapsMonitorNotify,
-    .errorNotify = virQEMUCapsMonitorNotify,
-};
-
-
 /**
  * virQEMUCapsInitQMPArch:
  * @qemuCaps: QEMU capabilities
@@ -4245,211 +4234,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
 }
 
 
-typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
-typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
-struct _virQEMUCapsInitQMPCommand {
-    char *binary;
-    uid_t runUid;
-    gid_t runGid;
-    char **qmperr;
-    char *monarg;
-    char *monpath;
-    char *pidfile;
-    virCommandPtr cmd;
-    qemuMonitorPtr mon;
-    virDomainChrSourceDef config;
-    pid_t pid;
-    virDomainObjPtr vm;
-};
-
-
-static void
-virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
-{
-    if (cmd->mon)
-        virObjectUnlock(cmd->mon);
-    qemuMonitorClose(cmd->mon);
-    cmd->mon = NULL;
-
-    virCommandAbort(cmd->cmd);
-    virCommandFree(cmd->cmd);
-    cmd->cmd = NULL;
-
-    if (cmd->monpath)
-        unlink(cmd->monpath);
-
-    virDomainObjEndAPI(&cmd->vm);
-
-    if (cmd->pid != 0) {
-        char ebuf[1024];
-
-        VIR_DEBUG("Killing QMP caps process %lld", (long long)cmd->pid);
-        if (virProcessKill(cmd->pid, SIGKILL) < 0 && errno != ESRCH)
-            VIR_ERROR(_("Failed to kill process %lld: %s"),
-                      (long long)cmd->pid,
-                      virStrerror(errno, ebuf, sizeof(ebuf)));
-
-        VIR_FREE(*cmd->qmperr);
-    }
-    if (cmd->pidfile)
-        unlink(cmd->pidfile);
-    cmd->pid = 0;
-}
-
-
-static void
-virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
-{
-    if (!cmd)
-        return;
-
-    virQEMUCapsInitQMPCommandAbort(cmd);
-    VIR_FREE(cmd->binary);
-    VIR_FREE(cmd->monpath);
-    VIR_FREE(cmd->monarg);
-    VIR_FREE(cmd->pidfile);
-    VIR_FREE(cmd);
-}
-
-
-static virQEMUCapsInitQMPCommandPtr
-virQEMUCapsInitQMPCommandNew(char *binary,
-                             const char *libDir,
-                             uid_t runUid,
-                             gid_t runGid,
-                             char **qmperr)
-{
-    virQEMUCapsInitQMPCommandPtr cmd = NULL;
-
-    if (VIR_ALLOC(cmd) < 0)
-        goto error;
-
-    if (VIR_STRDUP(cmd->binary, binary) < 0)
-        goto error;
-
-    cmd->runUid = runUid;
-    cmd->runGid = runGid;
-    cmd->qmperr = qmperr;
-
-    /* the ".sock" sufix is important to avoid a possible clash with a qemu
-     * domain called "capabilities"
-     */
-    if (virAsprintf(&cmd->monpath, "%s/%s", libDir,
-                    "capabilities.monitor.sock") < 0)
-        goto error;
-    if (virAsprintf(&cmd->monarg, "unix:%s,server,nowait", cmd->monpath) < 0)
-        goto error;
-
-    /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
-     * with a qemu domain called "capabilities"
-     * Normally we'd use runDir for pid files, but because we're using
-     * -daemonize we need QEMU to be allowed to create them, rather
-     * than libvirtd. So we're using libDir which QEMU can write to
-     */
-    if (virAsprintf(&cmd->pidfile, "%s/%s", libDir, "capabilities.pidfile") < 0)
-        goto error;
-
-    virPidFileForceCleanupPath(cmd->pidfile);
-
-    cmd->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-    cmd->config.data.nix.path = cmd->monpath;
-    cmd->config.data.nix.listen = false;
-
-    return cmd;
-
- error:
-    virQEMUCapsInitQMPCommandFree(cmd);
-    return NULL;
-}
-
-
-/* Returns -1 on fatal error,
- *          0 on success,
- *          1 when probing QEMU failed
- */
-static int
-virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
-                             bool forceTCG)
-{
-    virDomainXMLOptionPtr xmlopt = NULL;
-    const char *machine;
-    int status = 0;
-    int ret = -1;
-
-    if (forceTCG)
-        machine = "none,accel=tcg";
-    else
-        machine = "none,accel=kvm:tcg";
-
-    VIR_DEBUG("Try to probe capabilities of '%s' via QMP, machine %s",
-              cmd->binary, machine);
-
-    /*
-     * We explicitly need to use -daemonize here, rather than
-     * virCommandDaemonize, because we need to synchronize
-     * with QEMU creating its monitor socket API. Using
-     * daemonize guarantees control won't return to libvirt
-     * until the socket is present.
-     */
-    cmd->cmd = virCommandNewArgList(cmd->binary,
-                                    "-S",
-                                    "-no-user-config",
-                                    "-nodefaults",
-                                    "-nographic",
-                                    "-machine", machine,
-                                    "-qmp", cmd->monarg,
-                                    "-pidfile", cmd->pidfile,
-                                    "-daemonize",
-                                    NULL);
-    virCommandAddEnvPassCommon(cmd->cmd);
-    virCommandClearCaps(cmd->cmd);
-    virCommandSetGID(cmd->cmd, cmd->runGid);
-    virCommandSetUID(cmd->cmd, cmd->runUid);
-
-    virCommandSetErrorBuffer(cmd->cmd, cmd->qmperr);
-
-    /* Log, but otherwise ignore, non-zero status.  */
-    if (virCommandRun(cmd->cmd, &status) < 0)
-        goto cleanup;
-
-    if (status != 0) {
-        VIR_DEBUG("QEMU %s exited with status %d: %s",
-                  cmd->binary, status, *cmd->qmperr);
-        goto ignore;
-    }
-
-    if (virPidFileReadPath(cmd->pidfile, &cmd->pid) < 0) {
-        VIR_DEBUG("Failed to read pidfile %s", cmd->pidfile);
-        goto ignore;
-    }
-
-    if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
-        !(cmd->vm = virDomainObjNew(xmlopt)))
-        goto cleanup;
-
-    cmd->vm->pid = cmd->pid;
-
-    if (!(cmd->mon = qemuMonitorOpen(cmd->vm, &cmd->config, true, true,
-                                     0, &callbacks, NULL)))
-        goto ignore;
-
-    virObjectLock(cmd->mon);
-
-    ret = 0;
-
- cleanup:
-    if (!cmd->mon)
-        virQEMUCapsInitQMPCommandAbort(cmd);
-    virObjectUnref(xmlopt);
-
-    return ret;
-
- ignore:
-    ret = 1;
-    goto cleanup;
-}
-
-
 static int
 virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
                    const char *libDir,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 12d1fca0d4..2cc52d3394 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8093,3 +8093,202 @@ qemuProcessReconnectAll(virQEMUDriverPtr driver)
     struct qemuProcessReconnectData data = {.driver = driver};
     virDomainObjListForEach(driver->domains, qemuProcessReconnectHelper, &data);
 }
+
+
+static void virQEMUCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+                                     virDomainObjPtr vm ATTRIBUTE_UNUSED,
+                                     void *opaque ATTRIBUTE_UNUSED)
+{
+}
+
+static qemuMonitorCallbacks callbacks = {
+    .eofNotify = virQEMUCapsMonitorNotify,
+    .errorNotify = virQEMUCapsMonitorNotify,
+};
+
+
+void
+virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
+{
+    if (!cmd)
+        return;
+
+    virQEMUCapsInitQMPCommandAbort(cmd);
+    VIR_FREE(cmd->binary);
+    VIR_FREE(cmd->monpath);
+    VIR_FREE(cmd->monarg);
+    VIR_FREE(cmd->pidfile);
+    VIR_FREE(cmd);
+}
+
+
+virQEMUCapsInitQMPCommandPtr
+virQEMUCapsInitQMPCommandNew(char *binary,
+                             const char *libDir,
+                             uid_t runUid,
+                             gid_t runGid,
+                             char **qmperr)
+{
+    virQEMUCapsInitQMPCommandPtr cmd = NULL;
+
+    if (VIR_ALLOC(cmd) < 0)
+        goto error;
+
+    if (VIR_STRDUP(cmd->binary, binary) < 0)
+        goto error;
+
+    cmd->runUid = runUid;
+    cmd->runGid = runGid;
+    cmd->qmperr = qmperr;
+
+    /* the ".sock" sufix is important to avoid a possible clash with a qemu
+     * domain called "capabilities"
+     */
+    if (virAsprintf(&cmd->monpath, "%s/%s", libDir,
+                    "capabilities.monitor.sock") < 0)
+        goto error;
+    if (virAsprintf(&cmd->monarg, "unix:%s,server,nowait", cmd->monpath) < 0)
+        goto error;
+
+    /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
+     * with a qemu domain called "capabilities"
+     * Normally we'd use runDir for pid files, but because we're using
+     * -daemonize we need QEMU to be allowed to create them, rather
+     * than libvirtd. So we're using libDir which QEMU can write to
+     */
+    if (virAsprintf(&cmd->pidfile, "%s/%s", libDir, "capabilities.pidfile") < 0)
+        goto error;
+
+    virPidFileForceCleanupPath(cmd->pidfile);
+
+    cmd->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+    cmd->config.data.nix.path = cmd->monpath;
+    cmd->config.data.nix.listen = false;
+
+    return cmd;
+
+ error:
+    virQEMUCapsInitQMPCommandFree(cmd);
+    return NULL;
+}
+
+
+/* Returns -1 on fatal error,
+ *          0 on success,
+ *          1 when probing QEMU failed
+ */
+int
+virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
+                             bool forceTCG)
+{
+    virDomainXMLOptionPtr xmlopt = NULL;
+    const char *machine;
+    int status = 0;
+    int ret = -1;
+
+    if (forceTCG)
+        machine = "none,accel=tcg";
+    else
+        machine = "none,accel=kvm:tcg";
+
+    VIR_DEBUG("Try to probe capabilities of '%s' via QMP, machine %s",
+              cmd->binary, machine);
+
+    /*
+     * We explicitly need to use -daemonize here, rather than
+     * virCommandDaemonize, because we need to synchronize
+     * with QEMU creating its monitor socket API. Using
+     * daemonize guarantees control won't return to libvirt
+     * until the socket is present.
+     */
+    cmd->cmd = virCommandNewArgList(cmd->binary,
+                                    "-S",
+                                    "-no-user-config",
+                                    "-nodefaults",
+                                    "-nographic",
+                                    "-machine", machine,
+                                    "-qmp", cmd->monarg,
+                                    "-pidfile", cmd->pidfile,
+                                    "-daemonize",
+                                    NULL);
+    virCommandAddEnvPassCommon(cmd->cmd);
+    virCommandClearCaps(cmd->cmd);
+    virCommandSetGID(cmd->cmd, cmd->runGid);
+    virCommandSetUID(cmd->cmd, cmd->runUid);
+
+    virCommandSetErrorBuffer(cmd->cmd, cmd->qmperr);
+
+    /* Log, but otherwise ignore, non-zero status.  */
+    if (virCommandRun(cmd->cmd, &status) < 0)
+        goto cleanup;
+
+    if (status != 0) {
+        VIR_DEBUG("QEMU %s exited with status %d: %s",
+                  cmd->binary, status, *cmd->qmperr);
+        goto ignore;
+    }
+
+    if (virPidFileReadPath(cmd->pidfile, &cmd->pid) < 0) {
+        VIR_DEBUG("Failed to read pidfile %s", cmd->pidfile);
+        goto ignore;
+    }
+
+    if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
+        !(cmd->vm = virDomainObjNew(xmlopt)))
+        goto cleanup;
+
+    cmd->vm->pid = cmd->pid;
+
+    if (!(cmd->mon = qemuMonitorOpen(cmd->vm, &cmd->config, true, true,
+                                     0, &callbacks, NULL)))
+        goto ignore;
+
+    virObjectLock(cmd->mon);
+
+    ret = 0;
+
+ cleanup:
+    if (!cmd->mon)
+        virQEMUCapsInitQMPCommandAbort(cmd);
+    virObjectUnref(xmlopt);
+
+    return ret;
+
+ ignore:
+    ret = 1;
+    goto cleanup;
+}
+
+
+void
+virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
+{
+    if (cmd->mon)
+        virObjectUnlock(cmd->mon);
+    qemuMonitorClose(cmd->mon);
+    cmd->mon = NULL;
+
+    virCommandAbort(cmd->cmd);
+    virCommandFree(cmd->cmd);
+    cmd->cmd = NULL;
+
+    if (cmd->monpath)
+        unlink(cmd->monpath);
+
+    virDomainObjEndAPI(&cmd->vm);
+
+    if (cmd->pid != 0) {
+        char ebuf[1024];
+
+        VIR_DEBUG("Killing QMP caps process %lld", (long long)cmd->pid);
+        if (virProcessKill(cmd->pid, SIGKILL) < 0 && errno != ESRCH)
+            VIR_ERROR(_("Failed to kill process %lld: %s"),
+                      (long long)cmd->pid,
+                      virStrerror(errno, ebuf, sizeof(ebuf)));
+
+        VIR_FREE(*cmd->qmperr);
+    }
+    if (cmd->pidfile)
+        unlink(cmd->pidfile);
+    cmd->pid = 0;
+}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 2037467c94..5e6f7c76ac 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -214,4 +214,34 @@ int qemuProcessStartManagedPRDaemon(virDomainObjPtr vm);
 
 void qemuProcessKillManagedPRDaemon(virDomainObjPtr vm);
 
+typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
+typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
+struct _virQEMUCapsInitQMPCommand {
+    char *binary;
+    uid_t runUid;
+    gid_t runGid;
+    char **qmperr;
+    char *monarg;
+    char *monpath;
+    char *pidfile;
+    virCommandPtr cmd;
+    qemuMonitorPtr mon;
+    virDomainChrSourceDef config;
+    pid_t pid;
+    virDomainObjPtr vm;
+};
+
+virQEMUCapsInitQMPCommandPtr virQEMUCapsInitQMPCommandNew(char *binary,
+                                                          const char *libDir,
+                                                          uid_t runUid,
+                                                          gid_t runGid,
+                                                          char **qmperr);
+
+void virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd);
+
+int virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
+                                 bool forceTCG);
+
+void virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd);
+
 #endif /* __QEMU_PROCESS_H__ */
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 01/36] qemu_process: Move process code from qemu_capabilities to qemu_process
Posted by Jiri Denemark 7 years, 1 month ago
On Sun, Dec 02, 2018 at 23:09:55 -0600, Chris Venteicher wrote:
> Qemu process code in qemu_capabilities.c is moved to qemu_process.c in
> order to make the code usable outside the original capabilities
> usecases.
> 
> The moved code activates and manages Qemu processes without
> establishing a guest domain.
> 
> ** This patch is a straight cut/paste move between files. **

I don't think there's any need for the stars here.

> (Exception: I did change a function prototype in qemu_process.h to be one
>  parameter per line.)

There's really no exception. There were no prototypes at all in the
original code since the functions were used in the same file where they
were defined. While moving the code, you naturally had to add
prototypes. Please drop this part of the commit message.

> Then, subsequent patches modify the process code
> to make function prefixes and variable names match qemu_process,
> and make the code usable for more than the capabilities usecase.
> 
> Signed-off-by: Chris Venteicher <cventeic@redhat.com>

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list