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

[libvirt] [PATCH 29/30] qemu: directly create virResctrlInfo ignoring capabilities From: Daniel P. Berrangé <berrange redhat com> To: libvir-list redhat com Subject: [libvirt] [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,