11:10 p.m.
Move a step closer to the function structure used elsewhere in
qemu_process where qemuProcessStart and qemuProcessStop are the exposed
functions.
qemuProcessQmpStart mirrors qemuProcessStart in calling sub functions to
initialize, launch the process and connect the monitor to the QEMU
process.
static functions qemuProcessQmpInit, qemuProcessQmpLaunch and
qemuProcessQmpConnectMonitor are introduced.
qemuProcessQmpLaunch is just renamed from qemuProcessQmpRun and
encapsulates all of the original code.
qemuProcessQmpInit and qemuProcessQmpMonitor are nearly empty functions
acting as placeholders for later patches where blocks of semi-complicated code
are cut/pasted into these functions without modification
(hopefully making review easier.)
Looking forward, the patch series ultimately moves the code into this
partitioning:
- qemuProcessQmpInit
Becomes the location of ~25 lines of code to create storage
directory, in thread safe way, and initialize paths
for monpath, monarg and pidfile.
- qemuProcessQmpLaunch
Becomes the location of ~48 lines of code used to create and run the
QEMU command.
- qemuProcessQmpConnectMonitor
Becomes the final location of ~58 lines of code used to open and
initialize the monitor connection between libvirt and qemu.
Three smaller, purpose-identifying, functions of ~60 lines or less seem
better than a single large process "start" function of > 130 lines.
Being able to compare and contrast between the domain and non-domain
versions of process code is useful too. There is some significant
overlap between what the non-domain and domain functions do. There is
also significant additional functionality in the domain functions that
might be useful in the non-domain functions in the future.
Possibly there could be sharing between non-domain and
domain process code in the future but common code would have
to be carefully extracted from the domain process code (not trivial.)
Mirroring the domain process code has some value, but
partitioning the code into logical chunks of < 60 lines
is the main reason for the static functions.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 4 +-
src/qemu/qemu_process.c | 94 +++++++++++++++++++++++++++++++++++-
src/qemu/qemu_process.h | 2 +-
3 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 997f8c19d5..ce60648897 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4265,7 +4265,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
runUid, runGid, false)))
goto cleanup;
- if (qemuProcessQmpRun(proc) < 0) {
+ if (qemuProcessQmpStart(proc) < 0) {
if (proc->status != 0)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to probe QEMU binary with QMP: %s"),
@@ -4288,7 +4288,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
procTCG = qemuProcessQmpNew(qemuCaps->binary, libDir,
runUid, runGid, true);
- if (qemuProcessQmpRun(procTCG) < 0)
+ if (qemuProcessQmpStart(procTCG) < 0)
goto cleanup;
if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, procTCG->mon) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8ad685f3ea..938d328235 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8175,8 +8175,28 @@ qemuProcessQmpNew(const char *binary,
}
-int
-qemuProcessQmpRun(qemuProcessQmpPtr proc)
+/* Initialize configuration and paths prior to starting QEMU
+ */
+static int
+qemuProcessQmpInit(qemuProcessQmpPtr proc)
+{
+ int ret = -1;
+
+ VIR_DEBUG("Beginning VM startup process"
+ " proc=%p, emulator=%s",
+ proc, proc->binary);
+
+ ret = 0;
+
+ VIR_DEBUG("ret=%i", ret);
+ return ret;
+}
+
+
+/* Launch QEMU Process
+ */
+static int
+qemuProcessQmpLaunch(qemuProcessQmpPtr proc)
{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
@@ -8253,6 +8273,76 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc)
}
+/* Connect Monitor to QEMU Process
+ */
+static int
+qemuProcessQmpConnectMonitor(qemuProcessQmpPtr proc)
+{
+ int ret = -1;
+
+ VIR_DEBUG("proc=%p, emulator=%s, proc->pid=%lld",
+ proc, NULLSTR(proc->binary), (long long)proc->pid);
+
+ ret = 0;
+
+ VIR_DEBUG("ret=%i", ret);
+ return ret;
+}
+
+
+/**
+ * qemuProcessQmpStart:
+ * @proc: Stores Process and Connection State
+ *
+ * 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);
+ *
+ * Check monitor is not NULL before using.
+ *
+ * QEMU probing failure results in monitor being NULL but is not considered
+ * an error case and does not result in a negative return value.
+ *
+ * Both qemuProcessQmpStop and qemuProcessQmpFree must be called to cleanup and
+ * free resources, even in activation failure cases.
+ *
+ * Process error output (proc->qmperr) 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:
+ VIR_DEBUG("ret=%i", ret);
+ 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 3ddfa82918..1b64aeef4e 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -241,7 +241,7 @@ qemuProcessQmpPtr qemuProcessQmpNew(const char *binary,
void qemuProcessQmpFree(qemuProcessQmpPtr proc);
-int qemuProcessQmpRun(qemuProcessQmpPtr cmd);
+int qemuProcessQmpStart(qemuProcessQmpPtr proc);
void qemuProcessQmpStop(qemuProcessQmpPtr proc);
--
2.17.1