thr3ads.net - Xen devel - [RESEND PATCH v5 0/9] vcpu soft affinity for credit1 (first half only) [Dec 2013]

If this information is useful, please help other people find it:
Share via:

Dario Faggioli

2013-Dec-07 00:04 UTC

[RESEND PATCH v5 0/9] vcpu soft affinity for credit1 (first half only)

I almost kept the subject line as it was in actual v5, for making it easier to
''identify'' this series. However, this is *only* a resend of
the first half of
the soft affinity series, which, actually, doesn''t contain any bit of
the soft
affinity implementation. What''s inside here is:
 - the node-wise vcpu pinning specification for xl, which was around and pretty
   much all acked for months,
 - a couple of fixes for libxl, all acked as well.

Therefore, as discussed and decided already during the actual soft affinity
series review, these patches, and only these ones, are 4.4 material.

As said, everything is Acked-by already *except* for patch 4 ("libxl: move
libxl_{cpu,node}_bitmap_alloc()"), which is only code motion and no
functional
change, and which I included in this resend after explicit request of the tools
maintainers to do whatever it takes to keep motion and functional changes
separate.  Another thing is, in patch 6 ("libxc/libxl: allow to retrieve
the
number of online pCPUs"), I added a comment, next to the newly introduced
function declaration, explaining that it''s up to the caller to use the
information he gets correctly.

Both these things should be super-straightforward. :-)

There is a git branch from where the series can be pulled:

 git://xenbits.xen.org/people/dariof/xen.git numa/per-vcpu-affinity-v5b

Let me know if I can do anything else...

Thanks and Regards,
Dario

---

Dario Faggioli (9):
      xl: match output of vcpu-list with pinning syntax
      libxl: better name for parameters in libxl_list_vcpu
      libxl: fix memory leak in libxl_list_vcpu
      libxl: move libxl_{cpu,node}_bitmap_alloc()
      libxc/libxl: sanitize error handling in *_get_max_{cpus,nodes}
      libxc/libxl: allow to retrieve the number of online pCPUs
      xl: allow for node-wise specification of vcpu pinning
      xl: implement and enable dryrun mode for `xl vcpu-pin''
      xl: test script for the cpumap parser (for vCPU pinning)


 docs/man/xl.cfg.pod.5                           |   20 +-
 tools/libxc/xc_misc.c                           |   32 ++-
 tools/libxc/xenctrl.h                           |    3 
 tools/libxl/check-xl-vcpupin-parse              |  294 +++++++++++++++++++++++
 tools/libxl/check-xl-vcpupin-parse.data-example |   53 ++++
 tools/libxl/libxl.c                             |   49 ++--
 tools/libxl/libxl.h                             |   10 +
 tools/libxl/libxl_utils.c                       |   67 +++++
 tools/libxl/libxl_utils.h                       |   29 --
 tools/libxl/xl_cmdimpl.c                        |  228 ++++++++++++------
 tools/libxl/xl_cmdtable.c                       |    2 
 tools/python/xen/lowlevel/xc/xc.c               |    6 
 12 files changed, 655 insertions(+), 138 deletions(-)
 create mode 100755 tools/libxl/check-xl-vcpupin-parse
 create mode 100644 tools/libxl/check-xl-vcpupin-parse.data-example

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Dario Faggioli

2013-Dec-07 00:04 UTC

head link

[RESEND PATCH v5 1/9] xl: match output of vcpu-list with pinning syntax

in fact, pinning to all the pcpus happens by specifying "all"
(either on the command line or in the config file), while `xl
vcpu-list'' report it as "any cpu".

Change this into something more consistent, by using "all"
everywhere.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes since v1:
 * this patch was not there in v1. It is now as using the
   same syntax for both input and output was requested
   during review.
---
 tools/libxl/xl_cmdimpl.c |   27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4977a53..5bdb869 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3121,8 +3121,7 @@ out:
     }
 }
 
-/* If map is not full, prints it and returns 0. Returns 1 otherwise. */
-static int print_bitmap(uint8_t *map, int maplen, FILE *stream)
+static void print_bitmap(uint8_t *map, int maplen, FILE *stream)
 {
     int i;
     uint8_t pmap = 0, bitmask = 0;
@@ -3160,28 +3159,16 @@ static int print_bitmap(uint8_t *map, int maplen, FILE
*stream)
         case 2:
             break;
         case 1:
-            if (firstset == 0)
-                return 1;
+            if (firstset == 0) {
+                fprintf(stream, "all");
+                break;
+            }
         case 3:
             fprintf(stream, "%s%d", state > 1 ? "," :
"", firstset);
             if (i - 1 > firstset)
                 fprintf(stream, "-%d", i - 1);
             break;
     }
-
-    return 0;
-}
-
-static void print_cpumap(uint8_t *map, int maplen, FILE *stream)
-{
-    if (print_bitmap(map, maplen, stream))
-        fprintf(stream, "any cpu");
-}
-
-static void print_nodemap(uint8_t *map, int maplen, FILE *stream)
-{
-    if (print_bitmap(map, maplen, stream))
-        fprintf(stream, "any node");
 }
 
 static void list_domains(int verbose, int context, int claim, int numa,
@@ -3254,7 +3241,7 @@ static void list_domains(int verbose, int context, int
claim, int numa,
             libxl_domain_get_nodeaffinity(ctx, info[i].domid, &nodemap);
 
             putchar('' '');
-            print_nodemap(nodemap.map, physinfo.nr_nodes, stdout);
+            print_bitmap(nodemap.map, physinfo.nr_nodes, stdout);
         }
         putchar(''\n'');
     }
@@ -4466,7 +4453,7 @@ static void print_vcpuinfo(uint32_t tdomid,
     /*      TIM */
     printf("%9.1f  ", ((float)vcpuinfo->vcpu_time / 1e9));
     /* CPU AFFINITY */
-    print_cpumap(vcpuinfo->cpumap.map, nr_cpus, stdout);
+    print_bitmap(vcpuinfo->cpumap.map, nr_cpus, stdout);
     printf("\n");
 }

Dario Faggioli

2013-Dec-07 00:04 UTC

head link

[RESEND PATCH v5 2/9] libxl: better name for parameters in libxl_list_vcpu

so that the parameter that returns the number of pCPUs is
called nr_cpus_out, while the one that returns the number of
vCPUs is called nr_vcpus_out.

The patch is all about renaming, so no functional change.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes from v5:
 * renaming also nb_vcpu to nr_vcpus_out

Changes from v4:
 * new patch.
---
 tools/libxl/libxl.c |   15 +++++++++------
 tools/libxl/libxl.h |    2 +-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 771b45b..108da30 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4525,7 +4525,7 @@ const libxl_version_info* libxl_get_version_info(libxl_ctx
*ctx)
 }
 
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
-                                       int *nb_vcpu, int *nr_vcpus_out)
+                                       int *nr_vcpus_out, int *nr_cpus_out)
 {
     libxl_vcpuinfo *ptr, *ret;
     xc_domaininfo_t domaininfo;
@@ -4535,26 +4535,29 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t
domid,
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting infolist");
         return NULL;
     }
-    *nr_vcpus_out = libxl_get_max_cpus(ctx);
+    *nr_cpus_out = libxl_get_max_cpus(ctx);
     ret = ptr = calloc(domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
     if (!ptr) {
         return NULL;
     }
 
-    for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu,
++ptr) {
+    for (*nr_vcpus_out = 0;
+         *nr_vcpus_out <= domaininfo.max_vcpu_id;
+         ++*nr_vcpus_out, ++ptr) {
         if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0)) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating
