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

Chris Venteicher posted 36 patches 7 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH v5 07/36] qemu_process: Use qemuProcessQmp struct for a single process
Posted by Chris Venteicher 7 years, 2 months ago
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
@@ -4242,14 +4242,15 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
                    char **qmperr)
 {
     qemuProcessQmpPtr proc = NULL;
+    qemuProcessQmpPtr procTCG = NULL;
     int ret = -1;
     int rc;
 
     if (!(proc = qemuProcessQmpNew(qemuCaps->binary, libDir,
-                                   runUid, runGid, qmperr)))
+                                   runUid, runGid, qmperr, false)))
         goto cleanup;
 
-    if ((rc = qemuProcessQmpRun(proc, false)) != 0) {
+    if ((rc = qemuProcessQmpRun(proc)) != 0) {
         if (rc == 1)
             ret = 0;
         goto cleanup;
@@ -4259,14 +4260,23 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
         goto cleanup;
 
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
+
+        /* 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);
-        if ((rc = qemuProcessQmpRun(proc, true)) != 0) {
+
+        procTCG = qemuProcessQmpNew(qemuCaps->binary, libDir,
+                                    runUid, runGid, NULL, true);
+
+        if ((rc = qemuProcessQmpRun(procTCG)) != 0) {
             if (rc == 1)
                 ret = 0;
             goto cleanup;
         }
 
-        if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, proc->mon) < 0)
+        if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, procTCG->mon) < 0)
             goto cleanup;
     }
 
@@ -4274,7 +4284,9 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
 
  cleanup:
     qemuProcessQmpStop(proc);
+    qemuProcessQmpStop(procTCG);
     qemuProcessQmpFree(proc);
+    qemuProcessQmpFree(procTCG);
     return ret;
 }
 
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
@@ -8127,7 +8127,8 @@ qemuProcessQmpNew(const char *binary,
                   const char *libDir,
                   uid_t runUid,
                   gid_t runGid,
-                  char **qmperr)
+                  char **qmperr,
+                  bool forceTCG)
 {
     qemuProcessQmpPtr proc = NULL;
 
@@ -8137,9 +8138,11 @@ qemuProcessQmpNew(const char *binary,
     if (VIR_STRDUP(proc->binary, binary) < 0)
         goto error;
 
+
     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"
@@ -8178,15 +8181,14 @@ qemuProcessQmpNew(const char *binary,
  *          1 when probing QEMU failed
  */
 int
-qemuProcessQmpRun(qemuProcessQmpPtr proc,
-                  bool forceTCG)
+qemuProcessQmpRun(qemuProcessQmpPtr proc)
 {
     virDomainXMLOptionPtr xmlopt = NULL;
     const char *machine;
     int status = 0;
     int ret = -1;
 
-    if (forceTCG)
+    if (proc->forceTCG)
         machine = "none,accel=tcg";
     else
         machine = "none,accel=kvm:tcg";
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 9a72db9a08..51598b402e 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -229,18 +229,19 @@ struct _qemuProcessQmp {
     virDomainChrSourceDef config;
     pid_t pid;
     virDomainObjPtr vm;
+    bool forceTCG;
 };
 
 qemuProcessQmpPtr qemuProcessQmpNew(const char *binary,
                                     const char *libDir,
                                     uid_t runUid,
                                     gid_t runGid,
-                                    char **qmperr);
+                                    char **qmperr,
+                                    bool forceTCG);
 
 void qemuProcessQmpFree(qemuProcessQmpPtr proc);
 
-int qemuProcessQmpRun(qemuProcessQmpPtr cmd,
-                      bool forceTCG);
+int qemuProcessQmpRun(qemuProcessQmpPtr cmd);
 
 void qemuProcessQmpStop(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 07/36] qemu_process: Use qemuProcessQmp struct for a single process
Posted by Jiri Denemark 7 years, 1 month ago
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>

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