[libvirt] [PATCH v7 00/20] Refactor virQEMUCapsInitQMPCommand (for BaselineHypervisorCPU using QEMU QMP exchanges)
3:04 a.m.
This series is a replacement for the refactoring part (the first 22
patches) of the "BaselineHypervisorCPU using QEMU QMP exchanges" series
[1] from Chris.
In a nutshell this series moves virQEMUCapsInitQMPCommand related code
to qemu_process.c and refactors it so that it can be used by both the
QEMU capabilities probing code and the upcoming code for calling QMP
commands internally for some CPU related APIs on s390.
I'm sending this refactoring part separated because it's mostly ready
for being pushed. I ACKed most of the patches in v6, but some patches
needed to be modified a bit or replaced by a bit different code. Thus
I'm sending them for review rather than pushing this part right away.
See individual patches for changes since v6.
[1] https://www.redhat.com/archives/libvir-list/2019-January/msg00310.html
Chris Venteicher (15):
qemu_process: Move process code from qemu_capabilities
qemu_process: Rename identifiers moved from qemu_capabilities
qemu_process: Limit qemuProcessQMPNew to const input strings
qemu_process: Refer to proc not cmd in process code
qemu_process: Use consistent name for stop process function
qemu_capabilities: Stop QEMU process before freeing
qemu_process: Use qemuProcessQMP struct for a single process
qemu_process: Introduce qemuProcessQMPStart
qemu_process: Move monitor code to qemuProcessQMPConnectMonitor
qemu_process: Store libDir in qemuProcessQMP struct
qemu_process: Setup paths within qemuProcessQMPInit
qemu_process: Stop retaining monitor config in qemuProcessQMP
qemu_process: Document and cleanup qemuProcessQMPNew
qemu_process: Use unique directories for QMP processes
qemu_process: Enter QMP command mode when starting QEMU Process
Jiri Denemark (5):
qemu_capabilities: Refactor virQEMUCapsInitQMP
qemu_process: Don't ignore errors in virQEMUCapsInit
qemu_capabilities: Log probe failure in virQEMUCapsInitQMPSingle
qemu_process: Hide qmperr inside qemuProcessQMP
qemu_process: Hide qemuProcessQMPStop
src/qemu/qemu_capabilities.c | 356 ++++++-----------------------------
src/qemu/qemu_process.c | 324 +++++++++++++++++++++++++++++++
src/qemu/qemu_process.h | 29 +++
src/qemu/qemu_processpriv.h | 2 +
tests/qemucapabilitiestest.c | 9 +
5 files changed, 419 insertions(+), 301 deletions(-)
--
2.20.1
3:04 a.m.
New subject: [libvirt] [PATCH 01/20] qemu_process: Move process code from qemu_capabilities
From: Chris Venteicher <cventeic(a)redhat.com>
QEMU process code in qemu_capabilities.c is moved to qemu_process.c in
order to make the code usable outside the original capabilities use
cases.
The moved code activates and manages QEMU processes without establishing
a guest domain.
This patch is a straight cut/paste move between files.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- rebased
src/qemu/qemu_capabilities.c | 230 +----------------------------------
src/qemu/qemu_process.c | 211 ++++++++++++++++++++++++++++++++
src/qemu/qemu_process.h | 30 +++++
3 files changed, 242 insertions(+), 229 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9d9c8096ba..26f83eb1d3 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -45,6 +45,7 @@
#define LIBVIRT_QEMU_CAPSPRIV_H_ALLOW
#include "qemu_capspriv.h"
#include "qemu_qapi.h"
+#include "qemu_process.h"
#include <fcntl.h>
#include <sys/stat.h>
@@ -53,10 +54,6 @@
#include <stdarg.h>
#include <sys/utsname.h>
-#if WITH_CAPNG
-# include <cap-ng.h>
-#endif
-
#define VIR_FROM_THIS VIR_FROM_QEMU
VIR_LOG_INIT("qemu.qemu_capabilities");
@@ -4049,18 +4046,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
@@ -4369,219 +4354,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);
-
-#if WITH_CAPNG
- /* QEMU might run into permission issues, e.g. /dev/sev (0600), override
- * them just for the purpose of probing */
- if (geteuid() == 0)
- virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
-#endif
-
- 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 a62d89320b..c45a15a1c1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -34,6 +34,10 @@
#include <sys/utsname.h>
+#if WITH_CAPNG
+# include <cap-ng.h>
+#endif
+
#include "qemu_process.h"
#define LIBVIRT_QEMU_PROCESSPRIV_H_ALLOW
#include "qemu_processpriv.h"
@@ -8306,3 +8310,210 @@ 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);
+
+#if WITH_CAPNG
+ /* QEMU might run into permission issues, e.g. /dev/sev (0600), override
+ * them just for the purpose of probing */
+ if (geteuid() == 0)
+ virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
+#endif
+
+ 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 67a4d7139f..f76613a7f7 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 /* LIBVIRT_QEMU_PROCESS_H */
--
2.20.1
8:54 a.m.
New subject: [libvirt] [PATCH 01/20] qemu_process: Move process code from qemu_capabilities
On Tue, Feb 19, 2019 at 10:04:44AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
QEMU process code in qemu_capabilities.c is moved to qemu_process.c in
order to make the code usable outside the original capabilities use
cases.
The moved code activates and manages QEMU processes without establishing
a guest domain.
This patch is a straight cut/paste move between files.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- rebased
src/qemu/qemu_capabilities.c | 230 +----------------------------------
src/qemu/qemu_process.c | 211 ++++++++++++++++++++++++++++++++
src/qemu/qemu_process.h | 30 +++++
3 files changed, 242 insertions(+), 229 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
12:32 p.m.
New subject: [libvirt] [PATCH 01/20] qemu_process: Move process code from qemu_capabilities
On Tue, Feb 19, 2019 at 15:54:40 +0100, Ján Tomko wrote:
On Tue, Feb 19, 2019 at 10:04:44AM +0100, Jiri Denemark wrote:
>From: Chris Venteicher <cventeic(a)redhat.com>
>
>QEMU process code in qemu_capabilities.c is moved to qemu_process.c in
>order to make the code usable outside the original capabilities use
>cases.
>
>The moved code activates and manages QEMU processes without establishing
>a guest domain.
>
>This patch is a straight cut/paste move between files.
>
>Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
>Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
>Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
>---
>
>Notes:
> Version 7:
> - rebased
>
> src/qemu/qemu_capabilities.c | 230 +----------------------------------
> src/qemu/qemu_process.c | 211 ++++++++++++++++++++++++++++++++
> src/qemu/qemu_process.h | 30 +++++
> 3 files changed, 242 insertions(+), 229 deletions(-)
>
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Thanks for the reviews, especially when mailing list delivery is
unreliable again.
Jirka
3:04 a.m.
New subject: [libvirt] [PATCH 02/20] qemu_process: Rename identifiers moved from qemu_capabilities
From: Chris Venteicher <cventeic(a)redhat.com>
s/virQEMUCapsInitQMPCommand/qemuProcessQMP/
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 14 +++++++-------
src/qemu/qemu_process.c | 28 ++++++++++++++--------------
src/qemu/qemu_process.h | 24 ++++++++++++------------
3 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 26f83eb1d3..e7442ac010 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4361,15 +4361,15 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
gid_t runGid,
char **qmperr)
{
- virQEMUCapsInitQMPCommandPtr cmd = NULL;
+ qemuProcessQMPPtr cmd = NULL;
int ret = -1;
int rc;
- if (!(cmd = virQEMUCapsInitQMPCommandNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr)))
+ if (!(cmd = qemuProcessQMPNew(qemuCaps->binary, libDir,
+ runUid, runGid, qmperr)))
goto cleanup;
- if ((rc = virQEMUCapsInitQMPCommandRun(cmd, false)) != 0) {
+ if ((rc = qemuProcessQMPRun(cmd, false)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
@@ -4379,8 +4379,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
goto cleanup;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
- virQEMUCapsInitQMPCommandAbort(cmd);
- if ((rc = virQEMUCapsInitQMPCommandRun(cmd, true)) != 0) {
+ qemuProcessQMPAbort(cmd);
+ if ((rc = qemuProcessQMPRun(cmd, true)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
@@ -4393,7 +4393,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
ret = 0;
cleanup:
- virQEMUCapsInitQMPCommandFree(cmd);
+ qemuProcessQMPFree(cmd);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c45a15a1c1..05acdc6013 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8325,12 +8325,12 @@ static qemuMonitorCallbacks callbacks = {
void
-virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
+qemuProcessQMPFree(qemuProcessQMPPtr cmd)
{
if (!cmd)
return;
- virQEMUCapsInitQMPCommandAbort(cmd);
+ qemuProcessQMPAbort(cmd);
VIR_FREE(cmd->binary);
VIR_FREE(cmd->monpath);
VIR_FREE(cmd->monarg);
@@ -8339,14 +8339,14 @@ virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd)
}
-virQEMUCapsInitQMPCommandPtr
-virQEMUCapsInitQMPCommandNew(char *binary,
- const char *libDir,
- uid_t runUid,
- gid_t runGid,
- char **qmperr)
+qemuProcessQMPPtr
+qemuProcessQMPNew(char *binary,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ char **qmperr)
{
- virQEMUCapsInitQMPCommandPtr cmd = NULL;
+ qemuProcessQMPPtr cmd = NULL;
if (VIR_ALLOC(cmd) < 0)
goto error;
@@ -8385,7 +8385,7 @@ virQEMUCapsInitQMPCommandNew(char *binary,
return cmd;
error:
- virQEMUCapsInitQMPCommandFree(cmd);
+ qemuProcessQMPFree(cmd);
return NULL;
}
@@ -8395,8 +8395,8 @@ virQEMUCapsInitQMPCommandNew(char *binary,
* 1 when probing QEMU failed
*/
int
-virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
- bool forceTCG)
+qemuProcessQMPRun(qemuProcessQMPPtr cmd,
+ bool forceTCG)
{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
@@ -8474,7 +8474,7 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
cleanup:
if (!cmd->mon)
- virQEMUCapsInitQMPCommandAbort(cmd);
+ qemuProcessQMPAbort(cmd);
virObjectUnref(xmlopt);
return ret;
@@ -8486,7 +8486,7 @@ virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
void
-virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd)
+qemuProcessQMPAbort(qemuProcessQMPPtr cmd)
{
if (cmd->mon)
virObjectUnlock(cmd->mon);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index f76613a7f7..4843210f76 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -214,9 +214,9 @@ int qemuProcessStartManagedPRDaemon(virDomainObjPtr vm);
void qemuProcessKillManagedPRDaemon(virDomainObjPtr vm);
-typedef struct _virQEMUCapsInitQMPCommand virQEMUCapsInitQMPCommand;
-typedef virQEMUCapsInitQMPCommand *virQEMUCapsInitQMPCommandPtr;
-struct _virQEMUCapsInitQMPCommand {
+typedef struct _qemuProcessQMP qemuProcessQMP;
+typedef qemuProcessQMP *qemuProcessQMPPtr;
+struct _qemuProcessQMP {
char *binary;
uid_t runUid;
gid_t runGid;
@@ -231,17 +231,17 @@ struct _virQEMUCapsInitQMPCommand {
virDomainObjPtr vm;
};
-virQEMUCapsInitQMPCommandPtr virQEMUCapsInitQMPCommandNew(char *binary,
- const char *libDir,
- uid_t runUid,
- gid_t runGid,
- char **qmperr);
+qemuProcessQMPPtr qemuProcessQMPNew(char *binary,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ char **qmperr);
-void virQEMUCapsInitQMPCommandFree(virQEMUCapsInitQMPCommandPtr cmd);
+void qemuProcessQMPFree(qemuProcessQMPPtr cmd);
-int virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
- bool forceTCG);
+int qemuProcessQMPRun(qemuProcessQMPPtr cmd,
+ bool forceTCG);
-void virQEMUCapsInitQMPCommandAbort(virQEMUCapsInitQMPCommandPtr cmd);
+void qemuProcessQMPAbort(qemuProcessQMPPtr cmd);
#endif /* LIBVIRT_QEMU_PROCESS_H */
--
2.20.1
8:55 a.m.
New subject: [libvirt] [PATCH 02/20] qemu_process: Rename identifiers moved from qemu_capabilities
On Tue, Feb 19, 2019 at 10:04:45AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
s/virQEMUCapsInitQMPCommand/qemuProcessQMP/
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 14 +++++++-------
src/qemu/qemu_process.c | 28 ++++++++++++++--------------
src/qemu/qemu_process.h | 24 ++++++++++++------------
3 files changed, 33 insertions(+), 33 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 03/20] qemu_process: Limit qemuProcessQMPNew to const input strings
From: Chris Venteicher <cventeic(a)redhat.com>
Add the const qualifier on non modified strings
(string only copied inside qemuProcessQMPNew)
so that const strings can be used directly in calls to
qemuProcessQMPNew in future patches.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_process.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 05acdc6013..6b698c0622 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8340,7 +8340,7 @@ qemuProcessQMPFree(qemuProcessQMPPtr cmd)
qemuProcessQMPPtr
-qemuProcessQMPNew(char *binary,
+qemuProcessQMPNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 4843210f76..fe79873967 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -231,7 +231,7 @@ struct _qemuProcessQMP {
virDomainObjPtr vm;
};
-qemuProcessQMPPtr qemuProcessQMPNew(char *binary,
+qemuProcessQMPPtr qemuProcessQMPNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
--
2.20.1
8:57 a.m.
New subject: [libvirt] [PATCH 03/20] qemu_process: Limit qemuProcessQMPNew to const input strings
On Tue, Feb 19, 2019 at 10:04:46AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
Add the const qualifier on non modified strings
(string only copied inside qemuProcessQMPNew)
so that const strings can be used directly in calls to
qemuProcessQMPNew in future patches.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_process.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 04/20] qemu_process: Refer to proc not cmd in process code
From: Chris Venteicher <cventeic(a)redhat.com>
s/cmd/proc/ in process code imported from qemu_capabilities.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- s/cmd/proc/ in qemuProcessQMPRun prototype
src/qemu/qemu_capabilities.c | 18 ++---
src/qemu/qemu_process.c | 140 +++++++++++++++++------------------
src/qemu/qemu_process.h | 6 +-
3 files changed, 82 insertions(+), 82 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e7442ac010..aa6571457e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4361,39 +4361,39 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
gid_t runGid,
char **qmperr)
{
- qemuProcessQMPPtr cmd = NULL;
+ qemuProcessQMPPtr proc = NULL;
int ret = -1;
int rc;
- if (!(cmd = qemuProcessQMPNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr)))
+ if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
+ runUid, runGid, qmperr)))
goto cleanup;
- if ((rc = qemuProcessQMPRun(cmd, false)) != 0) {
+ if ((rc = qemuProcessQMPRun(proc, false)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
}
- if (virQEMUCapsInitQMPMonitor(qemuCaps, cmd->mon) < 0)
+ if (virQEMUCapsInitQMPMonitor(qemuCaps, proc->mon) < 0)
goto cleanup;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
- qemuProcessQMPAbort(cmd);
- if ((rc = qemuProcessQMPRun(cmd, true)) != 0) {
+ qemuProcessQMPAbort(proc);
+ if ((rc = qemuProcessQMPRun(proc, true)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
}
- if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, cmd->mon) < 0)
+ if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon) < 0)
goto cleanup;
}
ret = 0;
cleanup:
- qemuProcessQMPFree(cmd);
+ qemuProcessQMPFree(proc);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6b698c0622..8e45dbc0ce 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8325,17 +8325,17 @@ static qemuMonitorCallbacks callbacks = {
void
-qemuProcessQMPFree(qemuProcessQMPPtr cmd)
+qemuProcessQMPFree(qemuProcessQMPPtr proc)
{
- if (!cmd)
+ if (!proc)
return;
- qemuProcessQMPAbort(cmd);
- VIR_FREE(cmd->binary);
- VIR_FREE(cmd->monpath);
- VIR_FREE(cmd->monarg);
- VIR_FREE(cmd->pidfile);
- VIR_FREE(cmd);
+ qemuProcessQMPAbort(proc);
+ VIR_FREE(proc->binary);
+ VIR_FREE(proc->monpath);
+ VIR_FREE(proc->monarg);
+ VIR_FREE(proc->pidfile);
+ VIR_FREE(proc);
}
@@ -8346,25 +8346,25 @@ qemuProcessQMPNew(const char *binary,
gid_t runGid,
char **qmperr)
{
- qemuProcessQMPPtr cmd = NULL;
+ qemuProcessQMPPtr proc = NULL;
- if (VIR_ALLOC(cmd) < 0)
+ if (VIR_ALLOC(proc) < 0)
goto error;
- if (VIR_STRDUP(cmd->binary, binary) < 0)
+ if (VIR_STRDUP(proc->binary, binary) < 0)
goto error;
- cmd->runUid = runUid;
- cmd->runGid = runGid;
- cmd->qmperr = qmperr;
+ proc->runUid = runUid;
+ proc->runGid = runGid;
+ proc->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,
+ if (virAsprintf(&proc->monpath, "%s/%s", libDir,
"capabilities.monitor.sock") < 0)
goto error;
- if (virAsprintf(&cmd->monarg, "unix:%s,server,nowait",
cmd->monpath) < 0)
+ if (virAsprintf(&proc->monarg, "unix:%s,server,nowait",
proc->monpath) < 0)
goto error;
/* ".pidfile" suffix is used rather than ".pid" to avoid a
possible clash
@@ -8373,19 +8373,19 @@ qemuProcessQMPNew(const char *binary,
* -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)
+ if (virAsprintf(&proc->pidfile, "%s/%s", libDir,
"capabilities.pidfile") < 0)
goto error;
- virPidFileForceCleanupPath(cmd->pidfile);
+ virPidFileForceCleanupPath(proc->pidfile);
- cmd->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
- cmd->config.data.nix.path = cmd->monpath;
- cmd->config.data.nix.listen = false;
+ proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ proc->config.data.nix.path = proc->monpath;
+ proc->config.data.nix.listen = false;
- return cmd;
+ return proc;
error:
- qemuProcessQMPFree(cmd);
+ qemuProcessQMPFree(proc);
return NULL;
}
@@ -8395,7 +8395,7 @@ qemuProcessQMPNew(const char *binary,
* 1 when probing QEMU failed
*/
int
-qemuProcessQMPRun(qemuProcessQMPPtr cmd,
+qemuProcessQMPRun(qemuProcessQMPPtr proc,
bool forceTCG)
{
virDomainXMLOptionPtr xmlopt = NULL;
@@ -8409,7 +8409,7 @@ qemuProcessQMPRun(qemuProcessQMPPtr cmd,
machine = "none,accel=kvm:tcg";
VIR_DEBUG("Try to probe capabilities of '%s' via QMP, machine %s",
- cmd->binary, machine);
+ proc->binary, machine);
/*
* We explicitly need to use -daemonize here, rather than
@@ -8418,63 +8418,63 @@ qemuProcessQMPRun(qemuProcessQMPPtr cmd,
* 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",
+ proc->cmd = virCommandNewArgList(proc->binary,
+ "-S",
+ "-no-user-config",
+ "-nodefaults",
+ "-nographic",
+ "-machine", machine,
+ "-qmp", proc->monarg,
+ "-pidfile", proc->pidfile,
+ "-daemonize",
NULL);
- virCommandAddEnvPassCommon(cmd->cmd);
- virCommandClearCaps(cmd->cmd);
+ virCommandAddEnvPassCommon(proc->cmd);
+ virCommandClearCaps(proc->cmd);
#if WITH_CAPNG
/* QEMU might run into permission issues, e.g. /dev/sev (0600), override
* them just for the purpose of probing */
if (geteuid() == 0)
- virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
+ virCommandAllowCap(proc->cmd, CAP_DAC_OVERRIDE);
#endif
- virCommandSetGID(cmd->cmd, cmd->runGid);
- virCommandSetUID(cmd->cmd, cmd->runUid);
+ virCommandSetGID(proc->cmd, proc->runGid);
+ virCommandSetUID(proc->cmd, proc->runUid);
- virCommandSetErrorBuffer(cmd->cmd, cmd->qmperr);
+ virCommandSetErrorBuffer(proc->cmd, proc->qmperr);
/* Log, but otherwise ignore, non-zero status. */
- if (virCommandRun(cmd->cmd, &status) < 0)
+ if (virCommandRun(proc->cmd, &status) < 0)
goto cleanup;
if (status != 0) {
VIR_DEBUG("QEMU %s exited with status %d: %s",
- cmd->binary, status, *cmd->qmperr);
+ proc->binary, status, *proc->qmperr);
goto ignore;
}
- if (virPidFileReadPath(cmd->pidfile, &cmd->pid) < 0) {
- VIR_DEBUG("Failed to read pidfile %s", cmd->pidfile);
+ if (virPidFileReadPath(proc->pidfile, &proc->pid) < 0) {
+ VIR_DEBUG("Failed to read pidfile %s", proc->pidfile);
goto ignore;
}
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
- !(cmd->vm = virDomainObjNew(xmlopt)))
+ !(proc->vm = virDomainObjNew(xmlopt)))
goto cleanup;
- cmd->vm->pid = cmd->pid;
+ proc->vm->pid = proc->pid;
- if (!(cmd->mon = qemuMonitorOpen(cmd->vm, &cmd->config, true, true,
- 0, &callbacks, NULL)))
+ if (!(proc->mon = qemuMonitorOpen(proc->vm, &proc->config, true, true,
+ 0, &callbacks, NULL)))
goto ignore;
- virObjectLock(cmd->mon);
+ virObjectLock(proc->mon);
ret = 0;
cleanup:
- if (!cmd->mon)
- qemuProcessQMPAbort(cmd);
+ if (!proc->mon)
+ qemuProcessQMPAbort(proc);
virObjectUnref(xmlopt);
return ret;
@@ -8486,34 +8486,34 @@ qemuProcessQMPRun(qemuProcessQMPPtr cmd,
void
-qemuProcessQMPAbort(qemuProcessQMPPtr cmd)
+qemuProcessQMPAbort(qemuProcessQMPPtr proc)
{
- if (cmd->mon)
- virObjectUnlock(cmd->mon);
- qemuMonitorClose(cmd->mon);
- cmd->mon = NULL;
+ if (proc->mon)
+ virObjectUnlock(proc->mon);
+ qemuMonitorClose(proc->mon);
+ proc->mon = NULL;
- virCommandAbort(cmd->cmd);
- virCommandFree(cmd->cmd);
- cmd->cmd = NULL;
+ virCommandAbort(proc->cmd);
+ virCommandFree(proc->cmd);
+ proc->cmd = NULL;
- if (cmd->monpath)
- unlink(cmd->monpath);
+ if (proc->monpath)
+ unlink(proc->monpath);
- virDomainObjEndAPI(&cmd->vm);
+ virDomainObjEndAPI(&proc->vm);
- if (cmd->pid != 0) {
+ if (proc->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_DEBUG("Killing QMP caps process %lld", (long long)proc->pid);
+ if (virProcessKill(proc->pid, SIGKILL) < 0 && errno != ESRCH)
VIR_ERROR(_("Failed to kill process %lld: %s"),
- (long long)cmd->pid,
+ (long long)proc->pid,
virStrerror(errno, ebuf, sizeof(ebuf)));
- VIR_FREE(*cmd->qmperr);
+ VIR_FREE(*proc->qmperr);
}
- if (cmd->pidfile)
- unlink(cmd->pidfile);
- cmd->pid = 0;
+ if (proc->pidfile)
+ unlink(proc->pidfile);
+ proc->pid = 0;
}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index fe79873967..a480d006f9 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -237,11 +237,11 @@ qemuProcessQMPPtr qemuProcessQMPNew(const char *binary,
gid_t runGid,
char **qmperr);
-void qemuProcessQMPFree(qemuProcessQMPPtr cmd);
+void qemuProcessQMPFree(qemuProcessQMPPtr proc);
-int qemuProcessQMPRun(qemuProcessQMPPtr cmd,
+int qemuProcessQMPRun(qemuProcessQMPPtr proc,
bool forceTCG);
-void qemuProcessQMPAbort(qemuProcessQMPPtr cmd);
+void qemuProcessQMPAbort(qemuProcessQMPPtr proc);
#endif /* LIBVIRT_QEMU_PROCESS_H */
--
2.20.1
8:58 a.m.
New subject: [libvirt] [PATCH 04/20] qemu_process: Refer to proc not cmd in process code
On Tue, Feb 19, 2019 at 10:04:47AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
s/cmd/proc/ in process code imported from qemu_capabilities.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- s/cmd/proc/ in qemuProcessQMPRun prototype
src/qemu/qemu_capabilities.c | 18 ++---
src/qemu/qemu_process.c | 140 +++++++++++++++++------------------
src/qemu/qemu_process.h | 6 +-
3 files changed, 82 insertions(+), 82 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 05/20] qemu_process: Use consistent name for stop process function
From: Chris Venteicher <cventeic(a)redhat.com>
s/qemuProcessQMPAbort/qemuProcessQMPStop/ applied to change function
name used to stop QEMU processes in process code moved from
qemu_capabilities.
No functionality change.
The new name, qemuProcessQMPStop, is consistent with the existing
function qemuProcessStop used to stop Domain processes.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_process.c | 6 +++---
src/qemu/qemu_process.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index aa6571457e..d187e35a38 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4379,7 +4379,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
goto cleanup;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
- qemuProcessQMPAbort(proc);
+ qemuProcessQMPStop(proc);
if ((rc = qemuProcessQMPRun(proc, true)) != 0) {
if (rc == 1)
ret = 0;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8e45dbc0ce..d463a41ca0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8330,7 +8330,7 @@ qemuProcessQMPFree(qemuProcessQMPPtr proc)
if (!proc)
return;
- qemuProcessQMPAbort(proc);
+ qemuProcessQMPStop(proc);
VIR_FREE(proc->binary);
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
@@ -8474,7 +8474,7 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc,
cleanup:
if (!proc->mon)
- qemuProcessQMPAbort(proc);
+ qemuProcessQMPStop(proc);
virObjectUnref(xmlopt);
return ret;
@@ -8486,7 +8486,7 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc,
void
-qemuProcessQMPAbort(qemuProcessQMPPtr proc)
+qemuProcessQMPStop(qemuProcessQMPPtr proc)
{
if (proc->mon)
virObjectUnlock(proc->mon);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index a480d006f9..c59379facb 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -242,6 +242,6 @@ void qemuProcessQMPFree(qemuProcessQMPPtr proc);
int qemuProcessQMPRun(qemuProcessQMPPtr proc,
bool forceTCG);
-void qemuProcessQMPAbort(qemuProcessQMPPtr proc);
+void qemuProcessQMPStop(qemuProcessQMPPtr proc);
#endif /* LIBVIRT_QEMU_PROCESS_H */
--
2.20.1
8:59 a.m.
New subject: [libvirt] [PATCH 05/20] qemu_process: Use consistent name for stop process function
On Tue, Feb 19, 2019 at 10:04:48AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
s/qemuProcessQMPAbort/qemuProcessQMPStop/ applied to change function
name used to stop QEMU processes in process code moved from
qemu_capabilities.
No functionality change.
The new name, qemuProcessQMPStop, is consistent with the existing
function qemuProcessStop used to stop Domain processes.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_process.c | 6 +++---
src/qemu/qemu_process.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 06/20] qemu_capabilities: Stop QEMU process before freeing
From: Chris Venteicher <cventeic(a)redhat.com>
virQEMUCapsInitQMP now stops QEMU process in all execution paths,
before freeing the process structure.
The qemuProcessQMPStop function can be called multiple times without
problems... Won't attempt to stop processes and free resources multiple
times.
Follow the convention established in qemu_process of
1) alloc process structure
2) start process
3) use process
4) stop process
5) free process data structure
The process data structure persists after the process activation fails
or the process dies or is killed so stderr strings can be retrieved
until the process data structure is freed.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 1 +
src/qemu/qemu_process.c | 24 ++++++++++++++++--------
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d187e35a38..f578d4a5ae 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4393,6 +4393,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
ret = 0;
cleanup:
+ qemuProcessQMPStop(proc);
qemuProcessQMPFree(proc);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d463a41ca0..4ae2067782 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8488,14 +8488,20 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc,
void
qemuProcessQMPStop(qemuProcessQMPPtr proc)
{
- if (proc->mon)
- virObjectUnlock(proc->mon);
- qemuMonitorClose(proc->mon);
- proc->mon = NULL;
+ if (!proc)
+ return;
- virCommandAbort(proc->cmd);
- virCommandFree(proc->cmd);
- proc->cmd = NULL;
+ if (proc->mon) {
+ virObjectUnlock(proc->mon);
+ qemuMonitorClose(proc->mon);
+ proc->mon = NULL;
+ }
+
+ if (proc->cmd) {
+ virCommandAbort(proc->cmd);
+ virCommandFree(proc->cmd);
+ proc->cmd = NULL;
+ }
if (proc->monpath)
unlink(proc->monpath);
@@ -8512,8 +8518,10 @@ qemuProcessQMPStop(qemuProcessQMPPtr proc)
virStrerror(errno, ebuf, sizeof(ebuf)));
VIR_FREE(*proc->qmperr);
+
+ proc->pid = 0;
}
+
if (proc->pidfile)
unlink(proc->pidfile);
- proc->pid = 0;
}
--
2.20.1
9 a.m.
New subject: [libvirt] [PATCH 06/20] qemu_capabilities: Stop QEMU process before freeing
On Tue, Feb 19, 2019 at 10:04:49AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
virQEMUCapsInitQMP now stops QEMU process in all execution paths,
before freeing the process structure.
The qemuProcessQMPStop function can be called multiple times without
problems... Won't attempt to stop processes and free resources multiple
times.
Follow the convention established in qemu_process of
1) alloc process structure
2) start process
3) use process
4) stop process
5) free process data structure
The process data structure persists after the process activation fails
or the process dies or is killed so stderr strings can be retrieved
until the process data structure is freed.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 1 +
src/qemu/qemu_process.c | 24 ++++++++++++++++--------
2 files changed, 17 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 07/20] qemu_process: Use qemuProcessQMP struct for a single process
From: Chris Venteicher <cventeic(a)redhat.com>
In new process code, move from model where qemuProcessQMP struct can be
used to activate a series of Qemu processes to model where one
qemuProcessQMP struct is used for one and only one Qemu process.
By allowing only one process activation per qemuProcessQMP struct, the
struct can safely store process outputs like status and stderr, without
being overwritten, until qemuProcessQMPFree is called.
By doing this, process outputs like status and stderr can remain stored
in the qemuProcessQMP struct without being overwritten by subsequent
process activations.
The forceTCG parameter (use / don't use KVM) will be passed when the
qemuProcessQMP struct is initialized since the qemuProcessQMP struct
won't be reused.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 19 +++++++++++++++----
src/qemu/qemu_process.c | 9 +++++----
src/qemu/qemu_process.h | 7 ++++---
3 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f578d4a5ae..6716e62e4a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4362,14 +4362,15 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
char **qmperr)
{
qemuProcessQMPPtr proc = NULL;
+ qemuProcessQMPPtr procTCG = NULL;
int ret = -1;
int rc;
if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr)))
+ runUid, runGid, qmperr, false)))
goto cleanup;
- if ((rc = qemuProcessQMPRun(proc, false)) != 0) {
+ if ((rc = qemuProcessQMPRun(proc)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
@@ -4379,14 +4380,22 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
goto cleanup;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
+ /* The second QEMU process probes for TCG capabilities
+ * in case the first process reported KVM as enabled
+ * (otherwise the first one already reported TCG capabilities). */
+
qemuProcessQMPStop(proc);
- if ((rc = qemuProcessQMPRun(proc, true)) != 0) {
+
+ procTCG = qemuProcessQMPNew(qemuCaps->binary, libDir,
+ runUid, runGid, NULL, true);
+
+ if ((rc = qemuProcessQMPRun(procTCG)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
}
- if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon) < 0)
+ if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, procTCG->mon) < 0)
goto cleanup;
}
@@ -4394,7 +4403,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
cleanup:
qemuProcessQMPStop(proc);
+ qemuProcessQMPStop(procTCG);
qemuProcessQMPFree(proc);
+ qemuProcessQMPFree(procTCG);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4ae2067782..6c0f7165c7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8344,7 +8344,8 @@ qemuProcessQMPNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr)
+ char **qmperr,
+ bool forceTCG)
{
qemuProcessQMPPtr proc = NULL;
@@ -8357,6 +8358,7 @@ qemuProcessQMPNew(const char *binary,
proc->runUid = runUid;
proc->runGid = runGid;
proc->qmperr = qmperr;
+ proc->forceTCG = forceTCG;
/* the ".sock" sufix is important to avoid a possible clash with a qemu
* domain called "capabilities"
@@ -8395,15 +8397,14 @@ qemuProcessQMPNew(const char *binary,
* 1 when probing QEMU failed
*/
int
-qemuProcessQMPRun(qemuProcessQMPPtr proc,
- bool forceTCG)
+qemuProcessQMPRun(qemuProcessQMPPtr proc)
{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
int status = 0;
int ret = -1;
- if (forceTCG)
+ if (proc->forceTCG)
machine = "none,accel=tcg";
else
machine = "none,accel=kvm:tcg";
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c59379facb..46a0bd2475 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -229,18 +229,19 @@ struct _qemuProcessQMP {
virDomainChrSourceDef config;
pid_t pid;
virDomainObjPtr vm;
+ bool forceTCG;
};
qemuProcessQMPPtr qemuProcessQMPNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr);
+ char **qmperr,
+ bool forceTCG);
void qemuProcessQMPFree(qemuProcessQMPPtr proc);
-int qemuProcessQMPRun(qemuProcessQMPPtr proc,
- bool forceTCG);
+int qemuProcessQMPRun(qemuProcessQMPPtr proc);
void qemuProcessQMPStop(qemuProcessQMPPtr proc);
--
2.20.1
9:03 a.m.
New subject: [libvirt] [PATCH 07/20] qemu_process: Use qemuProcessQMP struct for a single process
On Tue, Feb 19, 2019 at 10:04:50AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
In new process code, move from model where qemuProcessQMP struct can be
used to activate a series of Qemu processes to model where one
qemuProcessQMP struct is used for one and only one Qemu process.
By allowing only one process activation per qemuProcessQMP struct, the
struct can safely store process outputs like status and stderr, without
being overwritten, until qemuProcessQMPFree is called.
By doing this, process outputs like status and stderr can remain stored
in the qemuProcessQMP struct without being overwritten by subsequent
process activations.
The forceTCG parameter (use / don't use KVM) will be passed when the
qemuProcessQMP struct is initialized since the qemuProcessQMP struct
won't be reused.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_capabilities.c | 19 +++++++++++++++----
src/qemu/qemu_process.c | 9 +++++----
src/qemu/qemu_process.h | 7 ++++---
3 files changed, 24 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 08/20] qemu_capabilities: Refactor virQEMUCapsInitQMP
The function contains two almost identical parts. Let's consolidate them
into a single helper function and call it twice.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- new patch
src/qemu/qemu_capabilities.c | 67 ++++++++++++++++++------------------
1 file changed, 34 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6716e62e4a..b695b23fcc 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4355,19 +4355,19 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
static int
-virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
- const char *libDir,
- uid_t runUid,
- gid_t runGid,
- char **qmperr)
+virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ char **qmperr,
+ bool onlyTCG)
{
qemuProcessQMPPtr proc = NULL;
- qemuProcessQMPPtr procTCG = NULL;
int ret = -1;
int rc;
if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr, false)))
+ runUid, runGid, qmperr, onlyTCG)))
goto cleanup;
if ((rc = qemuProcessQMPRun(proc)) != 0) {
@@ -4376,40 +4376,41 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
goto cleanup;
}
- if (virQEMUCapsInitQMPMonitor(qemuCaps, proc->mon) < 0)
- goto cleanup;
-
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
- /* The second QEMU process probes for TCG capabilities
- * in case the first process reported KVM as enabled
- * (otherwise the first one already reported TCG capabilities). */
-
- qemuProcessQMPStop(proc);
-
- procTCG = qemuProcessQMPNew(qemuCaps->binary, libDir,
- runUid, runGid, NULL, true);
-
- if ((rc = qemuProcessQMPRun(procTCG)) != 0) {
- if (rc == 1)
- ret = 0;
- goto cleanup;
- }
-
- if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, procTCG->mon) < 0)
- goto cleanup;
- }
-
- ret = 0;
+ if (onlyTCG)
+ ret = virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon);
+ else
+ ret = virQEMUCapsInitQMPMonitor(qemuCaps, proc->mon);
cleanup:
qemuProcessQMPStop(proc);
- qemuProcessQMPStop(procTCG);
qemuProcessQMPFree(proc);
- qemuProcessQMPFree(procTCG);
return ret;
}
+static int
+virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
+ const char *libDir,
+ uid_t runUid,
+ gid_t runGid,
+ char **qmperr)
+{
+ if (virQEMUCapsInitQMPSingle(qemuCaps, libDir, runUid, runGid, qmperr, false) <
0)
+ return -1;
+
+ /*
+ * If KVM was enabled during the first probe, we need to explicitly probe
+ * for TCG capabilities by asking the same binary again and turning KVM
+ * off.
+ */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
+ virQEMUCapsInitQMPSingle(qemuCaps, libDir, runUid, runGid, NULL, true) < 0)
+ return -1;
+
+ return 0;
+}
+
+
#define MESSAGE_ID_CAPS_PROBE_FAILURE "8ae2f3fb-2dbe-498e-8fbd-012d40afa361"
static void
--
2.20.1
9:09 a.m.
New subject: [libvirt] [PATCH 08/20] qemu_capabilities: Refactor virQEMUCapsInitQMP
On Tue, Feb 19, 2019 at 10:04:51AM +0100, Jiri Denemark wrote:
The function contains two almost identical parts. Let's
consolidate them
into a single helper function and call it twice.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- new patch
src/qemu/qemu_capabilities.c | 67 ++++++++++++++++++------------------
1 file changed, 34 insertions(+), 33 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 09/20] qemu_process: Don't ignore errors in virQEMUCapsInit
While qemuProcessQMPRun and virQEMUCapsInitQMPMonitor* functions called
from virQEMUCapsInit ignore some errors, the caller of virQEMUCapsInit
would report an error unless usedQMP is true anyway. And since usedQMP
can only be true if the probing code really succeeded (i.e., no errors
were ignored), we can just simplify the logic by not ignoring the errors
in the first place.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- this is a rewritten version of
[PATCH 08/33] qemu_process: All ProcessQMP errors are fatal
from version 6
src/qemu/qemu_capabilities.c | 17 +----------------
src/qemu/qemu_process.c | 27 +++++++++++----------------
2 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b695b23fcc..bb1920146e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4152,7 +4152,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (qemuMonitorSetCapabilities(mon) < 0) {
VIR_DEBUG("Failed to set monitor capabilities %s",
virGetLastErrorMessage());
- ret = 0;
goto cleanup;
}
@@ -4161,7 +4160,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
&package) < 0) {
VIR_DEBUG("Failed to query monitor version %s",
virGetLastErrorMessage());
- ret = 0;
goto cleanup;
}
@@ -4338,7 +4336,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
if (qemuMonitorSetCapabilities(mon) < 0) {
VIR_DEBUG("Failed to set monitor capabilities %s",
virGetLastErrorMessage());
- ret = 0;
goto cleanup;
}
@@ -4364,17 +4361,13 @@ virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps,
{
qemuProcessQMPPtr proc = NULL;
int ret = -1;
- int rc;
if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
runUid, runGid, qmperr, onlyTCG)))
goto cleanup;
- if ((rc = qemuProcessQMPRun(proc)) != 0) {
- if (rc == 1)
- ret = 0;
+ if (qemuProcessQMPRun(proc) < 0)
goto cleanup;
- }
if (onlyTCG)
ret = virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon);
@@ -4474,14 +4467,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
goto error;
}
- if (!qemuCaps->usedQMP) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to probe QEMU binary with QMP: %s"),
- qmperr ? qmperr : _("unknown error"));
- virQEMUCapsLogProbeFailure(binary);
- goto error;
- }
-
qemuCaps->libvirtCtime = virGetSelfLastChanged();
qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6c0f7165c7..728176bbdf 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8392,10 +8392,6 @@ qemuProcessQMPNew(const char *binary,
}
-/* Returns -1 on fatal error,
- * 0 on success,
- * 1 when probing QEMU failed
- */
int
qemuProcessQMPRun(qemuProcessQMPPtr proc)
{
@@ -8403,6 +8399,7 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc)
const char *machine;
int status = 0;
int ret = -1;
+ int rc;
if (proc->forceTCG)
machine = "none,accel=tcg";
@@ -8444,19 +8441,21 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc)
virCommandSetErrorBuffer(proc->cmd, proc->qmperr);
- /* Log, but otherwise ignore, non-zero status. */
if (virCommandRun(proc->cmd, &status) < 0)
goto cleanup;
if (status != 0) {
- VIR_DEBUG("QEMU %s exited with status %d: %s",
- proc->binary, status, *proc->qmperr);
- goto ignore;
+ VIR_DEBUG("QEMU %s exited with status %d", proc->binary, status);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to start QEMU binary %s for probing: %s"),
+ proc->binary,
+ *proc->qmperr ? *proc->qmperr : _("unknown
error"));
+ goto cleanup;
}
- if (virPidFileReadPath(proc->pidfile, &proc->pid) < 0) {
- VIR_DEBUG("Failed to read pidfile %s", proc->pidfile);
- goto ignore;
+ if ((rc = virPidFileReadPath(proc->pidfile, &proc->pid)) < 0) {
+ virReportSystemError(-rc, _("Failed to read pidfile %s"),
proc->pidfile);
+ goto cleanup;
}
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
@@ -8467,7 +8466,7 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc)
if (!(proc->mon = qemuMonitorOpen(proc->vm, &proc->config, true, true,
0, &callbacks, NULL)))
- goto ignore;
+ goto cleanup;
virObjectLock(proc->mon);
@@ -8479,10 +8478,6 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc)
virObjectUnref(xmlopt);
return ret;
-
- ignore:
- ret = 1;
- goto cleanup;
}
--
2.20.1
9:12 a.m.
New subject: [libvirt] [PATCH 09/20] qemu_process: Don't ignore errors in virQEMUCapsInit
On Tue, Feb 19, 2019 at 10:04:52AM +0100, Jiri Denemark wrote:
While qemuProcessQMPRun and virQEMUCapsInitQMPMonitor* functions
called
from virQEMUCapsInit ignore some errors, the caller of virQEMUCapsInit
would report an error unless usedQMP is true anyway. And since usedQMP
can only be true if the probing code really succeeded (i.e., no errors
were ignored), we can just simplify the logic by not ignoring the errors
in the first place.
Thank you.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- this is a rewritten version of
[PATCH 08/33] qemu_process: All ProcessQMP errors are fatal
from version 6
src/qemu/qemu_capabilities.c | 17 +----------------
src/qemu/qemu_process.c | 27 +++++++++++----------------
2 files changed, 12 insertions(+), 32 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 10/20] qemu_capabilities: Log probe failure in virQEMUCapsInitQMPSingle
Let's push the call to virQEMUCapsLogProbeFailure down the stack to
where the probing failure is detected.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- this patch was originally included in
[PATCH 09/33] qemu_process: Expose process exit status code
in version 6 with a bunch of other unrelated changes
- modified after virQEMUCapsInitQMP was refactored
src/qemu/qemu_capabilities.c | 47 ++++++++++++++++++------------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index bb1920146e..264dcae983 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4351,6 +4351,26 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
}
+#define MESSAGE_ID_CAPS_PROBE_FAILURE "8ae2f3fb-2dbe-498e-8fbd-012d40afa361"
+
+static void
+virQEMUCapsLogProbeFailure(const char *binary)
+{
+ virLogMetadata meta[] = {
+ { .key = "MESSAGE_ID", .s = MESSAGE_ID_CAPS_PROBE_FAILURE, .iv = 0 },
+ { .key = "LIBVIRT_QEMU_BINARY", .s = binary, .iv = 0 },
+ { .key = NULL },
+ };
+
+ virLogMessage(&virLogSelf,
+ VIR_LOG_WARN,
+ __FILE__, __LINE__, __func__,
+ meta,
+ _("Failed to probe capabilities for %s: %s"),
+ binary, virGetLastErrorMessage());
+}
+
+
static int
virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps,
const char *libDir,
@@ -4375,6 +4395,9 @@ virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps,
ret = virQEMUCapsInitQMPMonitor(qemuCaps, proc->mon);
cleanup:
+ if (ret < 0)
+ virQEMUCapsLogProbeFailure(qemuCaps->binary);
+
qemuProcessQMPStop(proc);
qemuProcessQMPFree(proc);
return ret;
@@ -4404,26 +4427,6 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
}
-#define MESSAGE_ID_CAPS_PROBE_FAILURE "8ae2f3fb-2dbe-498e-8fbd-012d40afa361"
-
-static void
-virQEMUCapsLogProbeFailure(const char *binary)
-{
- virLogMetadata meta[] = {
- { .key = "MESSAGE_ID", .s = MESSAGE_ID_CAPS_PROBE_FAILURE, .iv = 0 },
- { .key = "LIBVIRT_QEMU_BINARY", .s = binary, .iv = 0 },
- { .key = NULL },
- };
-
- virLogMessage(&virLogSelf,
- VIR_LOG_WARN,
- __FILE__, __LINE__, __func__,
- meta,
- _("Failed to probe capabilities for %s: %s"),
- binary, virGetLastErrorMessage());
-}
-
-
virQEMUCapsPtr
virQEMUCapsNewForBinaryInternal(virArch hostArch,
const char *binary,
@@ -4462,10 +4465,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
goto error;
}
- if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0) {
- virQEMUCapsLogProbeFailure(binary);
+ if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0)
goto error;
- }
qemuCaps->libvirtCtime = virGetSelfLastChanged();
qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER;
--
2.20.1
9:13 a.m.
New subject: [libvirt] [PATCH 10/20] qemu_capabilities: Log probe failure in virQEMUCapsInitQMPSingle
On Tue, Feb 19, 2019 at 10:04:53AM +0100, Jiri Denemark wrote:
Let's push the call to virQEMUCapsLogProbeFailure down the stack
to
where the probing failure is detected.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- this patch was originally included in
[PATCH 09/33] qemu_process: Expose process exit status code
in version 6 with a bunch of other unrelated changes
- modified after virQEMUCapsInitQMP was refactored
src/qemu/qemu_capabilities.c | 47 ++++++++++++++++++------------------
1 file changed, 24 insertions(+), 23 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 11/20] qemu_process: Hide qmperr inside qemuProcessQMP
Keep the pointer to QEMU stderr output in qemuProcessQMP struct instead
of requiring the caller to provide it (and free it).
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- this is a combination of the relevant parts of
[PATCH 09/33] qemu_process: Expose process exit status code
and
[PATCH 10/33] qemu_process: Persist stderr in qemuProcessQMP struct
adapted to the changes made in the previous patches
src/qemu/qemu_capabilities.c | 14 +++++---------
src/qemu/qemu_process.c | 9 +++------
src/qemu/qemu_process.h | 3 +--
3 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 264dcae983..7460b452a0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4376,14 +4376,13 @@ virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr,
bool onlyTCG)
{
qemuProcessQMPPtr proc = NULL;
int ret = -1;
if (!(proc = qemuProcessQMPNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr, onlyTCG)))
+ runUid, runGid, onlyTCG)))
goto cleanup;
if (qemuProcessQMPRun(proc) < 0)
@@ -4408,10 +4407,9 @@ static int
virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
const char *libDir,
uid_t runUid,
- gid_t runGid,
- char **qmperr)
+ gid_t runGid)
{
- if (virQEMUCapsInitQMPSingle(qemuCaps, libDir, runUid, runGid, qmperr, false) <
0)
+ if (virQEMUCapsInitQMPSingle(qemuCaps, libDir, runUid, runGid, false) < 0)
return -1;
/*
@@ -4420,7 +4418,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
* off.
*/
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
- virQEMUCapsInitQMPSingle(qemuCaps, libDir, runUid, runGid, NULL, true) < 0)
+ virQEMUCapsInitQMPSingle(qemuCaps, libDir, runUid, runGid, true) < 0)
return -1;
return 0;
@@ -4438,7 +4436,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
{
virQEMUCapsPtr qemuCaps;
struct stat sb;
- char *qmperr = NULL;
if (!(qemuCaps = virQEMUCapsNew()))
goto error;
@@ -4465,7 +4462,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
goto error;
}
- if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0)
+ if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0)
goto error;
qemuCaps->libvirtCtime = virGetSelfLastChanged();
@@ -4484,7 +4481,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
}
cleanup:
- VIR_FREE(qmperr);
return qemuCaps;
error:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 728176bbdf..d2859da2ec 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8335,6 +8335,7 @@ qemuProcessQMPFree(qemuProcessQMPPtr proc)
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
VIR_FREE(proc->pidfile);
+ VIR_FREE(proc->stderr);
VIR_FREE(proc);
}
@@ -8344,7 +8345,6 @@ qemuProcessQMPNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr,
bool forceTCG)
{
qemuProcessQMPPtr proc = NULL;
@@ -8357,7 +8357,6 @@ qemuProcessQMPNew(const char *binary,
proc->runUid = runUid;
proc->runGid = runGid;
- proc->qmperr = qmperr;
proc->forceTCG = forceTCG;
/* the ".sock" sufix is important to avoid a possible clash with a qemu
@@ -8439,7 +8438,7 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc)
virCommandSetGID(proc->cmd, proc->runGid);
virCommandSetUID(proc->cmd, proc->runUid);
- virCommandSetErrorBuffer(proc->cmd, proc->qmperr);
+ virCommandSetErrorBuffer(proc->cmd, &(proc->stderr));
if (virCommandRun(proc->cmd, &status) < 0)
goto cleanup;
@@ -8449,7 +8448,7 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to start QEMU binary %s for probing: %s"),
proc->binary,
- *proc->qmperr ? *proc->qmperr : _("unknown
error"));
+ proc->stderr ? proc->stderr : _("unknown
error"));
goto cleanup;
}
@@ -8513,8 +8512,6 @@ qemuProcessQMPStop(qemuProcessQMPPtr proc)
(long long)proc->pid,
virStrerror(errno, ebuf, sizeof(ebuf)));
- VIR_FREE(*proc->qmperr);
-
proc->pid = 0;
}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 46a0bd2475..e7d4ca0c92 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -220,7 +220,7 @@ struct _qemuProcessQMP {
char *binary;
uid_t runUid;
gid_t runGid;
- char **qmperr;
+ char *stderr;
char *monarg;
char *monpath;
char *pidfile;
@@ -236,7 +236,6 @@ qemuProcessQMPPtr qemuProcessQMPNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr,
bool forceTCG);
void qemuProcessQMPFree(qemuProcessQMPPtr proc);
--
2.20.1
9:14 a.m.
New subject: [libvirt] [PATCH 11/20] qemu_process: Hide qmperr inside qemuProcessQMP
On Tue, Feb 19, 2019 at 10:04:54AM +0100, Jiri Denemark wrote:
Keep the pointer to QEMU stderr output in qemuProcessQMP struct
instead
of requiring the caller to provide it (and free it).
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- this is a combination of the relevant parts of
[PATCH 09/33] qemu_process: Expose process exit status code
and
[PATCH 10/33] qemu_process: Persist stderr in qemuProcessQMP struct
adapted to the changes made in the previous patches
src/qemu/qemu_capabilities.c | 14 +++++---------
src/qemu/qemu_process.c | 9 +++------
src/qemu/qemu_process.h | 3 +--
3 files changed, 9 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 12/20] qemu_process: Introduce qemuProcessQMPStart
From: Chris Venteicher <cventeic(a)redhat.com>
This is a replacement for qemuProcessQMPRun to make the name consistent
with qemuProcessStart. The original qemuProcessQMPRun function is
renamed as qemuProcessQMPLaunch and becomes one of the simpler functions
called from the main qemuProcessQMPStart entry point. The following
patches will move parts of the code in qemuProcessQMPLaunch to the other
functions (qemuProcessQMPInit and qemuProcessQMPConnectMonitor).
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- removed some fairy tales from the commit message and the documentation
of qemuProcessQMPStart
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_process.c | 74 +++++++++++++++++++++++++++++++++++-
src/qemu/qemu_process.h | 2 +-
3 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7460b452a0..4fd2169d49 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4385,7 +4385,7 @@ virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps,
runUid, runGid, onlyTCG)))
goto cleanup;
- if (qemuProcessQMPRun(proc) < 0)
+ if (qemuProcessQMPStart(proc) < 0)
goto cleanup;
if (onlyTCG)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d2859da2ec..19bc804d18 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8391,8 +8391,21 @@ qemuProcessQMPNew(const char *binary,
}
-int
-qemuProcessQMPRun(qemuProcessQMPPtr proc)
+static int
+qemuProcessQMPInit(qemuProcessQMPPtr proc)
+{
+ int ret = -1;
+
+ VIR_DEBUG("proc=%p, emulator=%s", proc, proc->binary);
+
+ ret = 0;
+
+ return ret;
+}
+
+
+static int
+qemuProcessQMPLaunch(qemuProcessQMPPtr proc)
{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
@@ -8480,6 +8493,63 @@ qemuProcessQMPRun(qemuProcessQMPPtr proc)
}
+static int
+qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
+{
+ int ret = -1;
+
+ VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
+ proc, proc->binary, (long long)proc->pid);
+
+ ret = 0;
+
+ return ret;
+}
+
+
+/**
+ * qemuProcessQMPStart:
+ * @proc: QEMU process and connection state created by qemuProcessQMPNew()
+ *
+ * Start and connect to QEMU binary so QMP queries can be made.
+ *
+ * Usage:
+ * proc = qemuProcessQMPNew(binary, libDir, runUid, runGid, forceTCG);
+ * qemuProcessQMPStart(proc);
+ * ** Send QMP Queries to QEMU using monitor (proc->mon) **
+ * qemuProcessQMPStop(proc);
+ * qemuProcessQMPFree(proc);
+ *
+ * Process error output (proc->stderr) remains available in qemuProcessQMP
+ * struct until qemuProcessQMPFree is called.
+ */
+int
+qemuProcessQMPStart(qemuProcessQMPPtr proc)
+{
+ int ret = -1;
+
+ VIR_DEBUG("proc=%p, emulator=%s", proc, proc->binary);
+
+ if (qemuProcessQMPInit(proc) < 0)
+ goto cleanup;
+
+ if (qemuProcessQMPLaunch(proc) < 0)
+ goto stop;
+
+ if (qemuProcessQMPConnectMonitor(proc) < 0)
+ goto stop;
+
+ ret = 0;
+
+ cleanup:
+ return ret;
+
+ stop:
+ qemuProcessQMPStop(proc);
+ goto cleanup;
+}
+
+
void
qemuProcessQMPStop(qemuProcessQMPPtr proc)
{
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index e7d4ca0c92..c075ae6445 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -240,7 +240,7 @@ qemuProcessQMPPtr qemuProcessQMPNew(const char *binary,
void qemuProcessQMPFree(qemuProcessQMPPtr proc);
-int qemuProcessQMPRun(qemuProcessQMPPtr proc);
+int qemuProcessQMPStart(qemuProcessQMPPtr proc);
void qemuProcessQMPStop(qemuProcessQMPPtr proc);
--
2.20.1
9:18 a.m.
New subject: [libvirt] [PATCH 12/20] qemu_process: Introduce qemuProcessQMPStart
On Tue, Feb 19, 2019 at 10:04:55AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
This is a replacement for qemuProcessQMPRun to make the name consistent
with qemuProcessStart. The original qemuProcessQMPRun function is
renamed as qemuProcessQMPLaunch and becomes one of the simpler functions
called from the main qemuProcessQMPStart entry point. The following
patches will move parts of the code in qemuProcessQMPLaunch to the other
functions (qemuProcessQMPInit and qemuProcessQMPConnectMonitor).
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- removed some fairy tales from the commit message and the documentation
Thank you.
of qemuProcessQMPStart
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_process.c | 74 +++++++++++++++++++++++++++++++++++-
src/qemu/qemu_process.h | 2 +-
3 files changed, 74 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 13/20] qemu_process: Move monitor code to qemuProcessQMPConnectMonitor
From: Chris Venteicher <cventeic(a)redhat.com>
All code related to QEMU monitor is moved from qemuProcessQMPNew and
qemuProcessQMPInit into qemuProcessQMPConnectMonitor.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- simplified commit message
- adapted to changes in the preceding patches
src/qemu/qemu_process.c | 42 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 19bc804d18..85079c8c15 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8379,10 +8379,6 @@ qemuProcessQMPNew(const char *binary,
virPidFileForceCleanupPath(proc->pidfile);
- proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
- proc->config.data.nix.path = proc->monpath;
- proc->config.data.nix.listen = false;
-
return proc;
error:
@@ -8407,7 +8403,6 @@ qemuProcessQMPInit(qemuProcessQMPPtr proc)
static int
qemuProcessQMPLaunch(qemuProcessQMPPtr proc)
{
- virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
int status = 0;
int ret = -1;
@@ -8470,6 +8465,26 @@ qemuProcessQMPLaunch(qemuProcessQMPPtr proc)
goto cleanup;
}
+ ret = 0;
+
+ cleanup:
+ return ret;
+}
+
+
+static int
+qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
+{
+ virDomainXMLOptionPtr xmlopt = NULL;
+ int ret = -1;
+
+ VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
+ proc, proc->binary, (long long)proc->pid);
+
+ proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ proc->config.data.nix.path = proc->monpath;
+ proc->config.data.nix.listen = false;
+
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
!(proc->vm = virDomainObjNew(xmlopt)))
goto cleanup;
@@ -8485,24 +8500,7 @@ qemuProcessQMPLaunch(qemuProcessQMPPtr proc)
ret = 0;
cleanup:
- if (!proc->mon)
- qemuProcessQMPStop(proc);
virObjectUnref(xmlopt);
-
- return ret;
-}
-
-
-static int
-qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
-{
- int ret = -1;
-
- VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
- proc, proc->binary, (long long)proc->pid);
-
- ret = 0;
-
return ret;
}
--
2.20.1
9:21 a.m.
New subject: [libvirt] [PATCH 13/20] qemu_process: Move monitor code to qemuProcessQMPConnectMonitor
On Tue, Feb 19, 2019 at 10:04:56AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
All code related to QEMU monitor is moved from qemuProcessQMPNew and
qemuProcessQMPInit into qemuProcessQMPConnectMonitor.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- simplified commit message
- adapted to changes in the preceding patches
src/qemu/qemu_process.c | 42 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 14/20] qemu_process: Store libDir in qemuProcessQMP struct
From: Chris Venteicher <cventeic(a)redhat.com>
Store libDir path in the qemuProcessQMP struct in anticipation of moving
path construction code into qemuProcessQMPInit function.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 8 +++++---
src/qemu/qemu_process.h | 1 +
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 85079c8c15..c4cbd7c807 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8332,6 +8332,7 @@ qemuProcessQMPFree(qemuProcessQMPPtr proc)
qemuProcessQMPStop(proc);
VIR_FREE(proc->binary);
+ VIR_FREE(proc->libDir);
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
VIR_FREE(proc->pidfile);
@@ -8352,7 +8353,8 @@ qemuProcessQMPNew(const char *binary,
if (VIR_ALLOC(proc) < 0)
goto error;
- if (VIR_STRDUP(proc->binary, binary) < 0)
+ if (VIR_STRDUP(proc->binary, binary) < 0 ||
+ VIR_STRDUP(proc->libDir, libDir) < 0)
goto error;
proc->runUid = runUid;
@@ -8362,7 +8364,7 @@ qemuProcessQMPNew(const char *binary,
/* the ".sock" sufix is important to avoid a possible clash with a qemu
* domain called "capabilities"
*/
- if (virAsprintf(&proc->monpath, "%s/%s", libDir,
+ if (virAsprintf(&proc->monpath, "%s/%s", proc->libDir,
"capabilities.monitor.sock") < 0)
goto error;
if (virAsprintf(&proc->monarg, "unix:%s,server,nowait",
proc->monpath) < 0)
@@ -8374,7 +8376,7 @@ qemuProcessQMPNew(const char *binary,
* -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(&proc->pidfile, "%s/%s", libDir,
"capabilities.pidfile") < 0)
+ if (virAsprintf(&proc->pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
goto error;
virPidFileForceCleanupPath(proc->pidfile);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c075ae6445..9ab885c6c3 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -218,6 +218,7 @@ typedef struct _qemuProcessQMP qemuProcessQMP;
typedef qemuProcessQMP *qemuProcessQMPPtr;
struct _qemuProcessQMP {
char *binary;
+ char *libDir;
uid_t runUid;
gid_t runGid;
char *stderr;
--
2.20.1
9:22 a.m.
New subject: [libvirt] [PATCH 14/20] qemu_process: Store libDir in qemuProcessQMP struct
On Tue, Feb 19, 2019 at 10:04:57AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
Store libDir path in the qemuProcessQMP struct in anticipation of moving
path construction code into qemuProcessQMPInit function.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 8 +++++---
src/qemu/qemu_process.h | 1 +
2 files changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 15/20] qemu_process: Setup paths within qemuProcessQMPInit
From: Chris Venteicher <cventeic(a)redhat.com>
Move code for setting paths and prepping file system from
qemuProcessQMPNew to qemuProcessQMPInit.
This keeps qemuProcessQMPNew limited to data structures and path
initialization is done in qemuProcessQMPInit.
The patch is a non-functional, cut / paste change, however goto is now
"cleanup" rather than "error".
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 42 +++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c4cbd7c807..9341aa43a7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8361,26 +8361,6 @@ qemuProcessQMPNew(const char *binary,
proc->runGid = runGid;
proc->forceTCG = forceTCG;
- /* the ".sock" sufix is important to avoid a possible clash with a qemu
- * domain called "capabilities"
- */
- if (virAsprintf(&proc->monpath, "%s/%s", proc->libDir,
- "capabilities.monitor.sock") < 0)
- goto error;
- if (virAsprintf(&proc->monarg, "unix:%s,server,nowait",
proc->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(&proc->pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
- goto error;
-
- virPidFileForceCleanupPath(proc->pidfile);
-
return proc;
error:
@@ -8396,8 +8376,30 @@ qemuProcessQMPInit(qemuProcessQMPPtr proc)
VIR_DEBUG("proc=%p, emulator=%s", proc, proc->binary);
+ /* the ".sock" sufix is important to avoid a possible clash with a qemu
+ * domain called "capabilities"
+ */
+ if (virAsprintf(&proc->monpath, "%s/%s", proc->libDir,
+ "capabilities.monitor.sock") < 0)
+ goto cleanup;
+
+ if (virAsprintf(&proc->monarg, "unix:%s,server,nowait",
proc->monpath) < 0)
+ goto cleanup;
+
+ /* ".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(&proc->pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
+ goto cleanup;
+
+ virPidFileForceCleanupPath(proc->pidfile);
+
ret = 0;
+ cleanup:
return ret;
}
--
2.20.1
9:26 a.m.
New subject: [libvirt] [PATCH 15/20] qemu_process: Setup paths within qemuProcessQMPInit
On Tue, Feb 19, 2019 at 10:04:58AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
Move code for setting paths and prepping file system from
qemuProcessQMPNew to qemuProcessQMPInit.
This keeps qemuProcessQMPNew limited to data structures and path
initialization is done in qemuProcessQMPInit.
The patch is a non-functional, cut / paste change, however goto is now
"cleanup" rather than "error".
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 42 +++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:04 a.m.
New subject: [libvirt] [PATCH 16/20] qemu_process: Stop retaining monitor config in qemuProcessQMP
From: Chris Venteicher <cventeic(a)redhat.com>
The monitor config data is removed from the qemuProcessQMP struct.
The monitor config data can be initialized immediately before call to
qemuMonitorOpen and does not need to be maintained after the call
because qemuMonitorOpen copies any strings it needs.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 9 +++++----
src/qemu/qemu_process.h | 1 -
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9341aa43a7..79896b6232 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8480,14 +8480,15 @@ static int
qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
{
virDomainXMLOptionPtr xmlopt = NULL;
+ virDomainChrSourceDef monConfig;
int ret = -1;
VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
proc, proc->binary, (long long)proc->pid);
- proc->config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
- proc->config.data.nix.path = proc->monpath;
- proc->config.data.nix.listen = false;
+ monConfig.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+ monConfig.data.nix.path = proc->monpath;
+ monConfig.data.nix.listen = false;
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
!(proc->vm = virDomainObjNew(xmlopt)))
@@ -8495,7 +8496,7 @@ qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
proc->vm->pid = proc->pid;
- if (!(proc->mon = qemuMonitorOpen(proc->vm, &proc->config, true, true,
+ if (!(proc->mon = qemuMonitorOpen(proc->vm, &monConfig, true, true,
0, &callbacks, NULL)))
goto cleanup;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 9ab885c6c3..2422d1eb7c 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -227,7 +227,6 @@ struct _qemuProcessQMP {
char *pidfile;
virCommandPtr cmd;
qemuMonitorPtr mon;
- virDomainChrSourceDef config;
pid_t pid;
virDomainObjPtr vm;
bool forceTCG;
--
2.20.1
9:26 a.m.
New subject: [libvirt] [PATCH 16/20] qemu_process: Stop retaining monitor config in qemuProcessQMP
On Tue, Feb 19, 2019 at 10:04:59AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
The monitor config data is removed from the qemuProcessQMP struct.
The monitor config data can be initialized immediately before call to
qemuMonitorOpen and does not need to be maintained after the call
because qemuMonitorOpen copies any strings it needs.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- no change
src/qemu/qemu_process.c | 9 +++++----
src/qemu/qemu_process.h | 1 -
2 files changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:05 a.m.
New subject: [libvirt] [PATCH 17/20] qemu_process: Document and cleanup qemuProcessQMPNew
From: Chris Venteicher <cventeic(a)redhat.com>
qemuProcessQMPNew is one of the public functions used to create and
manage a QEMU process for QMP command exchanges outside of domain
operations.
Add descriptive comment block, debug statement and make source
consistent with the cleanup / VIR_STEAL_PTR format used elsewhere.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- just a few small tweaks, mainly in the commit message and comments
src/qemu/qemu_process.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 79896b6232..b5d127b7d3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8341,6 +8341,17 @@ qemuProcessQMPFree(qemuProcessQMPPtr proc)
}
+/**
+ * qemuProcessQMPNew:
+ * @binary: QEMU binary
+ * @libDir: Directory for process and connection artifacts
+ * @runUid: UserId for QEMU process
+ * @runGid: GroupId for QEMU process
+ * @forceTCG: Force TCG mode if true
+ *
+ * Allocate and initialize domain structure encapsulating QEMU process state
+ * and monitor connection for completing QMP queries.
+ */
qemuProcessQMPPtr
qemuProcessQMPNew(const char *binary,
const char *libDir,
@@ -8348,24 +8359,28 @@ qemuProcessQMPNew(const char *binary,
gid_t runGid,
bool forceTCG)
{
+ qemuProcessQMPPtr ret = NULL;
qemuProcessQMPPtr proc = NULL;
+ VIR_DEBUG("exec=%s, libDir=%s, runUid=%u, runGid=%u, forceTCG=%d",
+ binary, libDir, runUid, runGid, forceTCG);
+
if (VIR_ALLOC(proc) < 0)
- goto error;
+ goto cleanup;
if (VIR_STRDUP(proc->binary, binary) < 0 ||
VIR_STRDUP(proc->libDir, libDir) < 0)
- goto error;
+ goto cleanup;
proc->runUid = runUid;
proc->runGid = runGid;
proc->forceTCG = forceTCG;
- return proc;
+ VIR_STEAL_PTR(ret, proc);
- error:
+ cleanup:
qemuProcessQMPFree(proc);
- return NULL;
+ return ret;
}
--
2.20.1
9:30 a.m.
New subject: [libvirt] [PATCH 17/20] qemu_process: Document and cleanup qemuProcessQMPNew
On Tue, Feb 19, 2019 at 10:05:00AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
qemuProcessQMPNew is one of the public functions used to create and
manage a QEMU process for QMP command exchanges outside of domain
operations.
Add descriptive comment block, debug statement and make source
consistent with the cleanup / VIR_STEAL_PTR format used elsewhere.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- just a few small tweaks, mainly in the commit message and comments
src/qemu/qemu_process.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:05 a.m.
New subject: [libvirt] [PATCH 18/20] qemu_process: Hide qemuProcessQMPStop
Users qemuProcessQMP struct were always forced to call both
qemuProcessQMPStop and qemuProcessQMPFree when they are done with the
process. We can just call qemuProcessQMPStop from qemuProcessQMPFree and
let users call qemuProcessQMPFree only.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- new patch
src/qemu/qemu_capabilities.c | 1 -
src/qemu/qemu_process.c | 92 ++++++++++++++++++------------------
src/qemu/qemu_process.h | 2 -
3 files changed, 45 insertions(+), 50 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4fd2169d49..a546d9b5a1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4397,7 +4397,6 @@ virQEMUCapsInitQMPSingle(virQEMUCapsPtr qemuCaps,
if (ret < 0)
virQEMUCapsLogProbeFailure(qemuCaps->binary);
- qemuProcessQMPStop(proc);
qemuProcessQMPFree(proc);
return ret;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b5d127b7d3..ca5911bacd 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8324,6 +8324,49 @@ static qemuMonitorCallbacks callbacks = {
};
+static void
+qemuProcessQMPStop(qemuProcessQMPPtr proc)
+{
+ if (proc->mon) {
+ virObjectUnlock(proc->mon);
+ qemuMonitorClose(proc->mon);
+ proc->mon = NULL;
+ }
+
+ if (proc->cmd) {
+ virCommandAbort(proc->cmd);
+ virCommandFree(proc->cmd);
+ proc->cmd = NULL;
+ }
+
+ if (proc->monpath)
+ unlink(proc->monpath);
+
+ virDomainObjEndAPI(&proc->vm);
+
+ if (proc->pid != 0) {
+ char ebuf[1024];
+
+ VIR_DEBUG("Killing QMP caps process %lld", (long long)proc->pid);
+ if (virProcessKill(proc->pid, SIGKILL) < 0 && errno != ESRCH)
+ VIR_ERROR(_("Failed to kill process %lld: %s"),
+ (long long)proc->pid,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+
+ proc->pid = 0;
+ }
+
+ if (proc->pidfile)
+ unlink(proc->pidfile);
+}
+
+
+/**
+ * qemuProcessQMPFree:
+ * @proc: Stores process and connection state
+ *
+ * Kill QEMU process and free process data structure.
+ */
void
qemuProcessQMPFree(qemuProcessQMPPtr proc)
{
@@ -8535,7 +8578,6 @@ qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
* proc = qemuProcessQMPNew(binary, libDir, runUid, runGid, forceTCG);
* qemuProcessQMPStart(proc);
* ** Send QMP Queries to QEMU using monitor (proc->mon) **
- * qemuProcessQMPStop(proc);
* qemuProcessQMPFree(proc);
*
* Process error output (proc->stderr) remains available in qemuProcessQMP
@@ -8552,57 +8594,13 @@ qemuProcessQMPStart(qemuProcessQMPPtr proc)
goto cleanup;
if (qemuProcessQMPLaunch(proc) < 0)
- goto stop;
+ goto cleanup;
if (qemuProcessQMPConnectMonitor(proc) < 0)
- goto stop;
+ goto cleanup;
ret = 0;
cleanup:
return ret;
-
- stop:
- qemuProcessQMPStop(proc);
- goto cleanup;
-}
-
-
-void
-qemuProcessQMPStop(qemuProcessQMPPtr proc)
-{
- if (!proc)
- return;
-
- if (proc->mon) {
- virObjectUnlock(proc->mon);
- qemuMonitorClose(proc->mon);
- proc->mon = NULL;
- }
-
- if (proc->cmd) {
- virCommandAbort(proc->cmd);
- virCommandFree(proc->cmd);
- proc->cmd = NULL;
- }
-
- if (proc->monpath)
- unlink(proc->monpath);
-
- virDomainObjEndAPI(&proc->vm);
-
- if (proc->pid != 0) {
- char ebuf[1024];
-
- VIR_DEBUG("Killing QMP caps process %lld", (long long)proc->pid);
- if (virProcessKill(proc->pid, SIGKILL) < 0 && errno != ESRCH)
- VIR_ERROR(_("Failed to kill process %lld: %s"),
- (long long)proc->pid,
- virStrerror(errno, ebuf, sizeof(ebuf)));
-
- proc->pid = 0;
- }
-
- if (proc->pidfile)
- unlink(proc->pidfile);
}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 2422d1eb7c..c435a19726 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -242,6 +242,4 @@ void qemuProcessQMPFree(qemuProcessQMPPtr proc);
int qemuProcessQMPStart(qemuProcessQMPPtr proc);
-void qemuProcessQMPStop(qemuProcessQMPPtr proc);
-
#endif /* LIBVIRT_QEMU_PROCESS_H */
--
2.20.1
9:31 a.m.
New subject: [libvirt] [PATCH 18/20] qemu_process: Hide qemuProcessQMPStop
On Tue, Feb 19, 2019 at 10:05:01AM +0100, Jiri Denemark wrote:
Users qemuProcessQMP struct were always forced to call both
qemuProcessQMPStop and qemuProcessQMPFree when they are done with the
process. We can just call qemuProcessQMPStop from qemuProcessQMPFree and
let users call qemuProcessQMPFree only.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- new patch
src/qemu/qemu_capabilities.c | 1 -
src/qemu/qemu_process.c | 92 ++++++++++++++++++------------------
src/qemu/qemu_process.h | 2 -
3 files changed, 45 insertions(+), 50 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:05 a.m.
New subject: [libvirt] [PATCH 19/20] qemu_process: Use unique directories for QMP processes
From: Chris Venteicher <cventeic(a)redhat.com>
Multiple QEMU processes for QMP commands can operate concurrently.
Use a unique directory under libDir for each QEMU process to avoid
pidfile and unix socket collision between processes.
The pid file name is changed from "capabilities.pidfile" to "qmp.pid"
because we no longer need to avoid a possible clash with a qemu domain
called "capabilities" now that the processes artifacts are stored in
their own unique temporary directories.
"Capabilities" was changed to "qmp" in the pid file name because
these
processes are no longer specific to the capabilities usecase and are
more generic in terms of being used for any general purpose QMP message
exchanges with a QEMU process that is not associated with a domain.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- report error if mkdtemp fails
src/qemu/qemu_process.c | 30 ++++++++++++++++++++----------
src/qemu/qemu_process.h | 1 +
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ca5911bacd..820476dbb1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8358,6 +8358,9 @@ qemuProcessQMPStop(qemuProcessQMPPtr proc)
if (proc->pidfile)
unlink(proc->pidfile);
+
+ if (proc->uniqDir)
+ rmdir(proc->uniqDir);
}
@@ -8376,6 +8379,7 @@ qemuProcessQMPFree(qemuProcessQMPPtr proc)
qemuProcessQMPStop(proc);
VIR_FREE(proc->binary);
VIR_FREE(proc->libDir);
+ VIR_FREE(proc->uniqDir);
VIR_FREE(proc->monpath);
VIR_FREE(proc->monarg);
VIR_FREE(proc->pidfile);
@@ -8430,31 +8434,37 @@ qemuProcessQMPNew(const char *binary,
static int
qemuProcessQMPInit(qemuProcessQMPPtr proc)
{
+ char *template = NULL;
int ret = -1;
VIR_DEBUG("proc=%p, emulator=%s", proc, proc->binary);
- /* the ".sock" sufix is important to avoid a possible clash with a qemu
- * domain called "capabilities"
- */
- if (virAsprintf(&proc->monpath, "%s/%s", proc->libDir,
- "capabilities.monitor.sock") < 0)
+ if (virAsprintf(&template, "%s/qmp-XXXXXX", proc->libDir) < 0)
+ goto cleanup;
+
+ if (!(proc->uniqDir = mkdtemp(template))) {
+ virReportSystemError(errno,
+ _("Failed to create unique directory with "
+ "template '%s' for probing QEMU"),
+ template);
+ goto cleanup;
+ }
+
+ if (virAsprintf(&proc->monpath, "%s/%s", proc->uniqDir,
+ "qmp.monitor") < 0)
goto cleanup;
if (virAsprintf(&proc->monarg, "unix:%s,server,nowait",
proc->monpath) < 0)
goto cleanup;
- /* ".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(&proc->pidfile, "%s/%s", proc->libDir,
"capabilities.pidfile") < 0)
+ if (virAsprintf(&proc->pidfile, "%s/%s", proc->uniqDir,
"qmp.pid") < 0)
goto cleanup;
- virPidFileForceCleanupPath(proc->pidfile);
-
ret = 0;
cleanup:
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index c435a19726..3367cd3fe5 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -225,6 +225,7 @@ struct _qemuProcessQMP {
char *monarg;
char *monpath;
char *pidfile;
+ char *uniqDir;
virCommandPtr cmd;
qemuMonitorPtr mon;
pid_t pid;
--
2.20.1
9:43 a.m.
New subject: [libvirt] [PATCH 19/20] qemu_process: Use unique directories for QMP processes
On Tue, Feb 19, 2019 at 10:05:02AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
Multiple QEMU processes for QMP commands can operate concurrently.
Use a unique directory under libDir for each QEMU process to avoid
pidfile and unix socket collision between processes.
The pid file name is changed from "capabilities.pidfile" to "qmp.pid"
because we no longer need to avoid a possible clash with a qemu domain
called "capabilities" now that the processes artifacts are stored in
their own unique temporary directories.
"Capabilities" was changed to "qmp" in the pid file name because
these
processes are no longer specific to the capabilities usecase and are
more generic in terms of being used for any general purpose QMP message
exchanges with a QEMU process that is not associated with a domain.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- report error if mkdtemp fails
src/qemu/qemu_process.c | 30 ++++++++++++++++++++----------
src/qemu/qemu_process.h | 1 +
2 files changed, 21 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
3:05 a.m.
New subject: [libvirt] [PATCH 20/20] qemu_process: Enter QMP command mode when starting QEMU Process
From: Chris Venteicher <cventeic(a)redhat.com>
qemuProcessQMPStart starts a QEMU process and monitor connection that
can be used by multiple functions possibly for multiple QMP commands.
The QMP exchange to exit capabilities negotiation mode and enter command
mode can only be performed once after the monitor connection is
established.
Move responsibility for entering QMP command mode into the
qemuProcessQMP code so multiple functions can issue QMP commands in
arbitrary orders.
This also simplifies the functions using the connection provided by
qemuProcessQMPStart to issue QMP commands.
Test code now needs to call qemuMonitorSetCapabilities to send the
message to switch to command mode because the test code does not use the
qemuProcessQMP command that internally calls qemuMonitorSetCapabilities.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- qemuMonitorSetCapabilities call was moved into a new
qemuProcessQMPInitMonitor function, which is called from
qemuProcessQMPConnectMonitor and the tests
src/qemu/qemu_capabilities.c | 12 ------------
src/qemu/qemu_process.c | 16 ++++++++++++++++
src/qemu/qemu_processpriv.h | 2 ++
tests/qemucapabilitiestest.c | 9 +++++++++
4 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a546d9b5a1..b48bcbebee 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4149,12 +4149,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
/* @mon is supposed to be locked by callee */
- if (qemuMonitorSetCapabilities(mon) < 0) {
- VIR_DEBUG("Failed to set monitor capabilities %s",
- virGetLastErrorMessage());
- goto cleanup;
- }
-
if (qemuMonitorGetVersion(mon,
&major, &minor, µ,
&package) < 0) {
@@ -4333,12 +4327,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps
ATTRIBUTE_UNUSED,
{
int ret = -1;
- if (qemuMonitorSetCapabilities(mon) < 0) {
- VIR_DEBUG("Failed to set monitor capabilities %s",
- virGetLastErrorMessage());
- goto cleanup;
- }
-
if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, true) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 820476dbb1..85952b9975 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8544,6 +8544,19 @@ qemuProcessQMPLaunch(qemuProcessQMPPtr proc)
}
+int
+qemuProcessQMPInitMonitor(qemuMonitorPtr mon)
+{
+ if (qemuMonitorSetCapabilities(mon) < 0) {
+ VIR_DEBUG("Failed to set monitor capabilities %s",
+ virGetLastErrorMessage());
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int
qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
{
@@ -8570,6 +8583,9 @@ qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc)
virObjectLock(proc->mon);
+ if (qemuProcessQMPInitMonitor(proc->mon) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
diff --git a/src/qemu/qemu_processpriv.h b/src/qemu/qemu_processpriv.h
index 17f981829b..237d7bad58 100644
--- a/src/qemu/qemu_processpriv.h
+++ b/src/qemu/qemu_processpriv.h
@@ -38,4 +38,6 @@ int qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon,
const char *devAlias,
void *opaque);
+int qemuProcessQMPInitMonitor(qemuMonitorPtr mon);
+
#endif /* LIBVIRT_QEMU_PROCESSPRIV_H */
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index fab77db7ce..8d47133e6f 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -26,6 +26,8 @@
#include "qemu/qemu_capspriv.h"
#define LIBVIRT_QEMU_MONITOR_PRIV_H_ALLOW
#include "qemu/qemu_monitor_priv.h"
+#define LIBVIRT_QEMU_PROCESSPRIV_H_ALLOW
+#include "qemu/qemu_processpriv.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -60,6 +62,9 @@ testQemuCaps(const void *opaque)
if (!(mon = qemuMonitorTestNewFromFileFull(repliesFile, &data->driver,
NULL)))
goto cleanup;
+ if (qemuProcessQMPInitMonitor(qemuMonitorTestGetMonitor(mon)) < 0)
+ goto cleanup;
+
if (!(capsActual = virQEMUCapsNew()) ||
virQEMUCapsInitQMPMonitor(capsActual,
qemuMonitorTestGetMonitor(mon)) < 0)
@@ -67,6 +72,10 @@ testQemuCaps(const void *opaque)
if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) {
qemuMonitorResetCommandID(qemuMonitorTestGetMonitor(mon));
+
+ if (qemuProcessQMPInitMonitor(qemuMonitorTestGetMonitor(mon)) < 0)
+ goto cleanup;
+
if (virQEMUCapsInitQMPMonitorTCG(capsActual,
qemuMonitorTestGetMonitor(mon)) < 0)
goto cleanup;
--
2.20.1
9:46 a.m.
New subject: [libvirt] [PATCH 20/20] qemu_process: Enter QMP command mode when starting QEMU Process
On Tue, Feb 19, 2019 at 10:05:03AM +0100, Jiri Denemark wrote:
From: Chris Venteicher <cventeic(a)redhat.com>
qemuProcessQMPStart starts a QEMU process and monitor connection that
can be used by multiple functions possibly for multiple QMP commands.
The QMP exchange to exit capabilities negotiation mode and enter command
mode can only be performed once after the monitor connection is
established.
Move responsibility for entering QMP command mode into the
qemuProcessQMP code so multiple functions can issue QMP commands in
arbitrary orders.
This also simplifies the functions using the connection provided by
qemuProcessQMPStart to issue QMP commands.
Test code now needs to call qemuMonitorSetCapabilities to send the
message to switch to command mode because the test code does not use the
qemuProcessQMP command that internally calls qemuMonitorSetCapabilities.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Notes:
Version 7:
- qemuMonitorSetCapabilities call was moved into a new
qemuProcessQMPInitMonitor function, which is called from
qemuProcessQMPConnectMonitor and the tests
src/qemu/qemu_capabilities.c | 12 ------------
src/qemu/qemu_process.c | 16 ++++++++++++++++
src/qemu/qemu_processpriv.h | 2 ++
tests/qemucapabilitiestest.c | 9 +++++++++
4 files changed, 27 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2329
days inactive
2329
days old
41 comments
2 participants
participants (2)
-
Jiri Denemark -
Ján Tomko