cpumap");
             goto err;
         }
-        if (xc_vcpu_getinfo(ctx->xch, domid, *nb_vcpu, &vcpuinfo) == -1)
{
+        if (xc_vcpu_getinfo(ctx->xch, domid, *nr_vcpus_out, &vcpuinfo)
== -1) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu
info");
             goto err;
         }
-        if (xc_vcpu_getaffinity(ctx->xch, domid, *nb_vcpu,
ptr->cpumap.map) == -1) {
+        if (xc_vcpu_getaffinity(ctx->xch, domid, *nr_vcpus_out,
+                                ptr->cpumap.map) == -1) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu
affinity");
             goto err;
         }
-        ptr->vcpuid = *nb_vcpu;
+        ptr->vcpuid = *nr_vcpus_out;
         ptr->cpu = vcpuinfo.cpu;
         ptr->online = !!vcpuinfo.online;
         ptr->blocked = !!vcpuinfo.blocked;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 094ddd0..75b64ea 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -769,7 +769,7 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr);
 void libxl_numainfo_list_free(libxl_numainfo *, int nr);
 
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
-                                int *nb_vcpu, int *nr_vcpus_out);
+                                int *nb_vcpu, int *nr_cpus_out);
 void libxl_vcpuinfo_list_free(libxl_vcpuinfo *, int nr_vcpus);
 
 void libxl_device_vtpm_list_free(libxl_device_vtpm*, int nr_vtpms);

Dario Faggioli

2013-Dec-07 00:04 UTC

head link

[RESEND PATCH v5 3/9] libxl: fix memory leak in libxl_list_vcpu

more specifically, of the cpumap inside libxl_vcpuinfo, in case
of failure after it has been allocated.

While at it, use the correct libxl memory allocation wrapper for
calloc() in there and turn the function into using the new LOGE()
logging style.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes from v4:
 * new patch, wasn''t there in v4, but leak was identified during
   review.
---
 tools/libxl/libxl.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 108da30..2925e1e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4527,34 +4527,35 @@ const libxl_version_info*
libxl_get_version_info(libxl_ctx *ctx)
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
                                        int *nr_vcpus_out, int *nr_cpus_out)
 {
+    GC_INIT(ctx);
     libxl_vcpuinfo *ptr, *ret;
     xc_domaininfo_t domaininfo;
     xc_vcpuinfo_t vcpuinfo;
 
     if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting infolist");
+        LOGE(ERROR, "getting infolist");
+        GC_FREE;
         return NULL;
     }
     *nr_cpus_out = libxl_get_max_cpus(ctx);
-    ret = ptr = calloc(domaininfo.max_vcpu_id + 1, sizeof (libxl_vcpuinfo));
-    if (!ptr) {
-        return NULL;
-    }
+    ret = ptr = libxl__calloc(NOGC, domaininfo.max_vcpu_id + 1,
+                              sizeof(libxl_vcpuinfo));
 
     for (*nr_vcpus_out = 0;
          *nr_vcpus_out <= domaininfo.max_vcpu_id;
          ++*nr_vcpus_out, ++ptr) {
+        libxl_bitmap_init(&ptr->cpumap);
         if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0)) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating
cpumap");
+            LOGE(ERROR, "allocating cpumap");
             goto err;
         }
         if (xc_vcpu_getinfo(ctx->xch, domid, *nr_vcpus_out, &vcpuinfo)
== -1) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu
info");
+            LOGE(ERROR, "getting vcpu info");
             goto err;
         }
         if (xc_vcpu_getaffinity(ctx->xch, domid, *nr_vcpus_out,
                                 ptr->cpumap.map) == -1) {
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu
affinity");
+            LOGE(ERROR, "getting vcpu affinity");
             goto err;
         }
         ptr->vcpuid = *nr_vcpus_out;
@@ -4564,10 +4565,13 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t
domid,
         ptr->running = !!vcpuinfo.running;
         ptr->vcpu_time = vcpuinfo.cpu_time;
     }
+    GC_FREE;
     return ret;
 
 err:
+    libxl_bitmap_dispose(&ptr->cpumap);
     free(ret);
+    GC_FREE;
     return NULL;
 }

Dario Faggioli

2013-Dec-07 00:04 UTC

head link

[RESEND PATCH v5 4/9] libxl: move libxl_{cpu, node}_bitmap_alloc()

in libxl_utils.c (from .h), as they will be reworked in
the next commit ("libxc/libxl: sanitize error handling in
*_get_max_{cpus,nodes}") and we want to keep code motion
separate from functional changes.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Changes from v5:
 * new patch, as requested during review.
---
 tools/libxl/libxl_utils.c |   25 +++++++++++++++++++++++++
 tools/libxl/libxl_utils.h |   29 +++--------------------------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9f5f589..93f7a87 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -651,6 +651,31 @@ char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const
libxl_bitmap *bitmap)
     return q;
 }
 
+int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus)
+{
+    if (max_cpus < 0)
+        return ERROR_INVAL;
+    if (max_cpus == 0)
+        max_cpus = libxl_get_max_cpus(ctx);
+    if (max_cpus == 0)
+        return ERROR_FAIL;
+
+    return libxl_bitmap_alloc(ctx, cpumap, max_cpus);
+}
+
+int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
+                            int max_nodes)
+{
+    if (max_nodes < 0)
+        return ERROR_INVAL;
+    if (max_nodes == 0)
+        max_nodes = libxl_get_max_nodes(ctx);
+    if (max_nodes == 0)
+        return ERROR_FAIL;
+
+    return libxl_bitmap_alloc(ctx, nodemap, max_nodes);
+}
+
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
                             const libxl_bitmap *nodemap,
                             libxl_bitmap *cpumap)
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 7b84e6a..e37fb89 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -98,32 +98,9 @@ static inline int libxl_bitmap_cpu_valid(libxl_bitmap
*bitmap, int bit)
 #define libxl_for_each_set_bit(v, m) for (v = 0; v < (m).size * 8; v++) \
                                              if (libxl_bitmap_test(&(m),
v))
 
-static inline int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap,
-                                         int max_cpus)
-{
-    if (max_cpus < 0)
-        return ERROR_INVAL;
-    if (max_cpus == 0)
-        max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
-        return ERROR_FAIL;
-
-    return libxl_bitmap_alloc(ctx, cpumap, max_cpus);
-}
-
-static inline int libxl_node_bitmap_alloc(libxl_ctx *ctx,
-                                          libxl_bitmap *nodemap,
-                                          int max_nodes)
-{
-    if (max_nodes < 0)
-        return ERROR_INVAL;
-    if (max_nodes == 0)
-        max_nodes = libxl_get_max_nodes(ctx);
-    if (max_nodes == 0)
-        return ERROR_FAIL;
-
-    return libxl_bitmap_alloc(ctx, nodemap, max_nodes);
-}
+int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus);
+int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
+                            int max_nodes);
 
 /* Populate cpumap with the cpus spanned by the nodes in nodemap */
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,

Dario Faggioli

2013-Dec-07 00:05 UTC

head link

[RESEND PATCH v5 5/9] libxc/libxl: sanitize error handling in *_get_max_{cpus, nodes}

In libxc, make xc_get_max_{cpus,node}() always return either a
positive number or -1, and change all the callers to deal with
that.

In libxl, make libxl_get_max_{cpus,nodes}() always return either a
positive number or a libxl error code. Thanks to that, it is also
possible to fix loggig for libxl_{cpu,node}_bitmap_alloc(), which
now happens inside the functions themselves, more accurately
reporting what happened.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes from v5:
 * code motion of libxl_{cpu,node}_bitmap_alloc() happened in
   previous patch, so this one reworks the func in their new
   place, as requested during review.

