[PATCH] qemu_process: Separate VIR_PERF_EVENT_* setting into a function

Michal Privoznik posted 1 patch 3 years, 8 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/10e3eb5914707c31c0b7f3c92e6509eae2caf0de.1599490097.git.mprivozn@redhat.com
src/qemu/qemu_process.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
[PATCH] qemu_process: Separate VIR_PERF_EVENT_* setting into a function
Posted by Michal Privoznik 3 years, 8 months ago
When starting a domain, qemuProcessLaunch() iterates over all
VIR_PERF_EVENT_* values and (possibly) enables them. While there
is nothing wrong with the code, the for loop where its done makes
it harder to jump onto next block of code.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

Is this trivial enough to be pushed as such?

 src/qemu/qemu_process.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6b1644880a..dd60fb0ddf 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6700,6 +6700,25 @@ qemuProcessEnableDomainNamespaces(virQEMUDriverPtr driver,
 }
 
 
+static int
+qemuProcessEnablePerf(virDomainObjPtr vm)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    size_t i;
+
+    if (!(priv->perf = virPerfNew()))
+        return -1;
+
+    for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
+        if (vm->def->perf.events[i] == VIR_TRISTATE_BOOL_YES &&
+            virPerfEventEnable(priv->perf, i, vm->pid) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
 /**
  * qemuProcessLaunch:
  *
@@ -6733,7 +6752,6 @@ qemuProcessLaunch(virConnectPtr conn,
     g_autoptr(virQEMUDriverConfig) cfg = NULL;
     size_t nnicindexes = 0;
     g_autofree int *nicindexes = NULL;
-    size_t i;
 
     VIR_DEBUG("conn=%p driver=%p vm=%p name=%s if=%d asyncJob=%d "
               "incoming.launchURI=%s incoming.deferredURI=%s "
@@ -6885,15 +6903,10 @@ qemuProcessLaunch(virConnectPtr conn,
     if (qemuSetupCgroup(vm, nnicindexes, nicindexes) < 0)
         goto cleanup;
 
-    if (!(priv->perf = virPerfNew()))
+    VIR_DEBUG("Setting up domain perf (if required)");
+    if (qemuProcessEnablePerf(vm) < 0)
         goto cleanup;
 
-    for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
-        if (vm->def->perf.events[i] == VIR_TRISTATE_BOOL_YES &&
-            virPerfEventEnable(priv->perf, i, vm->pid) < 0)
-            goto cleanup;
-    }
-
     /* This must be done after cgroup placement to avoid resetting CPU
      * affinity */
     if (qemuProcessInitCpuAffinity(vm) < 0)
-- 
2.26.2

Re: [PATCH] qemu_process: Separate VIR_PERF_EVENT_* setting into a function
Posted by Ján Tomko 3 years, 8 months ago
On a Monday in 2020, Michal Privoznik wrote:
>When starting a domain, qemuProcessLaunch() iterates over all
>VIR_PERF_EVENT_* values and (possibly) enables them. While there
>is nothing wrong with the code, the for loop where its done makes

*it's

>it harder to jump onto next block of code.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
>
>Is this trivial enough to be pushed as such?
>

I don't think so.

> src/qemu/qemu_process.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano