[libvirt] [PATCH v5 09/36] qemu_process: Persist stderr in qemuProcessQmp struct

Chris Venteicher posted 36 patches 7 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH v5 09/36] qemu_process: Persist stderr in qemuProcessQmp struct
Posted by Chris Venteicher 7 years, 2 months ago
A qemuProcessQmp struct tracks the entire lifespan of a single QEMU Process
including storing error output when the process terminates or activation
fails.

Error output remains available until qemuProcessQmpFree is called.

The qmperr variable is renamed stderr (captures stderr from process.)

The stderr buffer no longer needs to be maintained outside of the
qemuProcessQmp structure because the structure is used for a single QEMU
process and the structures can be maintained as long as required
to retrieve the process error info.

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

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1efec85b57..997f8c19d5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4259,18 +4259,17 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
 {
     qemuProcessQmpPtr proc = NULL;
     qemuProcessQmpPtr procTCG = NULL;
-    char *qmperr = NULL;
     int ret = -1;
 
     if (!(proc = qemuProcessQmpNew(qemuCaps->binary, libDir,
-                                   runUid, runGid, &qmperr, false)))
+                                   runUid, runGid, false)))
         goto cleanup;
 
     if (qemuProcessQmpRun(proc) < 0) {
         if (proc->status != 0)
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Failed to probe QEMU binary with QMP: %s"),
-                           qmperr ? qmperr : _("uknown error"));
+                           proc->stderr ? proc->stderr : _("uknown error"));
 
         goto cleanup;
     }
@@ -4287,7 +4286,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
         qemuProcessQmpStop(proc);
 
         procTCG = qemuProcessQmpNew(qemuCaps->binary, libDir,
-                                    runUid, runGid, NULL, true);
+                                    runUid, runGid, true);
 
         if (qemuProcessQmpRun(procTCG) < 0)
             goto cleanup;
@@ -4306,7 +4305,6 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
     qemuProcessQmpStop(procTCG);
     qemuProcessQmpFree(proc);
     qemuProcessQmpFree(procTCG);
-    VIR_FREE(qmperr);
 
     return ret;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2ec0d5340d..8ad685f3ea 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8118,6 +8118,7 @@ qemuProcessQmpFree(qemuProcessQmpPtr proc)
     VIR_FREE(proc->monpath);
     VIR_FREE(proc->monarg);
     VIR_FREE(proc->pidfile);
+    VIR_FREE(proc->stderr);
     VIR_FREE(proc);
 }
 
@@ -8127,7 +8128,6 @@ qemuProcessQmpNew(const char *binary,
                   const char *libDir,
                   uid_t runUid,
                   gid_t runGid,
-                  char **qmperr,
                   bool forceTCG)
 {
     qemuProcessQmpPtr proc = NULL;
@@ -8141,7 +8141,6 @@ qemuProcessQmpNew(const char *binary,
 
     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
@@ -8213,7 +8212,7 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc)
     virCommandSetGID(proc->cmd, proc->runGid);
     virCommandSetUID(proc->cmd, proc->runUid);
 
-    virCommandSetErrorBuffer(proc->cmd, proc->qmperr);
+    virCommandSetErrorBuffer(proc->cmd, &(proc->stderr));
 
     proc->status = 0;
 
@@ -8222,7 +8221,7 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc)
 
     if (proc->status != 0) {
         VIR_DEBUG("QEMU %s exited with status %d: %s",
-                  proc->binary, proc->status, *proc->qmperr);
+                  proc->binary, proc->status, proc->stderr);
         goto cleanup;
     }
 
@@ -8283,8 +8282,6 @@ qemuProcessQmpStop(qemuProcessQmpPtr proc)
                       (long long)proc->pid,
                       virStrerror(errno, ebuf, sizeof(ebuf)));
 
-        VIR_FREE(*proc->qmperr);
-
         proc->pid = 0;
     }
 
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index a56e9608cd..3ddfa82918 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -221,7 +221,7 @@ struct _qemuProcessQmp {
     uid_t runUid;
     gid_t runGid;
     int status;
-    char **qmperr;
+    char *stderr;
     char *monarg;
     char *monpath;
     char *pidfile;
@@ -237,7 +237,6 @@ qemuProcessQmpPtr qemuProcessQmpNew(const char *binary,
                                     const char *libDir,
                                     uid_t runUid,
                                     gid_t runGid,
-                                    char **qmperr,
                                     bool forceTCG);
 
 void qemuProcessQmpFree(qemuProcessQmpPtr proc);
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 09/36] qemu_process: Persist stderr in qemuProcessQmp struct
Posted by Jiri Denemark 7 years, 1 month ago
On Sun, Dec 02, 2018 at 23:10:03 -0600, Chris Venteicher wrote:
> A qemuProcessQmp struct tracks the entire lifespan of a single QEMU Process
> including storing error output when the process terminates or activation
> fails.
> 
> Error output remains available until qemuProcessQmpFree is called.
> 
> The qmperr variable is renamed stderr (captures stderr from process.)
> 
> The stderr buffer no longer needs to be maintained outside of the
> qemuProcessQmp structure because the structure is used for a single QEMU
> process and the structures can be maintained as long as required
> to retrieve the process error info.
> 
> Signed-off-by: Chris Venteicher <cventeic@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 8 +++-----
>  src/qemu/qemu_process.c      | 9 +++------
>  src/qemu/qemu_process.h      | 3 +--
>  3 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 1efec85b57..997f8c19d5 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4259,18 +4259,17 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>  {
>      qemuProcessQmpPtr proc = NULL;
>      qemuProcessQmpPtr procTCG = NULL;
> -    char *qmperr = NULL;
>      int ret = -1;
>  
>      if (!(proc = qemuProcessQmpNew(qemuCaps->binary, libDir,
> -                                   runUid, runGid, &qmperr, false)))
> +                                   runUid, runGid, false)))
>          goto cleanup;
>  
>      if (qemuProcessQmpRun(proc) < 0) {
>          if (proc->status != 0)
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("Failed to probe QEMU binary with QMP: %s"),
> -                           qmperr ? qmperr : _("uknown error"));
> +                           proc->stderr ? proc->stderr : _("uknown error"));
>  
>          goto cleanup;
>      }

This patch will obviously change a bit according to the comments to
previous patches.

Jirka

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