Changes from v4:
 * fix error handling in xc_get_max_{cpus,nodes}() as well, as
   suggested during review;
 * no longer move libxl_{cpu,node}_bitmap_alloc() from .h to .c,
   as requested during review;
 * add more logging in libxl_{cpu,node}_bitmap_alloc(), as
   suggested during review;
 * propagate the error from libxl_get_max_{cpus,nodes}, as
   suggested during review;

Changes from v3:
 * switch the functions to LOG() / LOGE().
 * take care of the callers of libxl_get_max_{cpus,nodes}() too.

Changes from v2:
 * this wasn''t there in v2, but fixing this for v3 was requested
   during v2 review.
---
 tools/libxc/xc_misc.c             |   22 ++++++++++++---
 tools/libxl/libxl.c               |   16 ++++-------
 tools/libxl/libxl_utils.c         |   55 +++++++++++++++++++++++++++++--------
 tools/python/xen/lowlevel/xc/xc.c |    6 ++--
 4 files changed, 70 insertions(+), 29 deletions(-)

diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 56efe6a..c771469 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -30,9 +30,12 @@ int xc_get_max_cpus(xc_interface *xch)
         return max_cpus;
 
     if ( !xc_physinfo(xch, &physinfo) )
+    {
         max_cpus = physinfo.max_cpu_id + 1;
+        return max_cpus;
+    }
 
-    return max_cpus;
+    return -1;
 }
 
 int xc_get_max_nodes(xc_interface *xch)
@@ -44,19 +47,30 @@ int xc_get_max_nodes(xc_interface *xch)
         return max_nodes;
 
     if ( !xc_physinfo(xch, &physinfo) )
+    {
         max_nodes = physinfo.max_node_id + 1;
+        return max_nodes;
+    }
 
