Re: [PATCH v2] libxl: set vcpu affinity during domain creation

[Jim Fehlig <jfehlig@xxxxxxxx>]

Back from some time off...

On 5/5/21 8:06 AM, Olaf Hering wrote:
> Since Xen 4.5 libxl allows to set affinities during domain creation.
> This enables Xen to allocate the domain memory on NUMA systems close to
> the specified pcpus.
>
> Libvirt can now handle <domain/cputune/vcpupin> in domU.xml correctly.
>
> Without this change, Xen will create the domU and assign NUMA memory and
> vcpu affinities on its own. Later libvirt will adjust the affinity,
> which may move the vcpus away from the assigned NUMA node.
>
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>

I see Daniel reviewed this patch but it had not been pushed. I've added the r-b 
and pushed it.

Thanks!
Jim

> ---
>   src/libxl/libxl_conf.c   | 53 ++++++++++++++++++++++++++++++++++++++++
>   src/libxl/libxl_domain.c | 46 ----------------------------------
>   src/libxl/libxl_domain.h |  4 ---
>   3 files changed, 53 insertions(+), 50 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 6392d7795d..2a99626f92 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -283,6 +283,56 @@ libxlMakeChrdevStr(virDomainChrDef *def, char **buf)
>       return 0;
>   }
>   
> +static int
> +libxlSetVcpuAffinities(virDomainDef *def,
> +                       libxl_ctx *ctx,
> +                       libxl_domain_build_info *b_info)
> +{
> +    libxl_bitmap *vcpu_affinity_array;
> +    unsigned int vcpuid;
> +    unsigned int vcpu_idx = 0;
> +    virDomainVcpuDef *vcpu;
> +    bool has_vcpu_pin = false;
> +
> +    /* Get highest vcpuid with cpumask */
> +    for (vcpuid = 0; vcpuid < b_info->max_vcpus; vcpuid++) {
> +        vcpu = virDomainDefGetVcpu(def, vcpuid);
> +        if (!vcpu)
> +            continue;
> +        if (!vcpu->cpumask)
> +            continue;
> +        vcpu_idx = vcpuid;
> +        has_vcpu_pin = true;
> +    }
> +    /* Nothing to do */
> +    if (!has_vcpu_pin)
> +        return 0;
> +
> +    /* Adjust index */
> +    vcpu_idx++;
> +
> +    b_info->num_vcpu_hard_affinity = vcpu_idx;
> +    /* Will be released by libxl_domain_config_dispose */
> +    b_info->vcpu_hard_affinity = g_new0(libxl_bitmap, vcpu_idx);
> +    vcpu_affinity_array = b_info->vcpu_hard_affinity;
> +
> +    for (vcpuid = 0; vcpuid < vcpu_idx; vcpuid++) {
> +        libxl_bitmap *map = &vcpu_affinity_array[vcpuid];
> +        libxl_bitmap_init(map);
> +        /* libxl owns the bitmap */
> +        if (libxl_cpu_bitmap_alloc(ctx, map, 0))
> +            return -1;
> +        vcpu = virDomainDefGetVcpu(def, vcpuid);
> +        /* Apply the given mask, or allow unhandled vcpus to run anywhere */
> +        if (vcpu && vcpu->cpumask)
> +            virBitmapToDataBuf(vcpu->cpumask, map->map, map->size);
> +        else
> +            libxl_bitmap_set_any(map);
> +    }
> +    libxl_defbool_set(&b_info->numa_placement, false);
> +    return 0;
> +}
> +
>   static int
>   libxlMakeDomBuildInfo(virDomainDef *def,
>                         libxlDriverConfig *cfg,
> @@ -320,6 +370,9 @@ libxlMakeDomBuildInfo(virDomainDef *def,
>       for (i = 0; i < virDomainDefGetVcpus(def); i++)
>           libxl_bitmap_set((&b_info->avail_vcpus), i);
>   
> +    if (libxlSetVcpuAffinities(def, ctx, b_info))
> +        return -1;
> +
>       switch ((virDomainClockOffsetType) clock.offset) {
>       case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
>           if (clock.data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME)
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index d78765ad84..625e04a9b0 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -967,49 +967,6 @@ libxlDomainAutoCoreDump(libxlDriverPrivate *driver,
>       return 0;
>   }
>   
> -int
> -libxlDomainSetVcpuAffinities(libxlDriverPrivate *driver, virDomainObj *vm)
> -{
> -    g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
> -    virDomainVcpuDef *vcpu;
> -    libxl_bitmap map;
> -    virBitmap *cpumask = NULL;
> -    size_t i;
> -    int ret = -1;
> -
> -    libxl_bitmap_init(&map);
> -
> -    for (i = 0; i < virDomainDefGetVcpus(vm->def); ++i) {
> -        vcpu = virDomainDefGetVcpu(vm->def, i);
> -
> -        if (!vcpu->online)
> -            continue;
> -
> -        if (!(cpumask = vcpu->cpumask))
> -            cpumask = vm->def->cpumask;
> -
> -        if (!cpumask)
> -            continue;
> -
> -        if (virBitmapToData(cpumask, &map.map, (int *)&map.size) < 0)
> -            goto cleanup;
> -
> -        if (libxl_set_vcpuaffinity(cfg->ctx, vm->def->id, i, &map, NULL) != 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           _("Failed to pin vcpu '%zu' with libxenlight"), i);
> -            goto cleanup;
> -        }
> -
> -        libxl_bitmap_dispose(&map); /* Also returns to freshly-init'd state */
> -    }
> -
> -    ret = 0;
> -
> - cleanup:
> -    libxl_bitmap_dispose(&map);
> -    return ret;
> -}
> -
>   static int
>   libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config *d_config)
>   {
> @@ -1460,9 +1417,6 @@ libxlDomainStart(libxlDriverPrivate *driver,
>           goto destroy_dom;
>       }
>   
> -    if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> -        goto destroy_dom;
> -
>       if (!start_paused) {
>           libxlDomainUnpauseWrapper(cfg->ctx, domid);
>           virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> index cbe7ba19ba..928ee8f5cd 100644
> --- a/src/libxl/libxl_domain.h
> +++ b/src/libxl/libxl_domain.h
> @@ -124,10 +124,6 @@ int
>   libxlDomainAutoCoreDump(libxlDriverPrivate *driver,
>                           virDomainObj *vm);
>   
> -int
> -libxlDomainSetVcpuAffinities(libxlDriverPrivate *driver,
> -                             virDomainObj *vm);
> -
>   int
>   libxlDomainStartNew(libxlDriverPrivate *driver,
>                       virDomainObj *vm,