[libvirt] qemu: directly create virResctrlInfo ignoring capabilities

Wang Huaqiang huaqiang.wang at intel.com
Tue Dec 10 07:27:06 UTC 2019


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,





More information about the libvir-list mailing list