virQEMUCapsInitQMP now stops QEMU process in all execution paths,
before freeing the process structure.

The qemuProcessQmpStop function can be called multiple times without
problems... Won't attempt to stop processes and free resources multiple
times.

Follow the convention established in qemu_process of
1) alloc process structure
2) start process
3) use process
4) stop process
5) free process data structure

The process data structure persists after the process activation fails
or the process dies or is killed so stderr strings can be retrieved
until the process data structure is freed.

Signed-off-by: Chris Venteicher <cventeic@redhat.com>
---
 src/qemu/qemu_capabilities.c |  1 +
 src/qemu/qemu_process.c      | 19 ++++++++++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d903fbddf8..a79329a134 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4273,6 +4273,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
     ret = 0;
 
  cleanup:
+    qemuProcessQmpStop(proc);
     qemuProcessQmpFree(proc);
     return ret;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8465448a49..fa050a1a27 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8263,14 +8263,17 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc,
 void
 qemuProcessQmpStop(qemuProcessQmpPtr proc)
 {
-    if (proc->mon)
+    if (proc->mon) {
         virObjectUnlock(proc->mon);
-    qemuMonitorClose(proc->mon);
-    proc->mon = NULL;
+        qemuMonitorClose(proc->mon);
+        proc->mon = NULL;
+    }
 
-    virCommandAbort(proc->cmd);
-    virCommandFree(proc->cmd);
-    proc->cmd = NULL;
+    if (proc->cmd) {
+        virCommandAbort(proc->cmd);
+        virCommandFree(proc->cmd);
+        proc->cmd = NULL;
+    }
 
     if (proc->monpath)
         unlink(proc->monpath);
@@ -8287,8 +8290,10 @@ qemuProcessQmpStop(qemuProcessQmpPtr proc)
                       virStrerror(errno, ebuf, sizeof(ebuf)));
 
         VIR_FREE(*proc->qmperr);
+
+        proc->pid = 0;
     }
+
     if (proc->pidfile)
         unlink(proc->pidfile);
-    proc->pid = 0;
 }
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Sun, Dec 02, 2018 at 23:10:00 -0600, Chris Venteicher wrote:
> virQEMUCapsInitQMP now stops QEMU process in all execution paths,
> before freeing the process structure.
> 
> The qemuProcessQmpStop function can be called multiple times without
> problems... Won't attempt to stop processes and free resources multiple
> times.
> 
> Follow the convention established in qemu_process of
> 1) alloc process structure
> 2) start process
> 3) use process
> 4) stop process
> 5) free process data structure
> 
> The process data structure persists after the process activation fails
> or the process dies or is killed so stderr strings can be retrieved
> until the process data structure is freed.
> 
> Signed-off-by: Chris Venteicher <cventeic@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c |  1 +
>  src/qemu/qemu_process.c      | 19 ++++++++++++-------
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index d903fbddf8..a79329a134 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4273,6 +4273,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>      ret = 0;
>  
>   cleanup:
> +    qemuProcessQmpStop(proc);

This would still crash libvirt if qemuProcessQmpNew() failed.

>      qemuProcessQmpFree(proc);
>      return ret;
>  }
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8465448a49..fa050a1a27 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8263,14 +8263,17 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc,
>  void
>  qemuProcessQmpStop(qemuProcessQmpPtr proc)
>  {

Here you'd need to add

        if (!proc)
            return;

> -    if (proc->mon)
> +    if (proc->mon) {
>          virObjectUnlock(proc->mon);
...

And obviously s/Qmp/QMP/g.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list