Re: qemu: directly create virResctrlInfo ignoring capabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This patch introduced a bug and broke the 'resctrl' feature.

It introduced a 'divide by zero' error if you defined any 'resctrl'

allocation group through <cputune/cachetune/cache>.


Reason is 'caps->resctrl' is fully initialized through two steps, 'virResctrlInfoNew'

invokes 'virResctrlGetInfo' completes the first step, later, 'virResctrlInfoGetCache'

accomplishes the filling of 'caps->resctrl->levels->types->control.granularity'.


The simplest way to fix the bug is drawback this patch, but still have the undesirable

overhead.


Another way to fix but not that simple is moving the 'caps->host.cache' object

into 'virresctrl'. If no one takes this task, I can do it and need some days for it.


Br

Huaqiang

[PATCH 29/30] qemu: directly create virResctrlInfo ignoring capabilities
From: Daniel P. Berrangé <berrange redhat com>
To: libvir-list redhat com
Subject: [PATCH 29/30] qemu: directly create virResctrlInfo ignoring capabilities
Date: Wed, 4 Dec 2019 14:21:12 +0000
We always refresh the capabilities object when using virResctrlInfo
during process startup. This is undesirable overhead, because we can
just directly create a virResctrlInfo instead.

Signed-off-by: Daniel P. Berrangé <berrange redhat com>
---
 src/qemu/qemu_process.c | 24 ++++++++----------------
 src/util/virresctrl.h   |  2 ++
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3a3860b1a3..2b35680abc 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2724,29 +2724,24 @@ qemuProcessSetupEmulator(virDomainObjPtr vm)


 static int
-qemuProcessResctrlCreate(virQEMUDriverPtr driver,
-                         virDomainObjPtr vm)
+qemuProcessResctrlCreate(virDomainObjPtr vm)
 {
-    int ret = -1;
     size_t i = 0;
-    virCapsPtr caps = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    g_autoptr(virResctrlInfo) resctrl = NULL;

     if (!vm->def->nresctrls)
         return 0;

-    /* Force capability refresh since resctrl info can change
-     * XXX: move cache info into virresctrl so caps are not needed */
-    caps = virQEMUDriverGetCapabilities(driver, true);
-    if (!caps)
+    if (!(resctrl = virResctrlInfoNew()))
         return -1;

     for (i = 0; i < vm->def->nresctrls; i++) {
         size_t j = 0;
-        if (virResctrlAllocCreate(caps->host.resctrl,
+        if (virResctrlAllocCreate(resctrl,
vm->def->resctrls[i]->alloc,
                                   priv->machineName) < 0)
-            goto cleanup;
+            return -1;

         for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
             virDomainResctrlMonDefPtr mon = NULL;
@@ -2754,14 +2749,11 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
             mon = vm->def->resctrls[i]->monitors[j];
             if (virResctrlMonitorCreate(mon->instance,
                                         priv->machineName) < 0)
-                goto cleanup;
+                return -1;
         }
     }

-    ret = 0;
- cleanup:
-    virObjectUnref(caps);
-    return ret;
+    return 0;
 }


@@ -6882,7 +6874,7 @@ qemuProcessLaunch(virConnectPtr conn,
         goto cleanup;

     VIR_DEBUG("Setting up resctrl");
-    if (qemuProcessResctrlCreate(driver, vm) < 0)
+    if (qemuProcessResctrlCreate(vm) < 0)
         goto cleanup;

     VIR_DEBUG("Setting up managed PR daemon");
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index 3dd7c96348..759320d0fd 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -100,6 +100,8 @@ typedef virResctrlInfo *virResctrlInfoPtr;
 virResctrlInfoPtr
 virResctrlInfoNew(void);

+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virResctrlInfo, virObjectUnref);
+
 int
 virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
                        unsigned int level,


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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux