On 5/3/21 4:56 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.
That's not nice. Thanks for looking into it and providing a fix!
Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
---
src/libxl/libxl_conf.c | 53 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 3ccb00ec35..3a969c38cd 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
+libxlDomainSetVcpuAffinities(virDomainDef *def,
+ libxl_ctx *ctx,
+ libxl_domain_build_info *b_info)
We should tweak the name of this function after moving it from libxl_domain.c.
Dropping 'Domain' is probably enough, e.g. libxlSetVcpuAffinities.
+{
+ libxl_bitmap *vcpu_affinity_array;
+ unsigned int vcpuid;
+ unsigned int vcpu_idx = 0;
libvirt typically uses size_t.
+ 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 = calloc(vcpu_idx, sizeof(libxl_bitmap));
Fails syntax-check. You'll need to use g_new0.
Patch 3 looks good, but I'll wait to push it until this one is ready. BTW, any
reason for splitting 3 and 4? It's nice for review, but unless I'm missing
something I think they should be squashed together.
Jim
+ 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 (libxlDomainSetVcpuAffinities(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)