This reverts commit 7be5fe66cd024b9ffba7c52cdbf5effedeac2c0d.

This commit broke resctrl, because it missed the fact that the
virResctrlInfoGetCache() has side-effects causing it to actually
change the virResctrlInfo parameter, not merely get data from
it.

This code will need some refactoring before we can try separating
it from virCapabilities again.

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

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 10259027c9..23cc528817 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2724,24 +2724,29 @@ qemuProcessSetupEmulator(virDomainObjPtr vm)
 
 
 static int
-qemuProcessResctrlCreate(virDomainObjPtr vm)
+qemuProcessResctrlCreate(virQEMUDriverPtr driver,
+                         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;
 
-    if (!(resctrl = virResctrlInfoNew()))
+    /* 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)
         return -1;
 
     for (i = 0; i < vm->def->nresctrls; i++) {
         size_t j = 0;
-        if (virResctrlAllocCreate(resctrl,
+        if (virResctrlAllocCreate(caps->host.resctrl,
                                   vm->def->resctrls[i]->alloc,
                                   priv->machineName) < 0)
-            return -1;
+            goto cleanup;
 
         for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
             virDomainResctrlMonDefPtr mon = NULL;
@@ -2749,11 +2754,14 @@ qemuProcessResctrlCreate(virDomainObjPtr vm)
             mon = vm->def->resctrls[i]->monitors[j];
             if (virResctrlMonitorCreate(mon->instance,
                                         priv->machineName) < 0)
-                return -1;
+                goto cleanup;
         }
     }
 
-    return 0;
+    ret = 0;
+ cleanup:
+    virObjectUnref(caps);
+    return ret;
 }
 
 
@@ -6881,7 +6889,7 @@ qemuProcessLaunch(virConnectPtr conn,
         goto cleanup;
 
     VIR_DEBUG("Setting up resctrl");
-    if (qemuProcessResctrlCreate(vm) < 0)
+    if (qemuProcessResctrlCreate(driver, vm) < 0)
         goto cleanup;
 
     VIR_DEBUG("Setting up managed PR daemon");
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index 759320d0fd..3dd7c96348 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -100,8 +100,6 @@ typedef virResctrlInfo *virResctrlInfoPtr;
 virResctrlInfoPtr
 virResctrlInfoNew(void);
 
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(virResctrlInfo, virObjectUnref);
-
 int
 virResctrlInfoGetCache(virResctrlInfoPtr resctrl,
                        unsigned int level,
-- 
2.23.0

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

On 12/10/19 5:26 AM, Daniel P. Berrangé wrote:
> This reverts commit 7be5fe66cd024b9ffba7c52cdbf5effedeac2c0d.
> 
> This commit broke resctrl, because it missed the fact that the
> virResctrlInfoGetCache() has side-effects causing it to actually
> change the virResctrlInfo parameter, not merely get data from
> it.
> 
> This code will need some refactoring before we can try separating
> it from virCapabilities again.

Reviewed-by: Cole Robinson <crobinso@redhat.com>

- Cole

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