[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v5 07/36] qemu_process: Use qemuProcessQmp struct for a single process



On Sun, Dec 02, 2018 at 23:10:01 -0600, Chris Venteicher wrote:
> 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 redhat com>
> ---
>  src/qemu/qemu_capabilities.c | 20 ++++++++++++++++----
>  src/qemu/qemu_process.c      | 10 ++++++----
>  src/qemu/qemu_process.h      |  7 ++++---
>  3 files changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index a79329a134..bed9ca26a1 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
...
> @@ -4259,14 +4260,23 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>          goto cleanup;
>  
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
> +

I don't think there's any need for this empty line.

> +        /* 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);
...
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index fa050a1a27..324151a7c3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
...
> @@ -8137,9 +8138,11 @@ qemuProcessQmpNew(const char *binary,
>      if (VIR_STRDUP(proc->binary, binary) < 0)
>          goto error;
>  
> +

Drop this extra empty line.

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

With the empty lines removed and s/Qmp/QMP/g

Reviewed-by: Jiri Denemark <jdenemar redhat com>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]