All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0 of 3] libxl: Extend CPU affinity specification and enable it in config file.
@ 2012-01-11 17:49 Dario Faggioli
  2012-01-11 17:58 ` [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin Dario Faggioli
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Dario Faggioli @ 2012-01-11 17:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian.Campbell, Ian Jackson


[-- Attachment #1.1: Type: text/plain, Size: 1604 bytes --]

Hello Everyone,

This series slightly extends the current support for specifying CPU
affinity, basically adding the support for "^<cpuid>" kind of entries
(i.e., "^6", meaning "not on CPU#6), and enables doing so in a VM's
config file, like it (probably?) was possible with `xm'.

Should you have any comments or questions, feel free to throw them. :-)

Thanks and Regards,
Dario Faggioli

--
A generalize-vcpupin-parsig.patch
A support-cpus-par-in-config-file.patch
A adjust-examples.patch
-- 
tools/examples/xmexample.hvm         |    2 --
 tools/examples/xmexample.hvm-stubdom |    2 --
 tools/examples/xmexample.pv-grub     |    2 --
 tools/examples/xmexample.vti         |    2 --
 tools/examples/xmexample1            |    2 --
 tools/examples/xmexample2            |    3 ---
 tools/examples/xmexample3            |    3 ---
 tools/libxl/libxl_create.c           |    2 ++
 tools/libxl/libxl_dom.c              |    8 +++++++-
 tools/libxl/libxl_types.idl          |    1 +
 tools/libxl/libxl_utils.h            |    2 ++
 tools/libxl/xl_cmdimpl.c             |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
 12 files changed, 92 insertions(+), 46 deletions(-)

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-11 17:49 [PATCH 0 of 3] libxl: Extend CPU affinity specification and enable it in config file Dario Faggioli
@ 2012-01-11 17:58 ` Dario Faggioli
  2012-01-12  8:38   ` Ian Campbell
  2012-01-12 17:34   ` Ian Jackson
  2012-01-11 18:00 ` [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file Dario Faggioli
  2012-01-11 18:01 ` [PATCH 3 of 3] libxl: Align examples with current code Dario Faggioli
  2 siblings, 2 replies; 19+ messages in thread
From: Dario Faggioli @ 2012-01-11 17:58 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian.Campbell, Ian Jackson


[-- Attachment #1.1.1: Type: text/plain, Size: 5022 bytes --]

Allow for "^<cpuid>" syntax while specifying the pCPUs list
during a vcpu-pin. This enables doing the following:

 xl vcpu-pin 1 1 0-4,^2

and achieving:

 xl vcpu-list
 Name                                ID  VCPU   CPU State   Time(s) CPU Affinity
 ...
 Squeeze_pv                           1     1    3   -b-       2.4  0-1,3-4
 ...

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 9cdcedc133e5 tools/libxl/libxl_utils.h
--- a/tools/libxl/libxl_utils.h	Wed Jan 11 10:34:45 2012 +0100
+++ b/tools/libxl/libxl_utils.h	Wed Jan 11 17:38:04 2012 +0000
@@ -71,6 +71,8 @@ int libxl_cpumap_test(libxl_cpumap *cpum
 void libxl_cpumap_set(libxl_cpumap *cpumap, int cpu);
 void libxl_cpumap_reset(libxl_cpumap *cpumap, int cpu);
 #define libxl_for_each_cpu(var, map) for (var = 0; var < (map).size * 8; var++)
+#define libxl_for_each_set_cpu(var, map) for (var = 0; var < (map).size * 8; var++) \
+                                             if (libxl_cpumap_test(&(map), var))
 
 int libxl_cpuarray_alloc(libxl_ctx *ctx, libxl_cpuarray *cpuarray);
 
diff -r 9cdcedc133e5 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Jan 11 10:34:45 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:38:04 2012 +0000
@@ -3501,13 +3501,67 @@ int main_vcpulist(int argc, char **argv)
     return 0;
 }
 
+static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap)
+{
+    libxl_cpumap exclude_cpumap;
+    uint32_t cpuida, cpuidb;
+    char *endptr, *toka, *tokb;
+    int i, rmcpu, ret = 0;
+
+    if (libxl_cpumap_alloc(ctx, &exclude_cpumap))
+        return ENOMEM;
+
+    if (strcmp(cpu, "all")) {
+        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
+            rmcpu = 0;
+            if (*toka == '^') {
+                toka++; rmcpu = 1;
+            }
+            cpuida = strtoul(toka, &endptr, 10);
+            if (toka == endptr) {
+                fprintf(stderr, "Error: Invalid argument.\n");
+                ret = EINVAL;
+                goto vcpp_out;
+            }
+            if (rmcpu) {
+                libxl_cpumap_set(&exclude_cpumap, cpuida);
+            } else if (*endptr == '-') {
+                tokb = endptr + 1;
+                cpuidb = strtoul(tokb, &endptr, 10);
+                if ((tokb == endptr) || (cpuida > cpuidb)) {
+                    fprintf(stderr, "Error: Invalid argument.\n");
+                    ret = EINVAL;
+                    goto vcpp_out;
+                }
+                while (cpuida <= cpuidb) {
+                    libxl_cpumap_set(cpumap, cpuida);
+                    ++cpuida;
+                }
+            } else {
+                libxl_cpumap_set(cpumap, cpuida);
+            }
+        }
+
+        libxl_for_each_set_cpu(i, exclude_cpumap) {
+            libxl_cpumap_reset(cpumap, i);
+        }
+    } else {
+        memset(cpumap->map, -1, cpumap->size);
+    }
+
+vcpp_out:
+    libxl_cpumap_dispose(&exclude_cpumap);
+
+    return ret;
+}
+
 static void vcpupin(const char *d, const char *vcpu, char *cpu)
 {
     libxl_vcpuinfo *vcpuinfo;
     libxl_cpumap cpumap;
 
-    uint32_t vcpuid, cpuida, cpuidb;
-    char *endptr, *toka, *tokb;
+    uint32_t vcpuid;
+    char *endptr;
     int i, nb_vcpu;
 
     vcpuid = strtoul(vcpu, &endptr, 10);
@@ -3524,32 +3578,9 @@ static void vcpupin(const char *d, const
     if (libxl_cpumap_alloc(ctx, &cpumap)) {
         goto vcpupin_out;
     }
-    if (strcmp(cpu, "all")) {
-        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
-            cpuida = strtoul(toka, &endptr, 10);
-            if (toka == endptr) {
-                fprintf(stderr, "Error: Invalid argument.\n");
-                goto vcpupin_out1;
-            }
-            if (*endptr == '-') {
-                tokb = endptr + 1;
-                cpuidb = strtoul(tokb, &endptr, 10);
-                if ((tokb == endptr) || (cpuida > cpuidb)) {
-                    fprintf(stderr, "Error: Invalid argument.\n");
-                    goto vcpupin_out1;
-                }
-                while (cpuida <= cpuidb) {
-                    libxl_cpumap_set(&cpumap, cpuida);
-                    ++cpuida;
-                }
-            } else {
-                libxl_cpumap_set(&cpumap, cpuida);
-            }
-        }
-    }
-    else {
-        memset(cpumap.map, -1, cpumap.size);
-    }
+
+    if (vcpupin_parse(cpu, &cpumap))
+        goto vcpupin_out1;
 
     if (vcpuid != -1) {
         if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap) == -1) {

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.1.2: generalize-vcpupin-parsig.patch --]
[-- Type: text/x-patch, Size: 4809 bytes --]

# HG changeset patch
# Parent 9cdcedc133e5227c635dbb00bd4779015311107a
libxl: extend pCPUs specification for vcpu-pin.

Allow for "^<cpuid>" syntax while specifying the pCPUs list
during a vcpu-pin. This enables doing the following:

 xl vcpu-pin 1 1 0-4,^2

and achieving:

 xl vcpu-list
 Name                                ID  VCPU   CPU State   Time(s) CPU Affinity
 ...
 Squeeze_pv                           1     1    3   -b-       2.4  0-1,3-4
 ...

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 9cdcedc133e5 tools/libxl/libxl_utils.h
--- a/tools/libxl/libxl_utils.h	Wed Jan 11 10:34:45 2012 +0100
+++ b/tools/libxl/libxl_utils.h	Wed Jan 11 17:38:04 2012 +0000
@@ -71,6 +71,8 @@ int libxl_cpumap_test(libxl_cpumap *cpum
 void libxl_cpumap_set(libxl_cpumap *cpumap, int cpu);
 void libxl_cpumap_reset(libxl_cpumap *cpumap, int cpu);
 #define libxl_for_each_cpu(var, map) for (var = 0; var < (map).size * 8; var++)
+#define libxl_for_each_set_cpu(var, map) for (var = 0; var < (map).size * 8; var++) \
+                                             if (libxl_cpumap_test(&(map), var))
 
 int libxl_cpuarray_alloc(libxl_ctx *ctx, libxl_cpuarray *cpuarray);
 
diff -r 9cdcedc133e5 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Jan 11 10:34:45 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:38:04 2012 +0000
@@ -3501,13 +3501,67 @@ int main_vcpulist(int argc, char **argv)
     return 0;
 }
 
+static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap)
+{
+    libxl_cpumap exclude_cpumap;
+    uint32_t cpuida, cpuidb;
+    char *endptr, *toka, *tokb;
+    int i, rmcpu, ret = 0;
+
+    if (libxl_cpumap_alloc(ctx, &exclude_cpumap))
+        return ENOMEM;
+
+    if (strcmp(cpu, "all")) {
+        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
+            rmcpu = 0;
+            if (*toka == '^') {
+                toka++; rmcpu = 1;
+            }
+            cpuida = strtoul(toka, &endptr, 10);
+            if (toka == endptr) {
+                fprintf(stderr, "Error: Invalid argument.\n");
+                ret = EINVAL;
+                goto vcpp_out;
+            }
+            if (rmcpu) {
+                libxl_cpumap_set(&exclude_cpumap, cpuida);
+            } else if (*endptr == '-') {
+                tokb = endptr + 1;
+                cpuidb = strtoul(tokb, &endptr, 10);
+                if ((tokb == endptr) || (cpuida > cpuidb)) {
+                    fprintf(stderr, "Error: Invalid argument.\n");
+                    ret = EINVAL;
+                    goto vcpp_out;
+                }
+                while (cpuida <= cpuidb) {
+                    libxl_cpumap_set(cpumap, cpuida);
+                    ++cpuida;
+                }
+            } else {
+                libxl_cpumap_set(cpumap, cpuida);
+            }
+        }
+
+        libxl_for_each_set_cpu(i, exclude_cpumap) {
+            libxl_cpumap_reset(cpumap, i);
+        }
+    } else {
+        memset(cpumap->map, -1, cpumap->size);
+    }
+
+vcpp_out:
+    libxl_cpumap_dispose(&exclude_cpumap);
+
+    return ret;
+}
+
 static void vcpupin(const char *d, const char *vcpu, char *cpu)
 {
     libxl_vcpuinfo *vcpuinfo;
     libxl_cpumap cpumap;
 
-    uint32_t vcpuid, cpuida, cpuidb;
-    char *endptr, *toka, *tokb;
+    uint32_t vcpuid;
+    char *endptr;
     int i, nb_vcpu;
 
     vcpuid = strtoul(vcpu, &endptr, 10);
@@ -3524,32 +3578,9 @@ static void vcpupin(const char *d, const
     if (libxl_cpumap_alloc(ctx, &cpumap)) {
         goto vcpupin_out;
     }
-    if (strcmp(cpu, "all")) {
-        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
-            cpuida = strtoul(toka, &endptr, 10);
-            if (toka == endptr) {
-                fprintf(stderr, "Error: Invalid argument.\n");
-                goto vcpupin_out1;
-            }
-            if (*endptr == '-') {
-                tokb = endptr + 1;
-                cpuidb = strtoul(tokb, &endptr, 10);
-                if ((tokb == endptr) || (cpuida > cpuidb)) {
-                    fprintf(stderr, "Error: Invalid argument.\n");
-                    goto vcpupin_out1;
-                }
-                while (cpuida <= cpuidb) {
-                    libxl_cpumap_set(&cpumap, cpuida);
-                    ++cpuida;
-                }
-            } else {
-                libxl_cpumap_set(&cpumap, cpuida);
-            }
-        }
-    }
-    else {
-        memset(cpumap.map, -1, cpumap.size);
-    }
+
+    if (vcpupin_parse(cpu, &cpumap))
+        goto vcpupin_out1;
 
     if (vcpuid != -1) {
         if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap) == -1) {

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file.
  2012-01-11 17:49 [PATCH 0 of 3] libxl: Extend CPU affinity specification and enable it in config file Dario Faggioli
  2012-01-11 17:58 ` [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin Dario Faggioli
@ 2012-01-11 18:00 ` Dario Faggioli
  2012-01-12  8:43   ` Ian Campbell
  2012-01-11 18:01 ` [PATCH 3 of 3] libxl: Align examples with current code Dario Faggioli
  2 siblings, 1 reply; 19+ messages in thread
From: Dario Faggioli @ 2012-01-11 18:00 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian.Campbell, Ian Jackson


[-- Attachment #1.1.1: Type: text/plain, Size: 4939 bytes --]

Enable CPU affinity specification in a VM's config file with the
exact syntax `xl vcpu-pin' provides.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 9ce68a4036b1 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/libxl_create.c	Wed Jan 11 17:40:45 2012 +0000
@@ -78,6 +78,8 @@ int libxl_init_build_info(libxl_ctx *ctx
     memset(b_info, '\0', sizeof(*b_info));
     b_info->max_vcpus = 1;
     b_info->cur_vcpus = 1;
+    if (libxl_cpumap_alloc(ctx, &b_info->cpumap))
+        return ERROR_NOMEM;
     b_info->max_memkb = 32 * 1024;
     b_info->target_memkb = b_info->max_memkb;
     b_info->disable_migrate = 0;
diff -r 9ce68a4036b1 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/libxl_dom.c	Wed Jan 11 17:40:45 2012 +0000
@@ -72,8 +72,14 @@ int libxl__build_pre(libxl__gc *gc, uint
               libxl_domain_build_info *info, libxl__domain_build_state *state)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    int tsc_mode;
+    int i, tsc_mode;
     xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
+    for (i = 0; i < info->max_vcpus; i++) {
+        if (libxl_set_vcpuaffinity(ctx, domid, i, &info->cpumap) == -1) {
+                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "libxl_set_vcpuaffinity failed. "
+                           "Going ahead without setting affinity for cpu %d.\n", i);
+        }
+    }
     xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
     if (info->type == LIBXL_DOMAIN_TYPE_PV)
         xc_domain_set_memmap_limit(ctx->xch, domid,
diff -r 9ce68a4036b1 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Wed Jan 11 17:40:45 2012 +0000
@@ -162,6 +162,7 @@ libxl_domain_create_info = Struct("domai
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
     ("cur_vcpus",       integer),
+    ("cpumap",          libxl_cpumap),
     ("tsc_mode",        libxl_tsc_mode),
     ("max_memkb",       uint32),
     ("target_memkb",    uint32),
diff -r 9ce68a4036b1 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:40:45 2012 +0000
@@ -288,16 +288,24 @@ static void dolog(const char *file, int 
         libxl_write_exactly(NULL, logfile, s, rc, NULL, NULL);
 }
 
+static void print_bitmap(uint8_t *map, int maplen, FILE *stream);
+
 static void printf_info(int domid,
                         libxl_domain_config *d_config,
                         libxl_device_model_info *dm_info)
 {
-    int i;
+    int i, nr_cpus = -1;
     libxl_dominfo info;
+    libxl_physinfo physinfo;
 
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
 
+    if (libxl_get_physinfo(ctx, &physinfo) == 0)
+        nr_cpus = physinfo.nr_cpus;
+    else
+        fprintf(stderr, "libxl_physinfo failed.\n");
+
     printf("(domain\n\t(domid %d)\n", domid);
     printf("\t(create_info)\n");
     printf("\t(hvm %d)\n", c_info->type == LIBXL_DOMAIN_TYPE_HVM);
@@ -328,6 +336,9 @@ static void printf_info(int domid,
 
     printf("\t(build_info)\n");
     printf("\t(max_vcpus %d)\n", b_info->max_vcpus);
+    printf("\t(CPU affinity ");
+    print_bitmap(b_info->cpumap.map, nr_cpus, stdout);
+    printf(")\n");
     printf("\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
     printf("\t(max_memkb %d)\n", b_info->max_memkb);
     printf("\t(target_memkb %d)\n", b_info->target_memkb);
@@ -569,6 +580,8 @@ static void split_string_into_string_lis
     free(s);
 }
 
+static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap);
+
 static void parse_config_data(const char *configfile_filename_report,
                               const char *configfile_data,
                               int configfile_len,
@@ -661,6 +674,13 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
         b_info->max_vcpus = l;
 
+    if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
+        char *buf2 = strdup(buf);
+        vcpupin_parse(buf2, &b_info->cpumap);
+    } else {
+        memset(b_info->cpumap.map, -1, b_info->cpumap.size);
+    }
+
     if (!xlu_cfg_get_long (config, "memory", &l, 0)) {
         b_info->max_memkb = l * 1024;
         b_info->target_memkb = b_info->max_memkb;

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.1.2: support-cpus-par-in-config-file.patch --]
[-- Type: text/x-patch, Size: 4743 bytes --]

# HG changeset patch
# Parent 9ce68a4036b1a01b17ecf5db0bde5c114655ec0b
libxl: allow for specifying the CPU affinity in the config file.

Enable CPU affinity specification in a VM's config file with the
exact syntax `xl vcpu-pin' provides.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 9ce68a4036b1 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/libxl_create.c	Wed Jan 11 17:40:45 2012 +0000
@@ -78,6 +78,8 @@ int libxl_init_build_info(libxl_ctx *ctx
     memset(b_info, '\0', sizeof(*b_info));
     b_info->max_vcpus = 1;
     b_info->cur_vcpus = 1;
+    if (libxl_cpumap_alloc(ctx, &b_info->cpumap))
+        return ERROR_NOMEM;
     b_info->max_memkb = 32 * 1024;
     b_info->target_memkb = b_info->max_memkb;
     b_info->disable_migrate = 0;
diff -r 9ce68a4036b1 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/libxl_dom.c	Wed Jan 11 17:40:45 2012 +0000
@@ -72,8 +72,14 @@ int libxl__build_pre(libxl__gc *gc, uint
               libxl_domain_build_info *info, libxl__domain_build_state *state)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    int tsc_mode;
+    int i, tsc_mode;
     xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
+    for (i = 0; i < info->max_vcpus; i++) {
+        if (libxl_set_vcpuaffinity(ctx, domid, i, &info->cpumap) == -1) {
+                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "libxl_set_vcpuaffinity failed. "
+                           "Going ahead without setting affinity for cpu %d.\n", i);
+        }
+    }
     xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
     if (info->type == LIBXL_DOMAIN_TYPE_PV)
         xc_domain_set_memmap_limit(ctx->xch, domid,
diff -r 9ce68a4036b1 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Wed Jan 11 17:40:45 2012 +0000
@@ -162,6 +162,7 @@ libxl_domain_create_info = Struct("domai
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
     ("cur_vcpus",       integer),
+    ("cpumap",          libxl_cpumap),
     ("tsc_mode",        libxl_tsc_mode),
     ("max_memkb",       uint32),
     ("target_memkb",    uint32),
diff -r 9ce68a4036b1 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:38:04 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:40:45 2012 +0000
@@ -288,16 +288,24 @@ static void dolog(const char *file, int 
         libxl_write_exactly(NULL, logfile, s, rc, NULL, NULL);
 }
 
+static void print_bitmap(uint8_t *map, int maplen, FILE *stream);
+
 static void printf_info(int domid,
                         libxl_domain_config *d_config,
                         libxl_device_model_info *dm_info)
 {
-    int i;
+    int i, nr_cpus = -1;
     libxl_dominfo info;
+    libxl_physinfo physinfo;
 
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
 
+    if (libxl_get_physinfo(ctx, &physinfo) == 0)
+        nr_cpus = physinfo.nr_cpus;
+    else
+        fprintf(stderr, "libxl_physinfo failed.\n");
+
     printf("(domain\n\t(domid %d)\n", domid);
     printf("\t(create_info)\n");
     printf("\t(hvm %d)\n", c_info->type == LIBXL_DOMAIN_TYPE_HVM);
@@ -328,6 +336,9 @@ static void printf_info(int domid,
 
     printf("\t(build_info)\n");
     printf("\t(max_vcpus %d)\n", b_info->max_vcpus);
+    printf("\t(CPU affinity ");
+    print_bitmap(b_info->cpumap.map, nr_cpus, stdout);
+    printf(")\n");
     printf("\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
     printf("\t(max_memkb %d)\n", b_info->max_memkb);
     printf("\t(target_memkb %d)\n", b_info->target_memkb);
@@ -569,6 +580,8 @@ static void split_string_into_string_lis
     free(s);
 }
 
+static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap);
+
 static void parse_config_data(const char *configfile_filename_report,
                               const char *configfile_data,
                               int configfile_len,
@@ -661,6 +674,13 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
         b_info->max_vcpus = l;
 
+    if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
+        char *buf2 = strdup(buf);
+        vcpupin_parse(buf2, &b_info->cpumap);
+    } else {
+        memset(b_info->cpumap.map, -1, b_info->cpumap.size);
+    }
+
     if (!xlu_cfg_get_long (config, "memory", &l, 0)) {
         b_info->max_memkb = l * 1024;
         b_info->target_memkb = b_info->max_memkb;

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3 of 3] libxl: Align examples with current code.
  2012-01-11 17:49 [PATCH 0 of 3] libxl: Extend CPU affinity specification and enable it in config file Dario Faggioli
  2012-01-11 17:58 ` [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin Dario Faggioli
  2012-01-11 18:00 ` [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file Dario Faggioli
@ 2012-01-11 18:01 ` Dario Faggioli
  2012-01-12  8:37   ` Ian Campbell
  2 siblings, 1 reply; 19+ messages in thread
From: Dario Faggioli @ 2012-01-11 18:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian.Campbell, Ian Jackson


[-- Attachment #1.1.1: Type: text/plain, Size: 5066 bytes --]

Just make sure the syntax proposed in the various examples is
the right one.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 897ea4d3c2d7 tools/examples/xmexample.hvm
--- a/tools/examples/xmexample.hvm	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.hvm	Wed Jan 11 17:32:34 2012 +0000
@@ -59,10 +59,8 @@ name = "ExampleHVMDomain"
 # xen_extended_power_mgmt=0
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Optionally define mac and/or bridge for the network interfaces.
 # Random MACs are assigned if not given.
diff -r 897ea4d3c2d7 tools/examples/xmexample.hvm-stubdom
--- a/tools/examples/xmexample.hvm-stubdom	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.hvm-stubdom	Wed Jan 11 17:32:34 2012 +0000
@@ -50,10 +50,8 @@ name = "xmexample.hvm"
 #apic=1
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Optionally define mac and/or bridge for the network interfaces.
 # Random MACs are assigned if not given.
diff -r 897ea4d3c2d7 tools/examples/xmexample.pv-grub
--- a/tools/examples/xmexample.pv-grub	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.pv-grub	Wed Jan 11 17:32:34 2012 +0000
@@ -35,10 +35,8 @@ name = "ExampleDomain"
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Number of Virtual CPUS to use, default is 1
 #vcpus = 1
diff -r 897ea4d3c2d7 tools/examples/xmexample.vti
--- a/tools/examples/xmexample.vti	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.vti	Wed Jan 11 17:32:34 2012 +0000
@@ -31,10 +31,8 @@ name = "ExampleVTIDomain"
 #vcpus=1
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Log2 of VHPT size, default=23 (8MB), minimum=15 (32KB).
 # In Windows OS, smaller size shows better performance.
diff -r 897ea4d3c2d7 tools/examples/xmexample1
--- a/tools/examples/xmexample1	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample1	Wed Jan 11 17:32:34 2012 +0000
@@ -31,10 +31,8 @@ name = "ExampleDomain"
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Number of Virtual CPUS to use, default is 1
 #vcpus = 1
diff -r 897ea4d3c2d7 tools/examples/xmexample2
--- a/tools/examples/xmexample2	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample2	Wed Jan 11 17:32:34 2012 +0000
@@ -60,11 +60,8 @@ name = "VM%d" % vmid
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
-#cpus = "%s" % vmid # set based on vmid (mod number of CPUs)
 
 # Number of Virtual CPUS to use, default is 1
 #vcpus = 1
diff -r 897ea4d3c2d7 tools/examples/xmexample3
--- a/tools/examples/xmexample3	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample3	Wed Jan 11 17:32:34 2012 +0000
@@ -60,11 +60,8 @@ name = "VM%d" % vmid
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
-cpus = "%s" % vmid # set based on vmid (mod number of CPUs)
 
 #----------------------------------------------------------------------------
 # Define network interfaces.

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.1.2: adjust-examples.patch --]
[-- Type: text/x-patch, Size: 4846 bytes --]

# HG changeset patch
# Parent 897ea4d3c2d7617442e4b97a72370464cf7fc93f
libxl: Align examples with current code.

Just make sure the syntax proposed in the various examples is
the right one.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff -r 897ea4d3c2d7 tools/examples/xmexample.hvm
--- a/tools/examples/xmexample.hvm	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.hvm	Wed Jan 11 17:32:34 2012 +0000
@@ -59,10 +59,8 @@ name = "ExampleHVMDomain"
 # xen_extended_power_mgmt=0
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Optionally define mac and/or bridge for the network interfaces.
 # Random MACs are assigned if not given.
diff -r 897ea4d3c2d7 tools/examples/xmexample.hvm-stubdom
--- a/tools/examples/xmexample.hvm-stubdom	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.hvm-stubdom	Wed Jan 11 17:32:34 2012 +0000
@@ -50,10 +50,8 @@ name = "xmexample.hvm"
 #apic=1
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Optionally define mac and/or bridge for the network interfaces.
 # Random MACs are assigned if not given.
diff -r 897ea4d3c2d7 tools/examples/xmexample.pv-grub
--- a/tools/examples/xmexample.pv-grub	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.pv-grub	Wed Jan 11 17:32:34 2012 +0000
@@ -35,10 +35,8 @@ name = "ExampleDomain"
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Number of Virtual CPUS to use, default is 1
 #vcpus = 1
diff -r 897ea4d3c2d7 tools/examples/xmexample.vti
--- a/tools/examples/xmexample.vti	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample.vti	Wed Jan 11 17:32:34 2012 +0000
@@ -31,10 +31,8 @@ name = "ExampleVTIDomain"
 #vcpus=1
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Log2 of VHPT size, default=23 (8MB), minimum=15 (32KB).
 # In Windows OS, smaller size shows better performance.
diff -r 897ea4d3c2d7 tools/examples/xmexample1
--- a/tools/examples/xmexample1	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample1	Wed Jan 11 17:32:34 2012 +0000
@@ -31,10 +31,8 @@ name = "ExampleDomain"
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
 
 # Number of Virtual CPUS to use, default is 1
 #vcpus = 1
diff -r 897ea4d3c2d7 tools/examples/xmexample2
--- a/tools/examples/xmexample2	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample2	Wed Jan 11 17:32:34 2012 +0000
@@ -60,11 +60,8 @@ name = "VM%d" % vmid
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
-#cpus = "%s" % vmid # set based on vmid (mod number of CPUs)
 
 # Number of Virtual CPUS to use, default is 1
 #vcpus = 1
diff -r 897ea4d3c2d7 tools/examples/xmexample3
--- a/tools/examples/xmexample3	Wed Jan 11 17:28:53 2012 +0000
+++ b/tools/examples/xmexample3	Wed Jan 11 17:32:34 2012 +0000
@@ -60,11 +60,8 @@ name = "VM%d" % vmid
 #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
 
 # List of which CPUS this domain is allowed to use, default Xen picks
-#cpus = ""         # leave to Xen to pick
 #cpus = "0"        # all vcpus run on CPU0
 #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
-#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
-cpus = "%s" % vmid # set based on vmid (mod number of CPUs)
 
 #----------------------------------------------------------------------------
 # Define network interfaces.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3 of 3] libxl: Align examples with current code.
  2012-01-11 18:01 ` [PATCH 3 of 3] libxl: Align examples with current code Dario Faggioli
@ 2012-01-12  8:37   ` Ian Campbell
  2012-01-12 22:45     ` Dario Faggioli
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2012-01-12  8:37 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Ian Jackson

On Wed, 2012-01-11 at 18:01 +0000, Dario Faggioli wrote:
> Just make sure the syntax proposed in the various examples is
> the right one.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> diff -r 897ea4d3c2d7 tools/examples/xmexample.hvm
> --- a/tools/examples/xmexample.hvm	Wed Jan 11 17:28:53 2012 +0000
> +++ b/tools/examples/xmexample.hvm	Wed Jan 11 17:32:34 2012 +0000

These are the xend/xm examples which you didn't change. Or is it that
the current examples don't actually reflect the current xend syntax
either?

The xl examples are xl* in the same directory. These are more minimal
examples so I don't think including affinity there is necessary. Instead
you need to patch docs/man/xl.cfg.pod.5 to reflect the new syntax (and
if necessary deprecate the old).

BTW, I think we are in general happy for docs updates to go in the same
patch as the corresponding code update.

Ian.

> @@ -59,10 +59,8 @@ name = "ExampleHVMDomain"
>  # xen_extended_power_mgmt=0
>  
>  # List of which CPUS this domain is allowed to use, default Xen picks
> -#cpus = ""         # leave to Xen to pick
>  #cpus = "0"        # all vcpus run on CPU0
>  #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
> -#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
>  
>  # Optionally define mac and/or bridge for the network interfaces.
>  # Random MACs are assigned if not given.
> diff -r 897ea4d3c2d7 tools/examples/xmexample.hvm-stubdom
> --- a/tools/examples/xmexample.hvm-stubdom	Wed Jan 11 17:28:53 2012 +0000
> +++ b/tools/examples/xmexample.hvm-stubdom	Wed Jan 11 17:32:34 2012 +0000
> @@ -50,10 +50,8 @@ name = "xmexample.hvm"
>  #apic=1
>  
>  # List of which CPUS this domain is allowed to use, default Xen picks
> -#cpus = ""         # leave to Xen to pick
>  #cpus = "0"        # all vcpus run on CPU0
>  #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
> -#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
>  
>  # Optionally define mac and/or bridge for the network interfaces.
>  # Random MACs are assigned if not given.
> diff -r 897ea4d3c2d7 tools/examples/xmexample.pv-grub
> --- a/tools/examples/xmexample.pv-grub	Wed Jan 11 17:28:53 2012 +0000
> +++ b/tools/examples/xmexample.pv-grub	Wed Jan 11 17:32:34 2012 +0000
> @@ -35,10 +35,8 @@ name = "ExampleDomain"
>  #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
>  
>  # List of which CPUS this domain is allowed to use, default Xen picks
> -#cpus = ""         # leave to Xen to pick
>  #cpus = "0"        # all vcpus run on CPU0
>  #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
> -#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
>  
>  # Number of Virtual CPUS to use, default is 1
>  #vcpus = 1
> diff -r 897ea4d3c2d7 tools/examples/xmexample.vti
> --- a/tools/examples/xmexample.vti	Wed Jan 11 17:28:53 2012 +0000
> +++ b/tools/examples/xmexample.vti	Wed Jan 11 17:32:34 2012 +0000
> @@ -31,10 +31,8 @@ name = "ExampleVTIDomain"
>  #vcpus=1
>  
>  # List of which CPUS this domain is allowed to use, default Xen picks
> -#cpus = ""         # leave to Xen to pick
>  #cpus = "0"        # all vcpus run on CPU0
>  #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
> -#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
>  
>  # Log2 of VHPT size, default=23 (8MB), minimum=15 (32KB).
>  # In Windows OS, smaller size shows better performance.
> diff -r 897ea4d3c2d7 tools/examples/xmexample1
> --- a/tools/examples/xmexample1	Wed Jan 11 17:28:53 2012 +0000
> +++ b/tools/examples/xmexample1	Wed Jan 11 17:32:34 2012 +0000
> @@ -31,10 +31,8 @@ name = "ExampleDomain"
>  #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
>  
>  # List of which CPUS this domain is allowed to use, default Xen picks
> -#cpus = ""         # leave to Xen to pick
>  #cpus = "0"        # all vcpus run on CPU0
>  #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
> -#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
>  
>  # Number of Virtual CPUS to use, default is 1
>  #vcpus = 1
> diff -r 897ea4d3c2d7 tools/examples/xmexample2
> --- a/tools/examples/xmexample2	Wed Jan 11 17:28:53 2012 +0000
> +++ b/tools/examples/xmexample2	Wed Jan 11 17:32:34 2012 +0000
> @@ -60,11 +60,8 @@ name = "VM%d" % vmid
>  #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
>  
>  # List of which CPUS this domain is allowed to use, default Xen picks
> -#cpus = ""         # leave to Xen to pick
>  #cpus = "0"        # all vcpus run on CPU0
>  #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
> -#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
> -#cpus = "%s" % vmid # set based on vmid (mod number of CPUs)
>  
>  # Number of Virtual CPUS to use, default is 1
>  #vcpus = 1
> diff -r 897ea4d3c2d7 tools/examples/xmexample3
> --- a/tools/examples/xmexample3	Wed Jan 11 17:28:53 2012 +0000
> +++ b/tools/examples/xmexample3	Wed Jan 11 17:32:34 2012 +0000
> @@ -60,11 +60,8 @@ name = "VM%d" % vmid
>  #uuid = "06ed00fe-1162-4fc4-b5d8-11993ee4a8b9"
>  
>  # List of which CPUS this domain is allowed to use, default Xen picks
> -#cpus = ""         # leave to Xen to pick
>  #cpus = "0"        # all vcpus run on CPU0
>  #cpus = "0-3,5,^1" # all vcpus run on cpus 0,2,3,5
> -#cpus = ["2", "3"] # VCPU0 runs on CPU2, VCPU1 runs on CPU3
> -cpus = "%s" % vmid # set based on vmid (mod number of CPUs)
>  
>  #----------------------------------------------------------------------------
>  # Define network interfaces.
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-11 17:58 ` [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin Dario Faggioli
@ 2012-01-12  8:38   ` Ian Campbell
  2012-01-12 17:34   ` Ian Jackson
  1 sibling, 0 replies; 19+ messages in thread
From: Ian Campbell @ 2012-01-12  8:38 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Ian Jackson

On Wed, 2012-01-11 at 17:58 +0000, Dario Faggioli wrote:
> Allow for "^<cpuid>" syntax while specifying the pCPUs list
> during a vcpu-pin. This enables doing the following:
> 
>  xl vcpu-pin 1 1 0-4,^2
> 
> and achieving:
> 
>  xl vcpu-list
>  Name                                ID  VCPU   CPU State   Time(s) CPU Affinity
>  ...
>  Squeeze_pv                           1     1    3   -b-       2.4  0-1,3-4
>  ...
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> 
> diff -r 9cdcedc133e5 tools/libxl/libxl_utils.h
> --- a/tools/libxl/libxl_utils.h	Wed Jan 11 10:34:45 2012 +0100
> +++ b/tools/libxl/libxl_utils.h	Wed Jan 11 17:38:04 2012 +0000
> @@ -71,6 +71,8 @@ int libxl_cpumap_test(libxl_cpumap *cpum
>  void libxl_cpumap_set(libxl_cpumap *cpumap, int cpu);
>  void libxl_cpumap_reset(libxl_cpumap *cpumap, int cpu);
>  #define libxl_for_each_cpu(var, map) for (var = 0; var < (map).size * 8; var++)
> +#define libxl_for_each_set_cpu(var, map) for (var = 0; var < (map).size * 8; var++) \
> +                                             if (libxl_cpumap_test(&(map), var))
>  
>  int libxl_cpuarray_alloc(libxl_ctx *ctx, libxl_cpuarray *cpuarray);
>  
> diff -r 9cdcedc133e5 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Wed Jan 11 10:34:45 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:38:04 2012 +0000
> @@ -3501,13 +3501,67 @@ int main_vcpulist(int argc, char **argv)
>      return 0;
>  }
>  
> +static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap)
> +{
> +    libxl_cpumap exclude_cpumap;
> +    uint32_t cpuida, cpuidb;
> +    char *endptr, *toka, *tokb;
> +    int i, rmcpu, ret = 0;
> +
> +    if (libxl_cpumap_alloc(ctx, &exclude_cpumap))
> +        return ENOMEM;
> +
> +    if (strcmp(cpu, "all")) {
> +        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
> +            rmcpu = 0;
> +            if (*toka == '^') {
> +                toka++; rmcpu = 1;
> +            }
> +            cpuida = strtoul(toka, &endptr, 10);
> +            if (toka == endptr) {
> +                fprintf(stderr, "Error: Invalid argument.\n");
> +                ret = EINVAL;
> +                goto vcpp_out;
> +            }
> +            if (rmcpu) {
> +                libxl_cpumap_set(&exclude_cpumap, cpuida);
> +            } else if (*endptr == '-') {
> +                tokb = endptr + 1;
> +                cpuidb = strtoul(tokb, &endptr, 10);
> +                if ((tokb == endptr) || (cpuida > cpuidb)) {
> +                    fprintf(stderr, "Error: Invalid argument.\n");
> +                    ret = EINVAL;
> +                    goto vcpp_out;
> +                }
> +                while (cpuida <= cpuidb) {
> +                    libxl_cpumap_set(cpumap, cpuida);
> +                    ++cpuida;
> +                }
> +            } else {
> +                libxl_cpumap_set(cpumap, cpuida);
> +            }
> +        }
> +
> +        libxl_for_each_set_cpu(i, exclude_cpumap) {
> +            libxl_cpumap_reset(cpumap, i);
> +        }
> +    } else {
> +        memset(cpumap->map, -1, cpumap->size);
> +    }
> +
> +vcpp_out:
> +    libxl_cpumap_dispose(&exclude_cpumap);
> +
> +    return ret;
> +}
> +
>  static void vcpupin(const char *d, const char *vcpu, char *cpu)
>  {
>      libxl_vcpuinfo *vcpuinfo;
>      libxl_cpumap cpumap;
>  
> -    uint32_t vcpuid, cpuida, cpuidb;
> -    char *endptr, *toka, *tokb;
> +    uint32_t vcpuid;
> +    char *endptr;
>      int i, nb_vcpu;
>  
>      vcpuid = strtoul(vcpu, &endptr, 10);
> @@ -3524,32 +3578,9 @@ static void vcpupin(const char *d, const
>      if (libxl_cpumap_alloc(ctx, &cpumap)) {
>          goto vcpupin_out;
>      }
> -    if (strcmp(cpu, "all")) {
> -        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
> -            cpuida = strtoul(toka, &endptr, 10);
> -            if (toka == endptr) {
> -                fprintf(stderr, "Error: Invalid argument.\n");
> -                goto vcpupin_out1;
> -            }
> -            if (*endptr == '-') {
> -                tokb = endptr + 1;
> -                cpuidb = strtoul(tokb, &endptr, 10);
> -                if ((tokb == endptr) || (cpuida > cpuidb)) {
> -                    fprintf(stderr, "Error: Invalid argument.\n");
> -                    goto vcpupin_out1;
> -                }
> -                while (cpuida <= cpuidb) {
> -                    libxl_cpumap_set(&cpumap, cpuida);
> -                    ++cpuida;
> -                }
> -            } else {
> -                libxl_cpumap_set(&cpumap, cpuida);
> -            }
> -        }
> -    }
> -    else {
> -        memset(cpumap.map, -1, cpumap.size);
> -    }
> +
> +    if (vcpupin_parse(cpu, &cpumap))
> +        goto vcpupin_out1;
>  
>      if (vcpuid != -1) {
>          if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap) == -1) {
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file.
  2012-01-11 18:00 ` [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file Dario Faggioli
@ 2012-01-12  8:43   ` Ian Campbell
  2012-01-12 22:56     ` Dario Faggioli
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2012-01-12  8:43 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Ian Jackson

On Wed, 2012-01-11 at 18:00 +0000, Dario Faggioli wrote:
> Enable CPU affinity specification in a VM's config file with the
> exact syntax `xl vcpu-pin' provides.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> diff -r 9ce68a4036b1 tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c	Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/libxl_create.c	Wed Jan 11 17:40:45 2012 +0000
> @@ -78,6 +78,8 @@ int libxl_init_build_info(libxl_ctx *ctx
>      memset(b_info, '\0', sizeof(*b_info));
>      b_info->max_vcpus = 1;
>      b_info->cur_vcpus = 1;
> +    if (libxl_cpumap_alloc(ctx, &b_info->cpumap))
> +        return ERROR_NOMEM;

Should probably set all here since that is the best default?

>      b_info->max_memkb = 32 * 1024;
>      b_info->target_memkb = b_info->max_memkb;
>      b_info->disable_migrate = 0;
> diff -r 9ce68a4036b1 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/libxl_dom.c	Wed Jan 11 17:40:45 2012 +0000
> @@ -72,8 +72,14 @@ int libxl__build_pre(libxl__gc *gc, uint
>                libxl_domain_build_info *info, libxl__domain_build_state *state)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
> -    int tsc_mode;
> +    int i, tsc_mode;
>      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
> +    for (i = 0; i < info->max_vcpus; i++) {
> +        if (libxl_set_vcpuaffinity(ctx, domid, i, &info->cpumap) == -1) {
> +                LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "libxl_set_vcpuaffinity failed. "
> +                           "Going ahead without setting affinity for cpu %d.\n", i);
> +        }
> +    }4b
>      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
>      if (info->type == LIBXL_DOMAIN_TYPE_PV)
>          xc_domain_set_memmap_limit(ctx->xch, domid,
> diff -r 9ce68a4036b1 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/libxl_types.idl	Wed Jan 11 17:40:45 2012 +0000
> @@ -162,6 +162,7 @@ libxl_domain_create_info = Struct("domai
>  libxl_domain_build_info = Struct("domain_build_info",[
>      ("max_vcpus",       integer),
>      ("cur_vcpus",       integer),
> +    ("cpumap",          libxl_cpumap),
>      ("tsc_mode",        libxl_tsc_mode),
>      ("max_memkb",       uint32),
>      ("target_memkb",    uint32),
> diff -r 9ce68a4036b1 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:38:04 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c	Wed Jan 11 17:40:45 2012 +0000
> @@ -288,16 +288,24 @@ static void dolog(const char *file, int 
>          libxl_write_exactly(NULL, logfile, s, rc, NULL, NULL);
>  }
>  
> +static void print_bitmap(uint8_t *map, int maplen, FILE *stream);

Just move the function up? If we are going to do this sort of forward
declaration there should be single block of them near the top somewhere.

> +
>  static void printf_info(int domid,
>                          libxl_domain_config *d_config,
>                          libxl_device_model_info *dm_info)
>  {
> -    int i;
> +    int i, nr_cpus = -1;
>      libxl_dominfo info;
> +    libxl_physinfo physinfo;
>  
>      libxl_domain_create_info *c_info = &d_config->c_info;
>      libxl_domain_build_info *b_info = &d_config->b_info;
>  
> +    if (libxl_get_physinfo(ctx, &physinfo) == 0)
> +        nr_cpus = physinfo.nr_cpus;
> +    else
> +        fprintf(stderr, "libxl_physinfo failed.\n");
> +
>      printf("(domain\n\t(domid %d)\n", domid);
>      printf("\t(create_info)\n");
>      printf("\t(hvm %d)\n", c_info->type == LIBXL_DOMAIN_TYPE_HVM);
> @@ -328,6 +336,9 @@ static void printf_info(int domid,
>  
>      printf("\t(build_info)\n");
>      printf("\t(max_vcpus %d)\n", b_info->max_vcpus);
> +    printf("\t(CPU affinity ");
> +    print_bitmap(b_info->cpumap.map, nr_cpus, stdout);
> +    printf(")\n");
>      printf("\t(tsc_mode %s)\n", libxl_tsc_mode_to_string(b_info->tsc_mode));
>      printf("\t(max_memkb %d)\n", b_info->max_memkb);
>      printf("\t(target_memkb %d)\n", b_info->target_memkb);
> @@ -569,6 +580,8 @@ static void split_string_into_string_lis
>      free(s);
>  }
>  
> +static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap);

Again, maybe just reorder the functions?

>  static void parse_config_data(const char *configfile_filename_report,
>                                const char *configfile_data,
>                                int configfile_len,
> @@ -661,6 +674,13 @@ static void parse_config_data(const char
>      if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
>          b_info->max_vcpus = l;
>  
> +    if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {

The syntax supported here is different to that in a cpupool cfg? (see
main_cpupoolcreate) Perhaps they should be similar?

> +        char *buf2 = strdup(buf);
> +        vcpupin_parse(buf2, &b_info->cpumap);
> +    } else {
> +        memset(b_info->cpumap.map, -1, b_info->cpumap.size);

This could do with a helper function in libxl_utils.h.

Ian.


> +    }
> +
>      if (!xlu_cfg_get_long (config, "memory", &l, 0)) {
>          b_info->max_memkb = l * 1024;
>          b_info->target_memkb = b_info->max_memkb;
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-11 17:58 ` [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin Dario Faggioli
  2012-01-12  8:38   ` Ian Campbell
@ 2012-01-12 17:34   ` Ian Jackson
  2012-01-12 23:12     ` Dario Faggioli
  1 sibling, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2012-01-12 17:34 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: Ian.Campbell, xen-devel

Dario Faggioli writes ("[Xen-devel] [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin."):
> Allow for "^<cpuid>" syntax while specifying the pCPUs list
> during a vcpu-pin. This enables doing the following:
> 
>  xl vcpu-pin 1 1 0-4,^2
...
> +    if (strcmp(cpu, "all")) {
> +        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {

OMG you used strtok.  strtok is not thread-safe.  strtok_r is but
still modifies its input string - hence your need to strdup.  If you
do want to do it this way IMO the strdup should be in vcpupin_parse
which should take a const char*.  Are you sure this is the right
approach to parsing this ?

<record type="broken">
Your patch 2 has a number of overly long lines.
</record>

Ian.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3 of 3] libxl: Align examples with current code.
  2012-01-12  8:37   ` Ian Campbell
@ 2012-01-12 22:45     ` Dario Faggioli
  0 siblings, 0 replies; 19+ messages in thread
From: Dario Faggioli @ 2012-01-12 22:45 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Ian Jackson


[-- Attachment #1.1: Type: text/plain, Size: 913 bytes --]

On Thu, 2012-01-12 at 08:37 +0000, Ian Campbell wrote: 
> > diff -r 897ea4d3c2d7 tools/examples/xmexample.hvm
> > --- a/tools/examples/xmexample.hvm	Wed Jan 11 17:28:53 2012 +0000
> > +++ b/tools/examples/xmexample.hvm	Wed Jan 11 17:32:34 2012 +0000
> 
> These are the xend/xm examples which you didn't change. Or is it that
> the current examples don't actually reflect the current xend syntax
> either?
> 
Mmm... I see. No, you're right, they are different things and these
files shouldn't be touched. For next round, I'll just drop this patch.

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file.
  2012-01-12  8:43   ` Ian Campbell
@ 2012-01-12 22:56     ` Dario Faggioli
  2012-01-13  8:09       ` Ian Campbell
  0 siblings, 1 reply; 19+ messages in thread
From: Dario Faggioli @ 2012-01-12 22:56 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Ian Jackson


[-- Attachment #1.1: Type: text/plain, Size: 2729 bytes --]

On Thu, 2012-01-12 at 08:43 +0000, Ian Campbell wrote: 
> > diff -r 9ce68a4036b1 tools/libxl/libxl_create.c
> > --- a/tools/libxl/libxl_create.c	Wed Jan 11 17:38:04 2012 +0000
> > +++ b/tools/libxl/libxl_create.c	Wed Jan 11 17:40:45 2012 +0000
> > @@ -78,6 +78,8 @@ int libxl_init_build_info(libxl_ctx *ctx
> >      memset(b_info, '\0', sizeof(*b_info));
> >      b_info->max_vcpus = 1;
> >      b_info->cur_vcpus = 1;
> > +    if (libxl_cpumap_alloc(ctx, &b_info->cpumap))
> > +        return ERROR_NOMEM;
> 
> Should probably set all here since that is the best default?
> 
Having this set to "none" here simplifies a bit the code when we came to
config file parsing (if I set it to "any CPU" here, I'll have to reset
to "none" before parsing). Also, default is exactly what you're
suggesting already, as I set the map to "any CPU" if no "cpus" config
option is found.

Anyway, I guess I can do as you suggest if you really think it's
better. :-)

> > +static void print_bitmap(uint8_t *map, int maplen, FILE *stream);
>
> [..] 
>   
> > +static int vcpupin_parse(char *cpu, libxl_cpumap *cpumap);
> 
> Again, maybe just reorder the functions?
> 
Ok, fine, will do that.

> >  static void parse_config_data(const char *configfile_filename_report,
> >                                const char *configfile_data,
> >                                int configfile_len,
> > @@ -661,6 +674,13 @@ static void parse_config_data(const char
> >      if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
> >          b_info->max_vcpus = l;
> >  
> > +    if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
> 
> The syntax supported here is different to that in a cpupool cfg? (see
> main_cpupoolcreate) Perhaps they should be similar?
> 
It is different, and that was intentional. What I wanted, was the syntax
to be exactly the same of `xl vcpu-pin', which is the command line
equivalent of this option, much more than cpupool-*. If you want, I
think I can easily enable list-like CPU specification as in cpupool
config file so that _both_ syntax are allowed. What do you think?


> > +        char *buf2 = strdup(buf);
> > +        vcpupin_parse(buf2, &b_info->cpumap);
> > +    } else {
> > +        memset(b_info->cpumap.map, -1, b_info->cpumap.size);
> 
> This could do with a helper function in libxl_utils.h.
> 
Ok.

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-12 17:34   ` Ian Jackson
@ 2012-01-12 23:12     ` Dario Faggioli
  2012-01-13 13:34       ` Ian Jackson
  0 siblings, 1 reply; 19+ messages in thread
From: Dario Faggioli @ 2012-01-12 23:12 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Ian.Campbell, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 2239 bytes --]

On Thu, 2012-01-12 at 17:34 +0000, Ian Jackson wrote: 
> Dario Faggioli writes ("[Xen-devel] [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin."):
> > Allow for "^<cpuid>" syntax while specifying the pCPUs list
> > during a vcpu-pin. This enables doing the following:
> > 
> >  xl vcpu-pin 1 1 0-4,^2
> ...
> > +    if (strcmp(cpu, "all")) {
> > +        for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
> 
> OMG you used strtok.  strtok is not thread-safe.  
>
Well, that's not properly me, is it that?

hg annotate tools/libxl/xl_cmdimpl.c
...
21247:     if (strcmp(cpu, "all")) { 
21247:         for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
21247:             cpuida = strtoul(toka, &endptr, 10);
21247:             if (toka == endptr) {
...

> strtok_r is but
> still modifies its input string - hence your need to strdup.  If you
> do want to do it this way IMO the strdup should be in vcpupin_parse
> which should take a const char*.  Are you sure this is the right
> approach to parsing this ?
> 
Again, I know that's probably not an alibi, but that's not my code! :-P
What I did is just move this from the body of vcpupin() to an helper
function, and try to add as _less_ thing as I can to support a slightly
improved syntax.

That being said, if you think I should rewrite the parsing from scratch,
I can think about it, just wanted to clarify my position. :-)

> <record type="broken">
> Your patch 2 has a number of overly long lines.
> </record>
> 
I know. What I usually do is try to comply with the coding style I find
in a file. Thus, if there are, from time to time, lines overcoming
col80, I tend to relax such constrain a bit (especially when I change
one of those lines). Anyway, for v2 I'll double check that, as I don't
like long lines either. :-)

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file.
  2012-01-12 22:56     ` Dario Faggioli
@ 2012-01-13  8:09       ` Ian Campbell
  2012-01-13  9:13         ` Dario Faggioli
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2012-01-13  8:09 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Ian Jackson

On Thu, 2012-01-12 at 22:56 +0000, Dario Faggioli wrote:
> On Thu, 2012-01-12 at 08:43 +0000, Ian Campbell wrote: 
> > > diff -r 9ce68a4036b1 tools/libxl/libxl_create.c
> > > --- a/tools/libxl/libxl_create.c	Wed Jan 11 17:38:04 2012 +0000
> > > +++ b/tools/libxl/libxl_create.c	Wed Jan 11 17:40:45 2012 +0000
> > > @@ -78,6 +78,8 @@ int libxl_init_build_info(libxl_ctx *ctx
> > >      memset(b_info, '\0', sizeof(*b_info));
> > >      b_info->max_vcpus = 1;
> > >      b_info->cur_vcpus = 1;
> > > +    if (libxl_cpumap_alloc(ctx, &b_info->cpumap))
> > > +        return ERROR_NOMEM;
> > 
> > Should probably set all here since that is the best default?
> > 
> Having this set to "none" here simplifies a bit the code when we came to
> config file parsing (if I set it to "any CPU" here, I'll have to reset
> to "none" before parsing). Also, default is exactly what you're
> suggesting already, as I set the map to "any CPU" if no "cpus" config
> option is found.

xl's default is "any CPU" but libxl's default is "no CPU" which will
mean every toolstack author needs to do be aware of this even if they
don't care about affinity.

> Anyway, I guess I can do as you suggest if you really think it's
> better. :-)

Please ;-)

> > >  static void parse_config_data(const char *configfile_filename_report,
> > >                                const char *configfile_data,
> > >                                int configfile_len,
> > > @@ -661,6 +674,13 @@ static void parse_config_data(const char
> > >      if (!xlu_cfg_get_long (config, "maxvcpus", &l, 0))
> > >          b_info->max_vcpus = l;
> > >  
> > > +    if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
> > 
> > The syntax supported here is different to that in a cpupool cfg? (see
> > main_cpupoolcreate) Perhaps they should be similar?
> > 
> It is different, and that was intentional. What I wanted, was the syntax
> to be exactly the same of `xl vcpu-pin', which is the command line
> equivalent of this option, much more than cpupool-*. If you want, I
> think I can easily enable list-like CPU specification as in cpupool
> config file so that _both_ syntax are allowed. What do you think?

I think supporting both makes sense, we do something similar in a couple
of places already.

Ian.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file.
  2012-01-13  8:09       ` Ian Campbell
@ 2012-01-13  9:13         ` Dario Faggioli
  0 siblings, 0 replies; 19+ messages in thread
From: Dario Faggioli @ 2012-01-13  9:13 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Ian Jackson


[-- Attachment #1.1: Type: text/plain, Size: 1714 bytes --]

On Fri, 2012-01-13 at 08:09 +0000, Ian Campbell wrote: 
> > Having this set to "none" here simplifies a bit the code when we came to
> > config file parsing (if I set it to "any CPU" here, I'll have to reset
> > to "none" before parsing). Also, default is exactly what you're
> > suggesting already, as I set the map to "any CPU" if no "cpus" config
> > option is found.
> 
> xl's default is "any CPU" but libxl's default is "no CPU" which will
> mean every toolstack author needs to do be aware of this even if they
> don't care about affinity.
> 
Ok, I think I see your point now, which I didn't in the first place, and
neither yesterday while replying. Sorry, my fault, I agree I need to set
it to "any CPU" right here.

> > Anyway, I guess I can do as you suggest if you really think it's
> > better. :-)
> 
> Please ;-)
> 
Sure, will do like that.

> > It is different, and that was intentional. What I wanted, was the syntax
> > to be exactly the same of `xl vcpu-pin', which is the command line
> > equivalent of this option, much more than cpupool-*. If you want, I
> > think I can easily enable list-like CPU specification as in cpupool
> > config file so that _both_ syntax are allowed. What do you think?
> 
> I think supporting both makes sense, we do something similar in a couple
> of places already.
> 
Ok then.

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-12 23:12     ` Dario Faggioli
@ 2012-01-13 13:34       ` Ian Jackson
  2012-01-13 16:45         ` Dario Faggioli
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2012-01-13 13:34 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Ian Campbell

Dario Faggioli writes ("Re: [Xen-devel] [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin."):
> Well, that's not properly me, is it that?
> 
> hg annotate tools/libxl/xl_cmdimpl.c
> ...
> 21247:     if (strcmp(cpu, "all")) { 
> 21247:         for (toka = strtok(cpu, ","), i = 0; toka; toka = strtok(NULL, ","), ++i) {
> 21247:             cpuida = strtoul(toka, &endptr, 10);
> 21247:             if (toka == endptr) {
> ...

Oops, sorry for wrongly pointing the finger.

> Again, I know that's probably not an alibi, but that's not my code! :-P
> What I did is just move this from the body of vcpupin() to an helper
> function, and try to add as _less_ thing as I can to support a slightly
> improved syntax.

Right.

> That being said, if you think I should rewrite the parsing from scratch,
> I can think about it, just wanted to clarify my position. :-)

I really don't like the existing code here; feel free to do whatever
you think is best.

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-13 13:34       ` Ian Jackson
@ 2012-01-13 16:45         ` Dario Faggioli
  2012-01-13 16:48           ` Ian Jackson
  0 siblings, 1 reply; 19+ messages in thread
From: Dario Faggioli @ 2012-01-13 16:45 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Ian Campbell


[-- Attachment #1.1: Type: text/plain, Size: 933 bytes --]

On Fri, 2012-01-13 at 13:34 +0000, Ian Jackson wrote: 
> Oops, sorry for wrongly pointing the finger.
> 
NP. :-)

> > That being said, if you think I should rewrite the parsing from scratch,
> > I can think about it, just wanted to clarify my position. :-)
> 
> I really don't like the existing code here; feel free to do whatever
> you think is best.
> 
Ok, I'm not really a "string parsing guy" but I'm interested in getting
better. I can try restructuring this with strtok_r and strdup... Would
that be better or you where thinking to something more "radical"?

Thanks,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-13 16:45         ` Dario Faggioli
@ 2012-01-13 16:48           ` Ian Jackson
  2012-01-23 10:21             ` Ian Campbell
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2012-01-13 16:48 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Ian Campbell

Dario Faggioli writes ("Re: [Xen-devel] [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin."):
> Ok, I'm not really a "string parsing guy" but I'm interested in getting
> better. I can try restructuring this with strtok_r and strdup... Would
> that be better or you where thinking to something more "radical"?

Personally I'm a fan of flex although it's probably overkill for
this.  But you should use an approach your comfortable with.

What I care most about is that whatever approach you take doesn't make
it too easy to make and hide :-) programming mistakes, since otherwise
parsing code often turns out to be buggy.

Ian.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-13 16:48           ` Ian Jackson
@ 2012-01-23 10:21             ` Ian Campbell
  2012-01-23 10:27               ` Dario Faggioli
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2012-01-23 10:21 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Dario Faggioli

On Fri, 2012-01-13 at 16:48 +0000, Ian Jackson wrote:
> Dario Faggioli writes ("Re: [Xen-devel] [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin."):
> > Ok, I'm not really a "string parsing guy" but I'm interested in getting
> > better. I can try restructuring this with strtok_r and strdup... Would
> > that be better or you where thinking to something more "radical"?
> 
> Personally I'm a fan of flex although it's probably overkill for
> this.  But you should use an approach your comfortable with.
> 
> What I care most about is that whatever approach you take doesn't make
> it too easy to make and hide :-) programming mistakes, since otherwise
> parsing code often turns out to be buggy.

Can we take the version which just moves the dodgy parsee now and worry
about reworking it later? I think we'd like to see vcpu pinning in 4.2.

Ian.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-23 10:21             ` Ian Campbell
@ 2012-01-23 10:27               ` Dario Faggioli
  2012-01-23 10:50                 ` Ian Campbell
  0 siblings, 1 reply; 19+ messages in thread
From: Dario Faggioli @ 2012-01-23 10:27 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Ian Jackson


[-- Attachment #1.1: Type: text/plain, Size: 722 bytes --]

On Mon, 2012-01-23 at 10:21 +0000, Ian Campbell wrote: 
> Can we take the version which just moves the dodgy parsee now and worry
> about reworking it later? I think we'd like to see vcpu pinning in 4.2.
> 
Hey, sorry for being so late in resubmitting, I've been distrcted by
those (other) NUME issues... I'll send a new version of this in the
afternoon!

Sorry again,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-------------------------------------------------------------------
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin.
  2012-01-23 10:27               ` Dario Faggioli
@ 2012-01-23 10:50                 ` Ian Campbell
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Campbell @ 2012-01-23 10:50 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Ian Jackson

On Mon, 2012-01-23 at 10:27 +0000, Dario Faggioli wrote:
> On Mon, 2012-01-23 at 10:21 +0000, Ian Campbell wrote: 
> > Can we take the version which just moves the dodgy parsee now and worry
> > about reworking it later? I think we'd like to see vcpu pinning in 4.2.
> > 
> Hey, sorry for being so late in resubmitting, I've been distrcted by
> those (other) NUME issues... I'll send a new version of this in the
> afternoon!
> 
> Sorry again,

No need to apologise. I was just going over the 4.2 TODO list to repost
it. I've added "xl support for vcpu pinning" to the tools section and
"NUMA improvements (cpupool autopin?)" to the hypervisor (nice to have)
section

Ian.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2012-01-23 10:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-11 17:49 [PATCH 0 of 3] libxl: Extend CPU affinity specification and enable it in config file Dario Faggioli
2012-01-11 17:58 ` [PATCH 1 of 3] libxl: extend pCPUs specification for vcpu-pin Dario Faggioli
2012-01-12  8:38   ` Ian Campbell
2012-01-12 17:34   ` Ian Jackson
2012-01-12 23:12     ` Dario Faggioli
2012-01-13 13:34       ` Ian Jackson
2012-01-13 16:45         ` Dario Faggioli
2012-01-13 16:48           ` Ian Jackson
2012-01-23 10:21             ` Ian Campbell
2012-01-23 10:27               ` Dario Faggioli
2012-01-23 10:50                 ` Ian Campbell
2012-01-11 18:00 ` [PATCH 2 of 3] libxl: allow for specifying the CPU affinity in the config file Dario Faggioli
2012-01-12  8:43   ` Ian Campbell
2012-01-12 22:56     ` Dario Faggioli
2012-01-13  8:09       ` Ian Campbell
2012-01-13  9:13         ` Dario Faggioli
2012-01-11 18:01 ` [PATCH 3 of 3] libxl: Align examples with current code Dario Faggioli
2012-01-12  8:37   ` Ian Campbell
2012-01-12 22:45     ` Dario Faggioli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.