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

Olaf Hering posted 1 patch 2 years, 11 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210505140632.15241-1-olaf@aepfle.de
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(-)
[PATCH v2] libxl: set vcpu affinity during domain creation
Posted by Olaf Hering 2 years, 11 months ago
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@aepfle.de>
---
 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,

Re: [PATCH v2] libxl: set vcpu affinity during domain creation
Posted by Daniel Henrique Barboza 2 years, 11 months ago

On 5/5/21 11: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@aepfle.de>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   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,
> 

Re: [PATCH v2] libxl: set vcpu affinity during domain creation
Posted by Jim Fehlig 2 years, 11 months ago
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@aepfle.de>

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,
>