Re: [libvirt] [PATCH RFC 07/22] qemu_process: Use qemuProcess struct for a single process
On Sun, Nov 11, 2018 at 13:59:15 -0600, Chris Venteicher wrote:
In new process code, move from model where qemuProcess struct can be
used to activate a series of Qemu processes to model where one
qemuProcess struct is used for one and only one Qemu process.
The forceTCG parameter (use / don't use KVM) will be passed when the
qemuProcess struct is initialized since the qemuProcess struct won't be
reused.
Signed-off-by: Chris Venteicher <cventeic(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 16 ++++++++++++----
src/qemu/qemu_process.c | 11 +++++++----
src/qemu/qemu_process.h | 6 ++++--
3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 082874082b..a957c3bdbd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4220,14 +4220,16 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
char **qmperr)
{
qemuProcessPtr proc = NULL;
+ qemuProcessPtr proc_kvm = NULL;
s/proc_kvm/procTCG/
The second QEMU process probes for TCG capabilities in case the first
reported KVM as enabled (otherwise the first one already reported TCG
capabilities).
int ret = -1;
int rc;
+ bool forceTCG = false;
This variable does not make a lot of sense. You can just use false/true
directly when calling qemuProcessNew.
if (!(proc = qemuProcessNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr)))
+ runUid, runGid, qmperr, forceTCG)))
goto cleanup;
- if ((rc = qemuProcessRun(proc, false)) != 0) {
+ if ((rc = qemuProcessRun(proc)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
@@ -4238,13 +4240,17 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
qemuProcessStopQmp(proc);
- if ((rc = qemuProcessRun(proc, true)) != 0) {
+
+ forceTCG = true;
+ proc_kvm = qemuProcessNew(qemuCaps->binary, libDir, runUid, runGid, NULL,
forceTCG);
+
+ if ((rc = qemuProcessRun(proc_kvm)) != 0) {
if (rc == 1)
ret = 0;
goto cleanup;
}
- if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon) < 0)
+ if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc_kvm->mon) < 0)
goto cleanup;
}
@@ -4252,7 +4258,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
cleanup:
qemuProcessStopQmp(proc);
+ qemuProcessStopQmp(proc_kvm);
qemuProcessFree(proc);
+ qemuProcessFree(proc_kvm);
return ret;
}
After this patch virQEMUCapsInitQMP contains two almost identical parts.
It should be further refactored (in a follow up patch) to something like
(I was too lazy to come up with a good name for the function)
virQEMUCapsInitQMP()
{
if (virQEMUCapsInitQMP...(..., false, err) < 0)
return -1;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM) &&
virQEMUCapsInitQMP...(..., true, NULL) < 0)
return -1;
return 0;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2571024e8e..dda74d5b7a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8100,7 +8100,8 @@ qemuProcessNew(const char *binary,
const char *libDir,
uid_t runUid,
gid_t runGid,
- char **qmperr)
+ char **qmperr,
+ bool forceTCG)
{
qemuProcessPtr proc = NULL;
@@ -8110,10 +8111,13 @@ qemuProcessNew(const char *binary,
if (VIR_STRDUP(proc->binary, binary) < 0)
goto error;
+ proc->forceTCG = forceTCG;
I'd put this just after the "proc->qmperr = qmperr;" line with no empty
line separator to keep the order consistent with the order of function
parameters. But it's not a big deal.
+
proc->runUid = runUid;
proc->runGid = runGid;
proc->qmperr = qmperr;
+
Please, delete the extra empty line.
/* the ".sock" sufix is important to avoid a possible
clash with a qemu
* domain called "capabilities"
*/
Jirka