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@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