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

Re: [libvirt] [PATCH v5 18/36] qemu_process: Catch process free before process stop



On Sun, Dec 02, 2018 at 23:10:12 -0600, Chris Venteicher wrote:
> Catch execution paths where qemuProcessQmpFree is called before
> qemuProcessQmpStop then report error and force stop before proceeding.
> 
> Also added public function header and debug message.
> 
> Signed-off-by: Chris Venteicher <cventeic redhat com>
> ---
>  src/qemu/qemu_process.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e9b50745d3..1b6adf1b64 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8107,13 +8107,28 @@ static qemuMonitorCallbacks callbacks = {
>  };
>  
>  
> +/**
> + * qemuProcessQmpFree:
> + * @proc: Stores Process and Connection State

Strange capitalization again.

> + *
> + * Free process data structure.
> + */
>  void
>  qemuProcessQmpFree(qemuProcessQmpPtr proc)
>  {
> +    VIR_DEBUG("proc=%p, proc->mon=%p", proc, (proc ? proc->mon : NULL));
> +

I don't think we need to log qemuProcessQmpFree(NULL) calls. I'd just
push this debug message after the check for proc == NULL.

>      if (!proc)
>          return;
>  
> -    qemuProcessQmpStop(proc);
> +    /* This should never be non-NULL if we get here, but just in case... */
> +    if (proc->mon || proc->pid) {
> +        VIR_ERROR(_("Unexpected QEMU still active during process free"
> +                    " emulator: %s, pid: %lld, mon: %p"),
> +                  NULLSTR(proc->binary), (long long)proc->pid, proc->mon);
> +        qemuProcessQmpStop(proc);
> +    }
> +

Instead of doing this, I think it would make more sense to merge Stop
and Free functions into a single one.

>      VIR_FREE(proc->binary);
>      VIR_FREE(proc->libDir);
>      VIR_FREE(proc->monpath);

Jirka


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