From: Wim ten Have <wim.ten.have@oracle.com>

This patch generates a NUMA distance-aware libxl description from the
information extracted from a NUMA distance-aware libvirt XML file.

By default, if no NUMA node distance information is supplied in the
libvirt XML file, this patch uses the distances 10 for local and 21 for
remote nodes/sockets."

Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
---
 src/libxl/libxl_conf.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 04d9dd1..3e1a759 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -617,6 +617,129 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
     return 0;
 }
 
+#ifdef LIBXL_HAVE_VNUMA
+static int
+libxlMakeVnumaList(
+    virDomainDefPtr def,
+    libxl_ctx *ctx,
+    libxl_domain_config *d_config)
+{
+    int ret = -1;
+    char *vcpus = NULL;
+    size_t i, j;
+    size_t nr_nodes;
+    size_t num_vnuma;
+    virDomainNumaPtr numa = def->numa;
+    libxl_domain_build_info *b_info = &d_config->b_info;
+    libxl_physinfo physinfo;
+    libxl_vnode_info *vnuma_nodes = NULL;
+
+    if (!numa)
+        return 0;
+
+    num_vnuma = virDomainNumaGetNodeCount(numa);
+    if (!num_vnuma)
+        return 0;
+
+    libxl_physinfo_init(&physinfo);
+    if (libxl_get_physinfo(ctx, &physinfo) < 0) {
+        libxl_physinfo_dispose(&physinfo);
+        VIR_WARN("libxl_get_physinfo failed");
+        goto cleanup;
+    }
+    nr_nodes = physinfo.nr_nodes;
+    libxl_physinfo_dispose(&physinfo);
+
+    if (num_vnuma > nr_nodes) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Number of configured numa cells %zu exceeds the physical available nodes %zu"),
+                       num_vnuma, nr_nodes);
+        goto cleanup;
+    }
+
+    /*
+     * allocate the vnuma_nodes for assignment under b_info.
+     */
+    if (VIR_ALLOC_N(vnuma_nodes, num_vnuma) < 0)
+        goto cleanup;
+
+    /*
+     * parse the vnuma vnodes data.
+     */
+    for (i = 0; i < num_vnuma; i++) {
+        int cpu;
+        virBitmapPtr bitmap = NULL;
+        libxl_bitmap vcpu_bitmap;
+        libxl_vnode_info *p = &vnuma_nodes[i];
+
+        libxl_vnode_info_init(p);
+
+        /* pnode */
+        p->pnode = i;
+
+        /* memory size */
+        p->memkb = virDomainNumaGetNodeMemorySize(numa, i);
+
+        /* vcpus */
+        vcpus = virBitmapFormat(virDomainNumaGetNodeCpumask(numa, i));
+        if (vcpus == NULL) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("vnuma sibling %zu missing vcpus set"), i);
+            goto cleanup;
+        }
+
+        if (virBitmapParse(vcpus, &bitmap, VIR_DOMAIN_CPUMASK_LEN) < 0)
+            goto cleanup;
+
+        if ((cpu = virBitmapNextSetBit(bitmap, -1)) < 0) {
+            VIR_FREE(bitmap);
+            goto cleanup;
+        }
+
+        libxl_bitmap_init(&vcpu_bitmap);
+        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_bitmap, b_info->max_vcpus)) {
+            virReportOOMError();
+            goto cleanup;
+        }
+
+        do {
+            libxl_bitmap_set(&vcpu_bitmap, cpu);
+        } while ((cpu = virBitmapNextSetBit(bitmap, cpu)) >= 0);
+
+        libxl_bitmap_copy_alloc(ctx, &p->vcpus, &vcpu_bitmap);
+        libxl_bitmap_dispose(&vcpu_bitmap);
+
+        VIR_FREE(bitmap);
+        VIR_FREE(vcpus);
+
+        /* vdistances */
+        if (VIR_ALLOC_N(p->distances, num_vnuma) < 0)
+            goto cleanup;
+        p->num_distances = num_vnuma;
+
+        /*
+         * Apply the configured distance value. If not
+         * available set 10 for local or 21 for remote nodes.
+         */
+        for (j = 0; j < num_vnuma; j++)
+            p->distances[j] = virDomainNumaGetNodeDistance(numa, i, j) ?:
+                                (i == j ? 10 : 21);
+    }
+
+    b_info->vnuma_nodes = vnuma_nodes;
+    b_info->num_vnuma_nodes = num_vnuma;
+
+    ret = 0;
+
+ cleanup:
+    if (ret)
+        VIR_FREE(vnuma_nodes);
+    VIR_FREE(vcpus);
+
+    return ret;
+}
+#endif
+
 static int
 libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
 {
@@ -2207,6 +2330,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
     if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0)
         return -1;
 
+#ifdef LIBXL_HAVE_VNUMA
+    if (libxlMakeVnumaList(def, ctx, d_config) < 0)
+        return -1;
+#endif
+
     if (libxlMakeDiskList(def, d_config) < 0)
         return -1;
 
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 06/12/2017 12:54 PM, Wim Ten Have wrote:
> From: Wim ten Have <wim.ten.have@oracle.com>
>
> This patch generates a NUMA distance-aware libxl description from the
> information extracted from a NUMA distance-aware libvirt XML file.
>
> By default, if no NUMA node distance information is supplied in the
> libvirt XML file, this patch uses the distances 10 for local and 21 for
> remote nodes/sockets."
>
> Signed-off-by: Wim ten Have <wim.ten.have@oracle.com>
> ---
>  src/libxl/libxl_conf.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 128 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 04d9dd1..3e1a759 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -617,6 +617,129 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>      return 0;
>  }
>
> +#ifdef LIBXL_HAVE_VNUMA
> +static int
> +libxlMakeVnumaList(
> +    virDomainDefPtr def,
> +    libxl_ctx *ctx,
> +    libxl_domain_config *d_config)

