[libvirt] [PATCH RFC 01/22] qemu_process: Move process code from qemu_capabilities to qemu_process

Chris Venteicher posted 22 patches 6 years ago
[libvirt] [PATCH RFC 01/22] qemu_process: Move process code from qemu_capabilities to qemu_process
Posted by Chris Venteicher 6 years 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.

This process code activates and manages Qemu processes without
establishing a guest domain.

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

Following patches modify the process code
making it more generic and consistent with qemu_process.

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

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ca5af3297..0f70fdf46d 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>
@@ -3917,18 +3918,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
@@ -4223,211 +4212,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 06a65b44e4..0b3922fa39 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8064,3 +8064,204 @@ 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..4ba3988e3d 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -214,4 +214,33 @@ 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 RFC 01/22] qemu_process: Move process code from qemu_capabilities to qemu_process
Posted by Jiri Denemark 6 years ago
On Sun, Nov 11, 2018 at 13:59:09 -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.
> 
> This process code activates and manages Qemu processes without
> establishing a guest domain.
> 
> This patch is a straight cut/paste move between files.
> 
> Following patches modify the process code
> making it more generic and consistent with qemu_process.
> 
> Signed-off-by: Chris Venteicher <cventeic@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 218 +----------------------------------
>  src/qemu/qemu_process.c      | 201 ++++++++++++++++++++++++++++++++
>  src/qemu/qemu_process.h      |  29 +++++
>  3 files changed, 231 insertions(+), 217 deletions(-)
> 
...
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 06a65b44e4..0b3922fa39 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8064,3 +8064,204 @@ 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,
> +};
> +
> +
> +
> +

Two empty lines would be enough.

> +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);
> +}
...
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index 2037467c94..4ba3988e3d 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -214,4 +214,33 @@ 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);

Each parameter on its own line, please.

Jirka

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