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@xxxxxxxxxx>
> ---
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