The usual pattern is

static int
libxlMakeVnumaList(virDomainDefPtr def,
                    libxl_ctx *ctx,
                    libxl_domain_config *d_config)

> +{
> +    int ret = -1;
> +    char *vcpus = NULL;
> +    size_t i, j;
> +    size_t nr_nodes;
> +    size_t num_vnuma;
> +    virDomainNumaPtr numa = def->numa;
> +    libxl_domain_build_info *b_info = &d_config->b_info;
> +    libxl_physinfo physinfo;
> +    libxl_vnode_info *vnuma_nodes = NULL;
> +
> +    if (!numa)
> +        return 0;
> +
> +    num_vnuma = virDomainNumaGetNodeCount(numa);
> +    if (!num_vnuma)
> +        return 0;
> +
> +    libxl_physinfo_init(&physinfo);
> +    if (libxl_get_physinfo(ctx, &physinfo) < 0) {
> +        libxl_physinfo_dispose(&physinfo);
> +        VIR_WARN("libxl_get_physinfo failed");

The function, and hence domain startup, is going to fail, so we'll need proper 
error reporting here.

> +        goto cleanup;
> +    }
> +    nr_nodes = physinfo.nr_nodes;
> +    libxl_physinfo_dispose(&physinfo);
> +
> +    if (num_vnuma > nr_nodes) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Number of configured numa cells %zu exceeds the physical available nodes %zu"),
> +                       num_vnuma, nr_nodes);
> +        goto cleanup;
> +    }
> +
> +    /*
> +     * allocate the vnuma_nodes for assignment under b_info.
> +     */
> +    if (VIR_ALLOC_N(vnuma_nodes, num_vnuma) < 0)
> +        goto cleanup;
> +
> +    /*
> +     * parse the vnuma vnodes data.
> +     */
> +    for (i = 0; i < num_vnuma; i++) {
> +        int cpu;
> +        virBitmapPtr bitmap = NULL;
> +        libxl_bitmap vcpu_bitmap;
> +        libxl_vnode_info *p = &vnuma_nodes[i];
> +
> +        libxl_vnode_info_init(p);
> +
> +        /* pnode */
> +        p->pnode = i;
> +
> +        /* memory size */
> +        p->memkb = virDomainNumaGetNodeMemorySize(numa, i);
> +
> +        /* vcpus */
> +        vcpus = virBitmapFormat(virDomainNumaGetNodeCpumask(numa, i));
> +        if (vcpus == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("vnuma sibling %zu missing vcpus set"), i);
> +            goto cleanup;
> +        }
> +
> +        if (virBitmapParse(vcpus, &bitmap, VIR_DOMAIN_CPUMASK_LEN) < 0)
> +            goto cleanup;

Why the need to format and parse the bitmap returned by 
virtDomainNumaGetNodeCpumask? Can it be used directly where 'bitmap' is used below?

> +
> +        if ((cpu = virBitmapNextSetBit(bitmap, -1)) < 0) {
> +            VIR_FREE(bitmap);
> +            goto cleanup;
> +        }
> +
> +        libxl_bitmap_init(&vcpu_bitmap);
> +        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_bitmap, b_info->max_vcpus)) {
> +            virReportOOMError();

It looks like 'bitmap' will leak on this error path.

> +            goto cleanup;
> +        }
> +
> +        do {
> +            libxl_bitmap_set(&vcpu_bitmap, cpu);
> +        } while ((cpu = virBitmapNextSetBit(bitmap, cpu)) >= 0);
> +
> +        libxl_bitmap_copy_alloc(ctx, &p->vcpus, &vcpu_bitmap);
> +        libxl_bitmap_dispose(&vcpu_bitmap);
> +
> +        VIR_FREE(bitmap);
> +        VIR_FREE(vcpus);

'vcpus' is freed in cleanup.

> +
> +        /* vdistances */
> +        if (VIR_ALLOC_N(p->distances, num_vnuma) < 0)
> +            goto cleanup;
> +        p->num_distances = num_vnuma;
> +
> +        /*
> +         * Apply the configured distance value. If not
> +         * available set 10 for local or 21 for remote nodes.
> +         */
> +        for (j = 0; j < num_vnuma; j++)
> +            p->distances[j] = virDomainNumaGetNodeDistance(numa, i, j) ?:
> +                                (i == j ? 10 : 21);

Is the '?:' shorthand a GNU extension? I could only find one other use of it in 
the codebase, in src/vz/vz_sdk.c. I like the concise form, but also fear it is 
difficult to read.

Regards,
Jim

> +    }
> +
> +    b_info->vnuma_nodes = vnuma_nodes;
> +    b_info->num_vnuma_nodes = num_vnuma;
> +
> +    ret = 0;
> +
> + cleanup:
> +    if (ret)
> +        VIR_FREE(vnuma_nodes);
> +    VIR_FREE(vcpus);
> +
> +    return ret;
> +}
> +#endif
> +
>  static int
>  libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
>  {
> @@ -2207,6 +2330,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>      if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0)
>          return -1;
>
> +#ifdef LIBXL_HAVE_VNUMA
> +    if (libxlMakeVnumaList(def, ctx, d_config) < 0)
> +        return -1;
> +#endif
> +
>      if (libxlMakeDiskList(def, d_config) < 0)
>          return -1;
>
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list