[libvirt PATCH 14/16] qemu: use priv->logCtxt in qemuProcessLaunch

Ján Tomko posted 16 patches 4 years, 4 months ago
There is a newer version of this series
[libvirt PATCH 14/16] qemu: use priv->logCtxt in qemuProcessLaunch
Posted by Ján Tomko 4 years, 4 months ago
Remove the local variable in favor of the one stored in priv.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_process.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4a1fd753ee..17435c0ee9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7179,7 +7179,6 @@ qemuProcessLaunch(virConnectPtr conn,
     int ret = -1;
     int rv;
     int logfile = -1;
-    qemuDomainLogContext *logCtxt = NULL;
     qemuDomainObjPrivate *priv = vm->privateData;
     g_autoptr(virCommand) cmd = NULL;
     struct qemuProcessHookData hookData;
@@ -7228,25 +7227,24 @@ qemuProcessLaunch(virConnectPtr conn,
     hookData.cfg = cfg;
 
     VIR_DEBUG("Creating domain log file");
-    if (!(logCtxt = qemuDomainLogContextNew(driver, vm,
+    if (!(priv->logCtxt = qemuDomainLogContextNew(driver, vm,
                                             QEMU_DOMAIN_LOG_CONTEXT_MODE_START))) {
         virLastErrorPrefixMessage("%s", _("can't connect to virtlogd"));
         goto cleanup;
     }
-    priv->logCtxt = logCtxt;
-    logfile = qemuDomainLogContextGetWriteFD(logCtxt);
+    logfile = qemuDomainLogContextGetWriteFD(priv->logCtxt);
 
     if (qemuProcessGenID(vm, flags) < 0)
         goto cleanup;
 
     if (qemuExtDevicesStart(driver, vm,
-                            qemuDomainLogContextGetManager(logCtxt),
+                            qemuDomainLogContextGetManager(priv->logCtxt),
                             incoming != NULL) < 0)
         goto cleanup;
 
     VIR_DEBUG("Building emulator command line");
     if (!(cmd = qemuBuildCommandLine(driver,
-                                     qemuDomainLogContextGetManager(logCtxt),
+                                     qemuDomainLogContextGetManager(priv->logCtxt),
                                      driver->securityManager,
                                      vm,
                                      incoming ? incoming->launchURI : NULL,
@@ -7265,11 +7263,11 @@ qemuProcessLaunch(virConnectPtr conn,
                              VIR_HOOK_SUBOP_BEGIN) < 0)
         goto cleanup;
 
-    qemuLogOperation(vm, "starting up", cmd, logCtxt);
+    qemuLogOperation(vm, "starting up", cmd, priv->logCtxt);
 
-    qemuDomainObjCheckTaint(driver, vm, logCtxt, incoming != NULL);
+    qemuDomainObjCheckTaint(driver, vm, priv->logCtxt, incoming != NULL);
 
-    qemuDomainLogContextMarkPosition(logCtxt);
+    qemuDomainLogContextMarkPosition(priv->logCtxt);
 
     VIR_DEBUG("Building mount namespace");
 
@@ -7343,7 +7341,7 @@ qemuProcessLaunch(virConnectPtr conn,
     VIR_DEBUG("Waiting for handshake from child");
     if (virCommandHandshakeWait(cmd) < 0) {
         /* Read errors from child that occurred between fork and exec. */
-        qemuProcessReportLogError(logCtxt,
+        qemuProcessReportLogError(priv->logCtxt,
                                   _("Process exited prior to exec"));
         goto cleanup;
     }
@@ -7423,7 +7421,7 @@ qemuProcessLaunch(virConnectPtr conn,
         goto cleanup;
 
     VIR_DEBUG("Waiting for monitor to show up");
-    if (qemuProcessWaitForMonitor(driver, vm, asyncJob, logCtxt) < 0)
+    if (qemuProcessWaitForMonitor(driver, vm, asyncJob, priv->logCtxt) < 0)
         goto cleanup;
 
     if (qemuConnectAgent(driver, vm) < 0)
-- 
2.31.1

Re: [libvirt PATCH 14/16] qemu: use priv->logCtxt in qemuProcessLaunch
Posted by Peter Krempa 4 years, 4 months ago
On Wed, Oct 06, 2021 at 09:15:20 +0200, Ján Tomko wrote:
> Remove the local variable in favor of the one stored in priv.
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/qemu/qemu_process.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4a1fd753ee..17435c0ee9 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c

[...]

>  
>      if (qemuProcessGenID(vm, flags) < 0)
>          goto cleanup;
>  
>      if (qemuExtDevicesStart(driver, vm,
> -                            qemuDomainLogContextGetManager(logCtxt),
> +                            qemuDomainLogContextGetManager(priv->logCtxt),

I'm not a fan of making all the supporting processes log into the same
log file ...

>                              incoming != NULL) < 0)
>          goto cleanup;
>  
>      VIR_DEBUG("Building emulator command line");
>      if (!(cmd = qemuBuildCommandLine(driver,
> -                                     qemuDomainLogContextGetManager(logCtxt),
> +                                     qemuDomainLogContextGetManager(priv->logCtxt),
>                                       driver->securityManager,
>                                       vm,

.. as qemu itself, but this is prior art.

>                                       incoming ? incoming->launchURI : NULL,

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [libvirt PATCH 14/16] qemu: use priv->logCtxt in qemuProcessLaunch
Posted by Peter Krempa 4 years, 4 months ago
On Wed, Oct 06, 2021 at 09:15:20 +0200, Ján Tomko wrote:
> Remove the local variable in favor of the one stored in priv.
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  src/qemu/qemu_process.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4a1fd753ee..17435c0ee9 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c

[...]

> @@ -7228,25 +7227,24 @@ qemuProcessLaunch(virConnectPtr conn,
>      hookData.cfg = cfg;
>  
>      VIR_DEBUG("Creating domain log file");
> -    if (!(logCtxt = qemuDomainLogContextNew(driver, vm,
> +    if (!(priv->logCtxt = qemuDomainLogContextNew(driver, vm,
>                                              QEMU_DOMAIN_LOG_CONTEXT_MODE_START))) {a

Broken indentation.