-    return max_nodes;
+    return -1;
 }
 
 int xc_get_cpumap_size(xc_interface *xch)
 {
-    return (xc_get_max_cpus(xch) + 7) / 8;
+    int max_cpus = xc_get_max_cpus(xch);
+
+    if ( max_cpus < 0 )
+        return -1;
+    return (max_cpus + 7) / 8;
 }
 
 int xc_get_nodemap_size(xc_interface *xch)
 {
-    return (xc_get_max_nodes(xch) + 7) / 8;
+    int max_nodes = xc_get_max_nodes(xch);
+
+    if ( max_nodes < 0 )
+        return -1;
+    return (max_nodes + 7) / 8;
 }
 
 xc_cpumap_t xc_cpumap_alloc(xc_interface *xch)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2925e1e..fd8b988 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -615,10 +615,8 @@ static int cpupool_info(libxl__gc *gc,
     info->n_dom = xcinfo->n_dom;
     rc = libxl_cpu_bitmap_alloc(CTX, &info->cpumap, 0);
     if (rc)
-    {
-        LOG(ERROR, "unable to allocate cpumap %d\n", rc);
         goto out;
-    }
+
     memcpy(info->cpumap.map, xcinfo->cpumap, info->cpumap.size);
 
     rc = 0;
@@ -4355,7 +4353,7 @@ libxl_cputopology *libxl_get_cpu_topology(libxl_ctx *ctx,
int *nb_cpu_out)
     int max_cpus;
 
     max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
+    if (max_cpus < 0)
     {
         LIBXL__LOG(ctx, XTL_ERROR, "Unable to determine number of
CPUS");
         ret = NULL;
@@ -4420,7 +4418,7 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int
*nr)
     int i, j, max_nodes;
 
     max_nodes = libxl_get_max_nodes(ctx);
-    if (max_nodes == 0)
+    if (max_nodes < 0)
     {
         LIBXL__LOG(ctx, XTL_ERROR, "Unable to determine number of
NODES");
         ret = NULL;
@@ -4545,10 +4543,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t
domid,
          *nr_vcpus_out <= domaininfo.max_vcpu_id;
          ++*nr_vcpus_out, ++ptr) {
         libxl_bitmap_init(&ptr->cpumap);
-        if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0)) {
-            LOGE(ERROR, "allocating cpumap");
+        if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0))
             goto err;
-        }
         if (xc_vcpu_getinfo(ctx->xch, domid, *nr_vcpus_out, &vcpuinfo)
== -1) {
             LOGE(ERROR, "getting vcpu info");
             goto err;
@@ -5308,8 +5304,8 @@ int libxl_get_freecpus(libxl_ctx *ctx, libxl_bitmap
*cpumap)
     int ncpus;
 
     ncpus = libxl_get_max_cpus(ctx);
-    if (ncpus == 0)
-        return ERROR_FAIL;
+    if (ncpus < 0)
+        return ncpus;
 
     cpumap->map = xc_cpupool_freeinfo(ctx->xch);
     if (cpumap->map == NULL)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 93f7a87..0833de2 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -653,27 +653,54 @@ char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const
libxl_bitmap *bitmap)
 
 int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus)
 {
-    if (max_cpus < 0)
-        return ERROR_INVAL;
+    GC_INIT(ctx);
+    int rc = 0;
+
+    if (max_cpus < 0) {
+        rc = ERROR_INVAL;
+        LOG(ERROR, "invalid number of cpus provided");
+        goto out;
+    }
     if (max_cpus == 0)
         max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
-        return ERROR_FAIL;
+    if (max_cpus < 0) {
+        LOG(ERROR, "failed to retrieve the maximum number of cpus");
+        rc = max_cpus;
+        goto out;
+    }
+    /* This can''t fail: no need to check and log */
+    libxl_bitmap_alloc(ctx, cpumap, max_cpus);
 
-    return libxl_bitmap_alloc(ctx, cpumap, max_cpus);
+ out:
+    GC_FREE;
+    return rc;
 }
 
 int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
                             int max_nodes)
 {
-    if (max_nodes < 0)
-        return ERROR_INVAL;
+    GC_INIT(ctx);
+    int rc = 0;
+
+    if (max_nodes < 0) {
+        rc = ERROR_INVAL;
+        LOG(ERROR, "invalid number of nodes provided");
+        goto out;
+    }
+
     if (max_nodes == 0)
         max_nodes = libxl_get_max_nodes(ctx);
-    if (max_nodes == 0)
-        return ERROR_FAIL;
+    if (max_nodes < 0) {
+        LOG(ERROR, "failed to retrieve the maximum number of nodes");
+        rc = max_nodes;
+        goto out;
+    }
+    /* This can''t fail: no need to check and log */
+    libxl_bitmap_alloc(ctx, nodemap, max_nodes);
 
-    return libxl_bitmap_alloc(ctx, nodemap, max_nodes);
+ out:
+    GC_FREE;
+    return rc;
 }
 
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
@@ -744,12 +771,16 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
 
 int libxl_get_max_cpus(libxl_ctx *ctx)
 {
-    return xc_get_max_cpus(ctx->xch);
+    int max_cpus = xc_get_max_cpus(ctx->xch);
+
+    return max_cpus < 0 ? ERROR_FAIL : max_cpus;
 }
 
 int libxl_get_max_nodes(libxl_ctx *ctx)
 {
-    return xc_get_max_nodes(ctx->xch);
+    int max_nodes = xc_get_max_nodes(ctx->xch);
+
+    return max_nodes < 0 ? ERROR_FAIL : max_nodes;
 }
 
 int libxl__enum_from_string(const libxl_enum_string_table *t,
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index 2625fc4..737bdac 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -233,7 +233,7 @@ static PyObject *pyxc_vcpu_setaffinity(XcObject *self,
         return NULL;
 
     nr_cpus = xc_get_max_cpus(self->xc_handle);
-    if ( nr_cpus == 0 )
+    if ( nr_cpus < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     cpumap = xc_cpumap_alloc(self->xc_handle);
@@ -392,7 +392,7 @@ static PyObject *pyxc_vcpu_getinfo(XcObject *self,
         return NULL;
 
     nr_cpus = xc_get_max_cpus(self->xc_handle);
-    if ( nr_cpus == 0 )
+    if ( nr_cpus < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     rc = xc_vcpu_getinfo(self->xc_handle, dom, vcpu, &info);
@@ -1923,7 +1923,7 @@ static PyObject *cpumap_to_cpulist(XcObject *self,
xc_cpumap_t cpumap)
     int nr_cpus;
 
     nr_cpus = xc_get_max_cpus(self->xc_handle);
-    if ( nr_cpus == 0 )
+    if ( nr_cpus < 0 )
         return pyxc_error_to_exception(self->xc_handle);
 
     cpulist = PyList_New(0);

Dario Faggioli

2013-Dec-07 00:05 UTC

head link

[RESEND PATCH v5 6/9] libxc/libxl: allow to retrieve the number of online pCPUs

by introducing introduce xc_get_online_cpus() and
libxl_get_online_cpus().

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes from v5:
 * add a comment about consistency.

Changes from v4:
 * introduced the xc call and call it from libxl, as
   suggested during review.
---
 tools/libxc/xc_misc.c     |   10 ++++++++++
 tools/libxc/xenctrl.h     |    3 +++
 tools/libxl/libxl.h       |    8 ++++++++
 tools/libxl/libxl_utils.c |    7 +++++++
 4 files changed, 28 insertions(+)

diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index c771469..00cd0d8 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -38,6 +38,16 @@ int xc_get_max_cpus(xc_interface *xch)
     return -1;
 }
 
+int xc_get_online_cpus(xc_interface *xch)
+{
+    xc_physinfo_t physinfo;
+
+    if ( !xc_physinfo(xch, &physinfo) )
+        return physinfo.nr_cpus;
+
+    return -1;
+}
+
 int xc_get_max_nodes(xc_interface *xch)
 {
     static int max_nodes = 0;
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index cced208..6e58ebe 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -356,6 +356,9 @@ typedef uint8_t *xc_cpumap_t;
 /* return maximum number of cpus the hypervisor supports */
 int xc_get_max_cpus(xc_interface *xch);
 
+/* return the number of online cpus */
+int xc_get_online_cpus(xc_interface *xch);
+
 /* return array size for cpumap */
 int xc_get_cpumap_size(xc_interface *xch);
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 75b64ea..12d6c31 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -677,6 +677,14 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid,
libxl_domain_create_in
 /* get max. number of cpus supported by hypervisor */
 int libxl_get_max_cpus(libxl_ctx *ctx);
 
+/* get the actual number of currently online cpus on the host */
+int libxl_get_online_cpus(libxl_ctx *ctx);
+  /* Beware that no locking or serialization is provided by libxl,
+   * so the information can be outdated as far as the function
+   * returns. If there are other entities in the system capable
+   * of onlining/offlining CPUs, it is up to the application
+   * to guarantee consistency, if that is important. */
+
 /* get max. number of NUMA nodes supported by hypervisor */
 int libxl_get_max_nodes(libxl_ctx *ctx);
 
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 0833de2..c9cef66 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -776,6 +776,13 @@ int libxl_get_max_cpus(libxl_ctx *ctx)
     return max_cpus < 0 ? ERROR_FAIL : max_cpus;
 }
 
+int libxl_get_online_cpus(libxl_ctx *ctx)
+{
+    int online_cpus = xc_get_online_cpus(ctx->xch);
+
+    return online_cpus < 0 ? ERROR_FAIL : online_cpus;
+}
+
 int libxl_get_max_nodes(libxl_ctx *ctx)
 {
     int max_nodes = xc_get_max_nodes(ctx->xch);

Dario Faggioli

2013-Dec-07 00:05 UTC

head link

[RESEND PATCH v5 7/9] xl: allow for node-wise specification of vcpu pinning

Making it possible to use something like the following:
 * "nodes:0-3": all pCPUs of nodes 0,1,2,3;
 * "nodes:0-3,^node:2": all pCPUS of nodes 0,1,3;
 * "1,nodes:1-2,^6": pCPU 1 plus all pCPUs of nodes 1,2
   but not pCPU 6;
 * ...

In both domain config file and `xl vcpu-pin''.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes from v1 (of this very series):
 * actually checking for both "nodes:" and "node:" as per
   the doc says;
 * using strcmp() (rather than strncmp()) when matching
   "all", to avoid returning success on any longer string
   that just begins with "all";
 * fixing the handling (well, the rejection, actually) of
   "^all" and "^nodes:all";
 * make some string pointers const.

Changes from v2 (of original series):
 * turned a ''return'' into ''goto out'',
consistently with the
   most of exit patterns;
 * harmonized error handling: now parse_range() return a
   libxl error code, as requested during review;
 * dealing with "all" moved inside update_cpumap_range().
   It''s tricky to move it in parse_range() (as requested
   during review), since we need the cpumap being modified
   handy when dealing with it. However, having it in
   update_cpumap_range() simplifies the code just as much
   as that;
 * explicitly checking for junk after a valid value or range
   in parse_range(), as requested during review;
 * xl exits on parsing failing, so no need to reset the
   cpumap to something sensible in vcpupin_parse(), as
   suggested during review;

Changes from v1 (of original series):
 * code rearranged in order to look more simple to follow
   and understand, as requested during review;
 * improved docs in xl.cfg.pod.5, as requested during
   review;
 * strtoul() now returns into unsigned long, and the
   case where it returns ULONG_MAX is now taken into
   account, as requested during review;
 * stuff like "all,^7" now works, as requested during
   review. Specifying just "^7" does not work either
   before or after this change
 * killed some magic (i.e., `ptr += 5 + (ptr[4] == ''s''`)
   by introducing STR_SKIP_PREFIX() macro, as requested
   during review.
---
 docs/man/xl.cfg.pod.5    |   20 ++++++
 tools/libxl/xl_cmdimpl.c |  153 +++++++++++++++++++++++++++++++++-------------
 2 files changed, 128 insertions(+), 45 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a979a13..72efd88 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -115,7 +115,25 @@ To allow all the vcpus of the guest to run on all the cpus
on the host.
 
 =item "0-3,5,^1"
 
-To allow all the vcpus of the guest to run on cpus 0,2,3,5.
+To allow all the vcpus of the guest to run on cpus 0,2,3,5. Combining
+this with "all" is possible, meaning "all,^7" results in
all the vcpus
+of the guest running on all the cpus on the host except cpu 7.
+
+=item "nodes:0-3,node:^2"
+
+To allow all the vcpus of the guest to run on the cpus from NUMA nodes
+0,1,3 of the host. So, if cpus 0-3 belongs to node 0, cpus 4-7 belongs
+to node 1 and cpus 8-11 to node 3, the above would mean all the vcpus
+of the guest will run on cpus 0-3,8-11.
+
+Combining this notation with the one above is possible. For instance,
+"1,node:2,^6", means all the vcpus of the guest will run on cpu 1 and
+on all the cpus of NUMA node 2, but not on cpu 6. Following the same
+example as above, that would be cpus 1,4,5,7.
+
+Combining this with "all" is also possible, meaning
"all,^nodes:1"
+results in all the vcpus of the guest running on all the cpus on the
+host, except for the cpus belonging to the host NUMA node 1.
 
 =item ["2", "3"] (or [2, 3])
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5bdb869..fa5916e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -59,6 +59,11 @@
         }                                                               \
     })
 
+#define STR_HAS_PREFIX( a, b )  \
+    ( strncmp(a, b, strlen(b)) == 0 )
+#define STR_SKIP_PREFIX( a, b ) \
+    ( STR_HAS_PREFIX(a, b) ? ((a) += strlen(b), 1) : 0 )
+
 
 int logfile = 2;
 
@@ -562,61 +567,121 @@ static void split_string_into_string_list(const char
*str,
     free(s);
 }
 
-static int vcpupin_parse(char *cpu, libxl_bitmap *cpumap)
+static int parse_range(const char *str, unsigned long *a, unsigned long *b)
 {
-    libxl_bitmap exclude_cpumap;
-    uint32_t cpuida, cpuidb;
-    char *endptr, *toka, *tokb, *saveptr = NULL;
-    int i, rc = 0, rmcpu;
+    const char *nstr;
+    char *endptr;
 
-    if (!strcmp(cpu, "all")) {
-        libxl_bitmap_set_any(cpumap);
-        return 0;
+    *a = *b = strtoul(str, &endptr, 10);
+    if (endptr == str || *a == ULONG_MAX)
+        return ERROR_INVAL;
+
+    if (*endptr == ''-'') {
+        nstr = endptr + 1;
+
+        *b = strtoul(nstr, &endptr, 10);
+        if (endptr == nstr || *b == ULONG_MAX || *b < *a)
+            return ERROR_INVAL;
+    }
+
+    /* Valid value or range so far, but we also don''t want junk after
that */
+    if (*endptr != ''\0'')
+        return ERROR_INVAL;
+
+    return 0;
+}
+
+/*
+ * Add or removes a specific set of cpus (specified in str, either as
+ * single cpus or as entire NUMA nodes) to/from cpumap.
+ */
+static int update_cpumap_range(const char *str, libxl_bitmap *cpumap)
+{
+    unsigned long ida, idb;
+    libxl_bitmap node_cpumap;
+    bool is_not = false, is_nodes = false;
+    int rc = 0;
+
+    libxl_bitmap_init(&node_cpumap);
+
+    rc = libxl_node_bitmap_alloc(ctx, &node_cpumap, 0);
+    if (rc) {
+        fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
+        goto out;
     }
 
-    if (libxl_cpu_bitmap_alloc(ctx, &exclude_cpumap, 0)) {
-        fprintf(stderr, "Error: Failed to allocate cpumap.\n");
-        return ENOMEM;
+    /* Are we adding or removing cpus/nodes? */
+    if (STR_SKIP_PREFIX(str, "^")) {
+        is_not = true;
     }
 
-    for (toka = strtok_r(cpu, ",", &saveptr); toka;
-         toka = strtok_r(NULL, ",", &saveptr)) {
-        rmcpu = 0;
-        if (*toka == ''^'') {
-            /* This (These) Cpu(s) will be removed from the map */
-            toka++;
-            rmcpu = 1;
-        }
-        /* Extract a valid (range of) cpu(s) */
-        cpuida = cpuidb = strtoul(toka, &endptr, 10);
-        if (endptr == toka) {
-            fprintf(stderr, "Error: Invalid argument.\n");
-            rc = EINVAL;
-            goto vcpp_out;
-        }
-        if (*endptr == ''-'') {
-            tokb = endptr + 1;
-            cpuidb = strtoul(tokb, &endptr, 10);
-            if (endptr == tokb || cpuida > cpuidb) {
-                fprintf(stderr, "Error: Invalid argument.\n");
-                rc = EINVAL;
-                goto vcpp_out;
+    /* Are we dealing with cpus or full nodes? */
+    if (STR_SKIP_PREFIX(str, "node:") || STR_SKIP_PREFIX(str,
"nodes:")) {
+        is_nodes = true;
+    }
+
+    if (strcmp(str, "all") == 0) {
+        /* We do not accept "^all" or "^nodes:all" */
+        if (is_not) {
+            fprintf(stderr, "Can''t combine \"^\" and
\"all\".\n");
+            rc = ERROR_INVAL;
+        } else
+            libxl_bitmap_set_any(cpumap);
+        goto out;
+    }
+
+    rc = parse_range(str, &ida, &idb);
+    if (rc) {
+        fprintf(stderr, "Invalid pcpu range: %s.\n", str);
+        goto out;
+    }
+
+    /* Add or remove the specified cpus in the range */
+    while (ida <= idb) {
+        if (is_nodes) {
+            /* Add/Remove all the cpus of a NUMA node */
+            int i;
+
+            rc = libxl_node_to_cpumap(ctx, ida, &node_cpumap);
+            if (rc) {
+                fprintf(stderr, "libxl_node_to_cpumap failed.\n");
+                goto out;
             }
+
+            /* Add/Remove all the cpus in the node cpumap */
+            libxl_for_each_set_bit(i, node_cpumap) {
+                is_not ? libxl_bitmap_reset(cpumap, i) :
+                         libxl_bitmap_set(cpumap, i);
+            }
+        } else {
+            /* Add/Remove this cpu */
+            is_not ? libxl_bitmap_reset(cpumap, ida) :
+                     libxl_bitmap_set(cpumap, ida);
         }
-        while (cpuida <= cpuidb) {
-            rmcpu == 0 ? libxl_bitmap_set(cpumap, cpuida) :
-                         libxl_bitmap_set(&exclude_cpumap, cpuida);
-            cpuida++;
-        }
+        ida++;
     }
 
-    /* Clear all the cpus from the removal list */
-    libxl_for_each_set_bit(i, exclude_cpumap) {
-        libxl_bitmap_reset(cpumap, i);
-    }
+ out:
+    libxl_bitmap_dispose(&node_cpumap);
+    return rc;
+}
 
-vcpp_out:
-    libxl_bitmap_dispose(&exclude_cpumap);
+/*
+ * Takes a string representing a set of cpus (specified either as
+ * single cpus or as eintire NUMA nodes) and turns it into the
+ * corresponding libxl_bitmap (in cpumap).
+ */
+static int vcpupin_parse(char *cpu, libxl_bitmap *cpumap)
+{
+    char *ptr, *saveptr = NULL;
+    int rc = 0;
+
+    for (ptr = strtok_r(cpu, ",", &saveptr); ptr;
+         ptr = strtok_r(NULL, ",", &saveptr)) {
+        rc = update_cpumap_range(ptr, cpumap);
+        if (rc)
+            break;
+    }
 
     return rc;
 }

Dario Faggioli

2013-Dec-07 00:05 UTC

head link

[RESEND PATCH v5 8/9] xl: implement and enable dryrun mode for `xl vcpu-pin''

As it can be useful to see if the outcome of some complex vCPU
pinning bitmap specification looks as expected.

This also allow for the introduction of some automatic testing
and verification for the bitmap parsing code, as it happens
already in check-xl-disk-parse and check-xl-vif-parse.

In particular, to make the above possible, this commit also
changes the implementation of the vcpu-pin command so that,
instead of always returning 0, it returns an error if the
parsing fails.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
Changes since v2 (of original series):
 * fixed a typo in the changelog
---
 tools/libxl/xl_cmdimpl.c  |   48 +++++++++++++++++++++++++++++++++------------
 tools/libxl/xl_cmdtable.c |    2 +-
 2 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index fa5916e..bd26bcc 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4586,40 +4586,62 @@ int main_vcpulist(int argc, char **argv)
     return 0;
 }
 
-static void vcpupin(uint32_t domid, const char *vcpu, char *cpu)
+static int vcpupin(uint32_t domid, const char *vcpu, char *cpu)
 {
     libxl_vcpuinfo *vcpuinfo;
     libxl_bitmap cpumap;
 
     uint32_t vcpuid;
     char *endptr;
-    int i, nb_vcpu;
+    int i, nb_cpu, nb_vcpu, rc = -1;
+
+    libxl_bitmap_init(&cpumap);
 
     vcpuid = strtoul(vcpu, &endptr, 10);
     if (vcpu == endptr) {
         if (strcmp(vcpu, "all")) {
             fprintf(stderr, "Error: Invalid argument.\n");
-            return;
+            goto out;
         }
         vcpuid = -1;
     }
 
-    if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0)) {
-        goto vcpupin_out;
-    }
+    if (libxl_cpu_bitmap_alloc(ctx, &cpumap, 0))
+        goto out;
 
     if (vcpupin_parse(cpu, &cpumap))
-        goto vcpupin_out1;
+        goto out;
+
+    if (dryrun_only) {
+        nb_cpu = libxl_get_online_cpus(ctx);
+        if (nb_cpu < 0) {
+            fprintf(stderr, "libxl_get_online_cpus failed.\n");
+            goto out;
+        }
+
+        fprintf(stdout, "cpumap: ");
+        print_bitmap(cpumap.map, nb_cpu, stdout);
+        fprintf(stdout, "\n");
+
+        if (ferror(stdout) || fflush(stdout)) {
+            perror("stdout");
+            exit(-1);
+        }
+
+        rc = 0;
+        goto out;
+    }
 
     if (vcpuid != -1) {
         if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap) == -1) {
             fprintf(stderr, "Could not set affinity for vcpu
`%u''.\n", vcpuid);
+            goto out;
         }
     }
     else {
         if (!(vcpuinfo = libxl_list_vcpu(ctx, domid, &nb_vcpu, &i))) {
             fprintf(stderr, "libxl_list_vcpu failed.\n");
-            goto vcpupin_out1;
+            goto out;
         }
         for (i = 0; i < nb_vcpu; i++) {
             if (libxl_set_vcpuaffinity(ctx, domid, vcpuinfo[i].vcpuid,
@@ -4630,10 +4652,11 @@ static void vcpupin(uint32_t domid, const char *vcpu,
char *cpu)
         }
         libxl_vcpuinfo_list_free(vcpuinfo, nb_vcpu);
     }
-  vcpupin_out1:
+
+    rc = 0;
+ out:
     libxl_bitmap_dispose(&cpumap);
-  vcpupin_out:
-    ;
+    return rc;
 }
 
 int main_vcpupin(int argc, char **argv)
@@ -4644,8 +4667,7 @@ int main_vcpupin(int argc, char **argv)
         /* No options */
     }
 
-    vcpupin(find_domain(argv[optind]), argv[optind+1] , argv[optind+2]);
-    return 0;
+    return vcpupin(find_domain(argv[optind]), argv[optind+1] , argv[optind+2]);
 }
 
 static void vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 7709206..ebe0220 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -211,7 +211,7 @@ struct cmd_spec cmd_table[] = {
       "[Domain, ...]",
     },
     { "vcpu-pin",
-      &main_vcpupin, 0, 1,
+      &main_vcpupin, 1, 1,
       "Set which CPUs a VCPU can use",
       "<Domain> <VCPU|all> <CPUs|all>",
     },

Dario Faggioli

2013-Dec-07 00:05 UTC

head link

[RESEND PATCH v5 9/9] xl: test script for the cpumap parser (for vCPU pinning)

This commit introduces "check-xl-vcpupin-parse" for helping
verifying and debugging the (v)CPU bitmap parsing code in xl.

The script runs "xl -N vcpu-pin 0 all <some strings>"
repeatedly, with various input strings, and checks that the
output is as expected.

This is what the script can do:

# ./check-xl-vcpupin-parse -h
 usage: ./check-xl-vcpupin-parse [options]

 Tests various vcpu-pinning strings. If run without arguments acts
 as follows:
  - generates some test data and saves them in
    check-xl-vcpupin-parse.data;
  - tests all the generated configurations (reading them back from
    check-xl-vcpupin-parse.data).

 An example of a test vector file is provided in
 check-xl-vcpupin-parse.data-example.

 Options:
  -h         prints this message
  -r seed    uses seed for initializing the rundom number generator
             (default: the script PID)
  -s string  tries using string as a vcpu pinning configuration and
             reports whether that succeeds or not
  -o ofile   save the test data in ofile
             (default: check-xl-vcpupin-parse.data)
  -i ifile   read test data from ifile

An example test data file (generated on a 2 NUMA nodes, 16 CPUs
host) is being provided in check-xl-vcpupin-parse.data-example.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes from v2 (of original series):
 * killed the `sleep 1'', as requested during review;
 * allow for passing a custom randon seed, and report what is
   the actual random seed used, as requested during review;
 * allow for testing for specific pinning configuration strings,
   as suggested during review;
 * stores the test data in a file, after generating them, and
   read them back from there for actual testing, as suggested
   during review;
 * allow for reading the test data from an existing test file
   instead than always generating new ones.

Changes from v1 (of original series):
 * this was not there in v1, and adding it has been requested
   during review.
---
 tools/libxl/check-xl-vcpupin-parse              |  294 +++++++++++++++++++++++
 tools/libxl/check-xl-vcpupin-parse.data-example |   53 ++++
 2 files changed, 347 insertions(+)
 create mode 100755 tools/libxl/check-xl-vcpupin-parse
 create mode 100644 tools/libxl/check-xl-vcpupin-parse.data-example

diff --git a/tools/libxl/check-xl-vcpupin-parse
b/tools/libxl/check-xl-vcpupin-parse
new file mode 100755
index 0000000..21f8421
--- /dev/null
+++ b/tools/libxl/check-xl-vcpupin-parse
@@ -0,0 +1,294 @@
+#!/bin/bash
+
+set -e
+
+if [ -x ./xl ] ; then
+    export LD_LIBRARY_PATH=.:../libxc:../xenstore:
+    XL=./xl
+else
+    XL=xl
+fi
+
+fprefix=tmp.check-xl-vcpupin-parse
+outfile=check-xl-vcpupin-parse.data
+
+usage () {
+cat <<END
+usage: $0 [options]
+
+Tests various vcpu-pinning strings. If run without arguments acts
+as follows:
+ - generates some test data and saves them in $outfile;
+ - tests all the generated configurations (reading them back from
+   $outfile).
+
+An example of a test vector file is provided in ${outfile}-example.
+
+Options:
+ -h         prints this message
+ -r seed    uses seed for initializing the rundom number generator
+            (default: the script PID)
+ -s string  tries using string as a vcpu pinning configuration and
+            reports whether that succeeds or not
+ -o ofile   save the test data in ofile (default: $outfile)
+ -i ifile   read test data from ifile
+END
+}
+
+expected () {
+    cat >$fprefix.expected
+}
+
+# by default, re-seed with our PID
+seed=$$
+failures=0
+
+# Execute one test and check the result against the provided
+# rc value and output
+one () {
+    expected_rc=$1; shift
+    printf "test case %s...\n" "$*"
+    set +e
+    ${XL} -N vcpu-pin 0 all "$@" </dev/null >$fprefix.actual
2>/dev/null
+    actual_rc=$?
+    if [ $actual_rc != $expected_rc ]; then
+        diff -u $fprefix.expected $fprefix.actual
+        echo >&2 "test case \`$*'' failed ($actual_rc
$diff_rc)"
+        failures=$(( $failures + 1 ))
+    fi
+    set -e
+}
+
+# Write an entry in the test vector file. Format is as follows:
+#  test-string*expected-rc*expected-output
+write () {
+    printf "$1*$2*$3\n" >> $outfile
+}
+
+complete () {
+    if [ "$failures" = 0 ]; then
+        echo all ok.; exit 0
+    else
+        echo "$failures tests failed."; exit 1
+    fi
+}
+
+# Test a specific pinning string
+string () {
+    expected_rc=$1; shift
+    printf "test case %s...\n" "$*"
+    set +e
+    ${XL} -N vcpu-pin 0 all "$@" &> /dev/null
+    actual_rc=$?
+    set -e
+
+    if [ $actual_rc != $expected_rc ]; then
+        echo >&2 "test case \`$*'' failed
($actual_rc)"
+    else
+        echo >&2 "test case \`$*'' succeeded"
+    fi
+
+    exit 0
+}
+
+# Read a test vector file (provided as $1) line by line and
+# test all the entries it contains
+run ()
+{
+    while read line
+    do
+        if [ ${line:0:1} != ''#'' ]; then
+            test_string="`echo $line | cut -f1
-d''*''`"
+            exp_rc="`echo $line | cut -f2 -d''*''`"
+            exp_output="`echo $line | cut -f3
-d''*''`"
+
+            expected <<END
+$exp_output
+END
+            one $exp_rc "$test_string"
+        fi
+    done < $1
+
+    complete
+
+    exit 0
+}
+
+while getopts "hr:s:o:i:" option
+do
+    case $option in
+    h)
+        usage
+        exit 0
+        ;;
+    r)
+        seed=$OPTARG
+        ;;
+    s)
+        string 0 "$OPTARG"
+        ;;
+    o)
+        outfile=$OPTARG
+        ;;
+    i)
+        run $OPTARG
+        ;;
+    esac
+done
+
+#---------- test data ----------
+#
+nr_cpus=`xl info | grep nr_cpus | cut -f2 -d'':''`
+nr_nodes=`xl info | grep nr_nodes | cut -f2 -d'':''`
+nr_cpus_per_node=`xl info -n | sed ''/cpu:/,/numa_info/!d'' |
head -n -1 | \
+    awk ''{print $4}'' | uniq -c | tail -1 | awk
''{print $1}''`
+cat >$outfile <<END
+# WARNING: some of these tests are topology based tests.
+# Expect failures if the topology is not detected correctly
+# detected topology: $nr_cpus CPUs, $nr_nodes nodes, $nr_cpus_per_node CPUs per
node.
+#
+# seed used for random number generation: seed=${seed}.
+#
+# Format is as follows:
+#  test-string*expected-return-code*expected-output
+#
+END
+
+# Re-seed the random number generator
+RANDOM=$seed
+
+echo "# Testing a wrong configuration" >> $outfile
+write foo 255 ""
+
+echo "# Testing the ''all'' syntax" >> $outfile
+write "all" 0 "cpumap: all"
+write "nodes:all" 0 "cpumap: all"
+write "all,nodes:all" 0 "cpumap: all"
+write "all,^nodes:0,all" 0 "cpumap: all"
+
+echo "# Testing the empty cpumap case" >> $outfile
+write "^0" 0 "cpumap: none"
+
+echo "# A few attempts of pinning to just one random cpu" >>
$outfile
+if [ $nr_cpus -gt 1 ]; then
+    for i in `seq 0 3`; do
+        cpu=$(($RANDOM % nr_cpus))
+        write "$cpu" 0 "cpumap: $cpu"
+    done
+fi
+
+echo "# A few attempts of pinning to all but one random cpu" >>
$outfile
+if [ $nr_cpus -gt 2 ]; then
+    for i in `seq 0 3`; do
+        cpu=$(($RANDOM % nr_cpus))
+        if [ $cpu -eq 0 ]; then
+            expected_range="1-$((nr_cpus - 1))"
+        elif [ $cpu -eq 1 ]; then
+            expected_range="0,2-$((nr_cpus - 1))"
+        elif [ $cpu -eq $((nr_cpus - 2)) ]; then
+            expected_range="0-$((cpu - 1)),$((nr_cpus - 1))"
+        elif [ $cpu -eq $((nr_cpus - 1)) ]; then
+            expected_range="0-$((nr_cpus - 2))"
+        else
+            expected_range="0-$((cpu - 1)),$((cpu + 1))-$((nr_cpus -
1))"
+        fi
+        write "all,^$cpu" 0 "cpumap: $expected_range"
+    done
+fi
+
+echo "# A few attempts of pinning to a random range of cpus" >>
$outfile
+if [ $nr_cpus -gt 2 ]; then
+    for i in `seq 0 3`; do
+        cpua=$(($RANDOM % nr_cpus))
+        range=$((nr_cpus - cpua))
+        cpub=$(($RANDOM % range))
+        cpubb=$((cpua + cpub))
+        if [ $cpua -eq $cpubb ]; then
+            expected_range="$cpua"
+        else
+            expected_range="$cpua-$cpubb"
+        fi
+        write "$expected_range" 0 "cpumap: $expected_range"
+    done
+fi
+
+echo "# A few attempts of pinning to just one random node" >>
$outfile
+if [ $nr_nodes -gt 1 ]; then
+    for i in `seq 0 3`; do
+        node=$(($RANDOM % nr_nodes))
+        # this assumes that the first $nr_cpus_per_node (from cpu
+        # 0 to cpu $nr_cpus_per_node-1) are assigned to the first node
+        # (node 0), the second $nr_cpus_per_node (from $nr_cpus_per_node
+        # to 2*$nr_cpus_per_node-1) are assigned to the second node (node
+        # 1), etc. Expect failures if that is not the case.
+        write "nodes:$node" 0 "cpumap:
$((nr_cpus_per_node*node))-$((nr_cpus_per_node*(node+1)-1))"
+    done
+fi
+
+echo "# A few attempts of pinning to all but one random node"
>> $outfile
+if [ $nr_nodes -gt 1 ]; then
+    for i in `seq 0 3`; do
+        node=$(($RANDOM % nr_nodes))
+        # this assumes that the first $nr_cpus_per_node (from cpu
+        # 0 to cpu $nr_cpus_per_node-1) are assigned to the first node
+        # (node 0), the second $nr_cpus_per_node (from $nr_cpus_per_node
+        # to 2*$nr_cpus_per_node-1) are assigned to the second node (node
+        # 1), etc. Expect failures if that is not the case.
+        if [ $node -eq 0 ]; then
+            expected_range="$nr_cpus_per_node-$((nr_cpus - 1))"
+        elif [ $node -eq $((nr_nodes - 1)) ]; then
+            expected_range="0-$((nr_cpus - nr_cpus_per_node - 1))"
+        else
+           
expected_range="0-$((nr_cpus_per_node*node-1)),$((nr_cpus_per_node*(node+1)))-$nr_cpus"
+        fi
+        write "all,^nodes:$node" 0 "cpumap:
$expected_range"
+    done
+fi
+
+echo "# A few attempts of pinning to a random range of nodes"
>> $outfile
+if [ $nr_nodes -gt 1 ]; then
+    for i in `seq 0 3`; do
+        nodea=$(($RANDOM % nr_nodes))
+        range=$((nr_nodes - nodea))
+        nodeb=$(($RANDOM % range))
+        nodebb=$((nodea + nodeb))
+        # this assumes that the first $nr_cpus_per_node (from cpu
+        # 0 to cpu $nr_cpus_per_node-1) are assigned to the first node
+        # (node 0), the second $nr_cpus_per_node (from $nr_cpus_per_node
+        # to 2*$nr_cpus_per_node-1) are assigned to the second node (node
+        # 1), etc. Expect failures if that is not the case.
+        if [ $nodea -eq 0 ] && [ $nodebb -eq $((nr_nodes - 1)) ]; then
+            expected_range="all"
+        else
+           
expected_range="$((nr_cpus_per_node*nodea))-$((nr_cpus_per_node*(nodebb+1)
- 1))"
+        fi
+        write "nodes:$nodea-$nodebb" 0 "cpumap:
$expected_range"
+    done
+fi
+
+echo "# A few attempts of pinning to a node but excluding one random
cpu" >> $outfile
+if [ $nr_nodes -gt 1 ]; then
+    for i in `seq 0 3`; do
+        node=$(($RANDOM % nr_nodes))
+        # this assumes that the first $nr_cpus_per_node (from cpu
+        # 0 to cpu $nr_cpus_per_node-1) are assigned to the first node
+        # (node 0), the second $nr_cpus_per_node (from $nr_cpus_per_node
+        # to 2*$nr_cpus_per_node-1) are assigned to the second node (node
+        # 1), etc. Expect failures if that is not the case.
+        cpu=$(($RANDOM % nr_cpus_per_node + nr_cpus_per_node*node))
+        if [ $cpu -eq $((nr_cpus_per_node*node)) ]; then
+            expected_range="$((nr_cpus_per_node*node +
1))-$((nr_cpus_per_node*(node+1) - 1))"
+        elif [ $cpu -eq $((nr_cpus_per_node*node + 1)) ]; then
+           
expected_range="$((nr_cpus_per_node*node)),$((nr_cpus_per_node*node +
2))-$((nr_cpus_per_node*(node+1) - 1))"
+        elif [ $cpu -eq $((nr_cpus_per_node*(node+1) - 2)) ]; then
+           
expected_range="$((nr_cpus_per_node*node))-$((nr_cpus_per_node*(node+1) -
3)),$((nr_cpus_per_node*(node+1) - 1))"
+        elif [ $cpu -eq $((nr_cpus_per_node*(node+1) - 1)) ]; then
+           
expected_range="$((nr_cpus_per_node*node))-$((nr_cpus_per_node*(node+1) -
2))"
+        else
+            expected_range="$((nr_cpus_per_node*node))-$((cpu - 1)),$((cpu
+ 1))-$((nr_cpus_per_node*(node+1) - 1))"
+        fi
+        write "nodes:$node,^$cpu" 0 "cpumap:
$expected_range"
+    done
+fi
+
+run $outfile
diff --git a/tools/libxl/check-xl-vcpupin-parse.data-example
b/tools/libxl/check-xl-vcpupin-parse.data-example
new file mode 100644
index 0000000..4bbd5de
--- /dev/null
+++ b/tools/libxl/check-xl-vcpupin-parse.data-example
@@ -0,0 +1,53 @@
+# WARNING: some of these tests are topology based tests.
+# Expect failures if the topology is not detected correctly
+# detected topology:  16 CPUs,  2 nodes, 8 CPUs per node.
+#
+# seed used for random number generation: seed=13328.
+#
+# Format is as follows:
+#  test-string*expected-return-code*expected-output
+#
+# Testing a wrong configuration
+foo*255*
+# Testing the ''all'' syntax
+all*0*cpumap: all
+nodes:all*0*cpumap: all
+all,nodes:all*0*cpumap: all
+all,^nodes:0,all*0*cpumap: all
+# Testing the empty cpumap case
+^0*0*cpumap: none
+# A few attempts of pinning to just one random cpu
+0*0*cpumap: 0
+9*0*cpumap: 9
+6*0*cpumap: 6
+0*0*cpumap: 0
+# A few attempts of pinning to all but one random cpu
+all,^12*0*cpumap: 0-11,13-15
+all,^6*0*cpumap: 0-5,7-15
+all,^3*0*cpumap: 0-2,4-15
+all,^7*0*cpumap: 0-6,8-15
+# A few attempts of pinning to a random range of cpus
+13-15*0*cpumap: 13-15
+7*0*cpumap: 7
+3-5*0*cpumap: 3-5
+8-11*0*cpumap: 8-11
+# A few attempts of pinning to just one random node
+nodes:1*0*cpumap: 8-15
+nodes:0*0*cpumap: 0-7
+nodes:0*0*cpumap: 0-7
+nodes:0*0*cpumap: 0-7
+# A few attempts of pinning to all but one random node
+all,^nodes:0*0*cpumap: 8-15
+all,^nodes:1*0*cpumap: 0-7
+all,^nodes:1*0*cpumap: 0-7
+all,^nodes:0*0*cpumap: 8-15
+# A few attempts of pinning to a random range of nodes
+nodes:1-1*0*cpumap: 8-15
+nodes:1-1*0*cpumap: 8-15
+nodes:0-1*0*cpumap: all
+nodes:0-0*0*cpumap: 0-7
+# A few attempts of pinning to a node but excluding one random cpu
+nodes:1,^8*0*cpumap: 9-15
+nodes:0,^6*0*cpumap: 0-5,7
+nodes:1,^9*0*cpumap: 8,10-15
+nodes:0,^5*0*cpumap: 0-4,6-7

Ian Campbell

2013-Dec-09 15:16 UTC

head link

Re: [RESEND PATCH v5 4/9] libxl: move libxl_{cpu, node}_bitmap_alloc()

On Sat, 2013-12-07 at 01:04 +0100, Dario Faggioli wrote:> in libxl_utils.c (from .h), as they will be reworked in
> the next commit ("libxc/libxl: sanitize error handling in
> *_get_max_{cpus,nodes}") and we want to keep code motion
> separate from functional changes.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian Campbell

2013-Dec-09 15:45 UTC

head link

Re: [RESEND PATCH v5 0/9] vcpu soft affinity for credit1 (first half only)

On Sat, 2013-12-07 at 01:04 +0100, Dario Faggioli wrote:> Therefore, as discussed and decided already during the actual soft affinity
> series review, these patches, and only these ones, are 4.4 material.
Acked #4 and applied after discussion with George.

Xen devel - Dec 2013 - [RESEND PATCH v5 0/9] vcpu soft affinity for credit1 (first half only)

[RESEND PATCH v5 0/9] vcpu soft affinity for credit1 (first half only)

[RESEND PATCH v5 1/9] xl: match output of vcpu-list with pinning syntax

[RESEND PATCH v5 2/9] libxl: better name for parameters in libxl_list_vcpu

[RESEND PATCH v5 3/9] libxl: fix memory leak in libxl_list_vcpu

[RESEND PATCH v5 4/9] libxl: move libxl_{cpu, node}_bitmap_alloc()

[RESEND PATCH v5 5/9] libxc/libxl: sanitize error handling in *_get_max_{cpus, nodes}

[RESEND PATCH v5 6/9] libxc/libxl: allow to retrieve the number of online pCPUs

[RESEND PATCH v5 7/9] xl: allow for node-wise specification of vcpu pinning

[RESEND PATCH v5 8/9] xl: implement and enable dryrun mode for `xl vcpu-pin''

[RESEND PATCH v5 9/9] xl: test script for the cpumap parser (for vCPU pinning)

Re: [RESEND PATCH v5 4/9] libxl: move libxl_{cpu, node}_bitmap_alloc()

Re: [RESEND PATCH v5 0/9] vcpu soft affinity for credit1 (first half only)