[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH for-4.6 2/3] xl: fix vNUMA vcpus parsing



Originally, if user didn't specify maxvcpus= in xl config file, the
maximum size of vcpu bitmap was always equal to maximum number of pcpus.
This might not be what user wants.

Calculate the maximum number of vcpus before allocating vcpu bitmap.
This requires parsing the same config options twice. Extra a macro to do
that.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
Valgrind is still clean after this patch applied.

This patch looks massive because it involves indentation changes, but in
fact the meat of it is small.
---
 tools/libxl/xl_cmdimpl.c | 189 ++++++++++++++++++++++++++---------------------
 1 file changed, 106 insertions(+), 83 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 078acd1..0fcef98 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1079,6 +1079,10 @@ static void parse_vnuma_config(const XLU_Config *config,
      * parsing config twice. This array has num_vnuma elements.
      */
     libxl_bitmap *vcpu_parsed;
+    libxl_string_list cpu_spec_list;
+    unsigned long s, e;
+    libxl_string_list vdist;
+    unsigned long val;
 
     libxl_physinfo_init(&physinfo);
     if (libxl_get_physinfo(ctx, &physinfo) != 0) {
@@ -1096,13 +1100,6 @@ static void parse_vnuma_config(const XLU_Config *config,
     b_info->num_vnuma_nodes = num_vnuma;
     b_info->vnuma_nodes = xcalloc(num_vnuma, sizeof(libxl_vnode_info));
     vcpu_parsed = xcalloc(num_vnuma, sizeof(libxl_bitmap));
-    for (i = 0; i < num_vnuma; i++) {
-        libxl_bitmap_init(&vcpu_parsed[i]);
-        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], b_info->max_vcpus)) {
-            fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
-            exit(1);
-        }
-    }
 
     for (i = 0; i < b_info->num_vnuma_nodes; i++) {
         libxl_vnode_info *p = &b_info->vnuma_nodes[i];
@@ -1113,93 +1110,119 @@ static void parse_vnuma_config(const XLU_Config 
*config,
         p->num_distances = b_info->num_vnuma_nodes;
     }
 
-    for (i = 0; i < num_vnuma; i++) {
-        XLU_ConfigValue *vnode_spec, *conf_option;
-        XLU_ConfigList *vnode_config_list;
-        int conf_count;
-        libxl_vnode_info *p = &b_info->vnuma_nodes[i];
+#define PARSE_VNUMA_SPEC(body)                                          \
+    for (i = 0; i < num_vnuma; i++) {                                   \
+        XLU_ConfigValue *vnode_spec, *conf_option;                      \
+        XLU_ConfigList *vnode_config_list;                              \
+        int conf_count;                                                 \
+                                                                        \
+        vnode_spec = xlu_cfg_get_listitem2(vnuma, i);                   \
+        assert(vnode_spec);                                             \
+                                                                        \
+        xlu_cfg_value_get_list(config, vnode_spec,                      \
+                               &vnode_config_list, 0);                  \
+        if (!vnode_config_list) {                                       \
+            fprintf(stderr,                                             \
+                    "xl: cannot get vnode config option list\n");       \
+            exit(1);                                                    \
+        }                                                               \
+                                                                        \
+        for (conf_count = 0;                                            \
+             (conf_option =                                             \
+              xlu_cfg_get_listitem2(vnode_config_list, conf_count));    \
+             conf_count++) {                                            \
+                                                                        \
+            if (xlu_cfg_value_type(conf_option) == XLU_STRING) {        \
+                char *buf, *option_untrimmed, *value_untrimmed;         \
+                char *option, *value;                                   \
+                                                                        \
+                xlu_cfg_value_get_string(config, conf_option, &buf, 0); \
+                                                                        \
+                if (!buf) continue;                                     \
+                                                                        \
+                if (split_string_into_pair(buf, "=",                    \
+                                           &option_untrimmed,           \
+                                           &value_untrimmed)) {         \
+                    fprintf(stderr,                                     \
+                            "xl: failed to split \"%s\" into pair\n",   \
+                            buf);                                       \
+                    exit(1);                                            \
+                }                                                       \
+                trim(isspace, option_untrimmed, &option);               \
+                trim(isspace, value_untrimmed, &value);                 \
+                                                                        \
+                body;                                                   \
+                                                                        \
+                free(option);                                           \
+                free(value);                                            \
+                free(option_untrimmed);                                 \
+                free(value_untrimmed);                                  \
+            }                                                           \
+        }                                                               \
+    }
+
+    /* Pass one, get the total number of vcpus */
+    PARSE_VNUMA_SPEC({
+            if (!strcmp("vcpus", option)) {
+                split_string_into_string_list(value, ",", &cpu_spec_list);
+                len = libxl_string_list_length(&cpu_spec_list);
 
-        vnode_spec = xlu_cfg_get_listitem2(vnuma, i);
-        assert(vnode_spec);
+                for (j = 0; j < len; j++) {
+                    parse_range(cpu_spec_list[j], &s, &e);
+                    for (; s <= e; s++)
+                        max_vcpus++;
+                }
+                libxl_string_list_dispose(&cpu_spec_list);
+            }
+        });
 
-        xlu_cfg_value_get_list(config, vnode_spec, &vnode_config_list, 0);
-        if (!vnode_config_list) {
-            fprintf(stderr, "xl: cannot get vnode config option list\n");
+    for (i = 0; i < num_vnuma; i++) {
+        libxl_bitmap_init(&vcpu_parsed[i]);
+        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], max_vcpus)) {
+            fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
             exit(1);
         }
+    }
 
-        for (conf_count = 0;
-             (conf_option =
-              xlu_cfg_get_listitem2(vnode_config_list, conf_count));
-             conf_count++) {
-
-            if (xlu_cfg_value_type(conf_option) == XLU_STRING) {
-                char *buf, *option_untrimmed, *value_untrimmed;
-                char *option, *value;
-                unsigned long val;
-
-                xlu_cfg_value_get_string(config, conf_option, &buf, 0);
-
-                if (!buf) continue;
-
-                if (split_string_into_pair(buf, "=",
-                                           &option_untrimmed,
-                                           &value_untrimmed)) {
-                    fprintf(stderr, "xl: failed to split \"%s\" into pair\n",
-                            buf);
+    /* Pass two, fill in structs */
+    PARSE_VNUMA_SPEC({
+            if (!strcmp("pnode", option)) {
+                val = parse_ulong(value);
+                if (val >= nr_nodes) {
+                    fprintf(stderr,
+                            "xl: invalid pnode number: %lu\n", val);
                     exit(1);
                 }
-                trim(isspace, option_untrimmed, &option);
-                trim(isspace, value_untrimmed, &value);
-
-                if (!strcmp("pnode", option)) {
-                    val = parse_ulong(value);
-                    if (val >= nr_nodes) {
-                        fprintf(stderr,
-                                "xl: invalid pnode number: %lu\n", val);
-                        exit(1);
-                    }
-                    p->pnode = val;
-                    libxl_defbool_set(&b_info->numa_placement, false);
-                } else if (!strcmp("size", option)) {
-                    val = parse_ulong(value);
-                    p->memkb = val << 10;
-                    max_memkb += p->memkb;
-                } else if (!strcmp("vcpus", option)) {
-                    libxl_string_list cpu_spec_list;
-                    unsigned long s, e;
-
-                    split_string_into_string_list(value, ",", &cpu_spec_list);
-                    len = libxl_string_list_length(&cpu_spec_list);
-
-                    for (j = 0; j < len; j++) {
-                        parse_range(cpu_spec_list[j], &s, &e);
-                        for (; s <= e; s++) {
-                            libxl_bitmap_set(&vcpu_parsed[i], s);
-                            max_vcpus++;
-                        }
-                    }
-
-                    libxl_string_list_dispose(&cpu_spec_list);
-                } else if (!strcmp("vdistances", option)) {
-                    libxl_string_list vdist;
+                b_info->vnuma_nodes[i].pnode = val;
+                libxl_defbool_set(&b_info->numa_placement, false);
+            } else if (!strcmp("size", option)) {
+                val = parse_ulong(value);
+                b_info->vnuma_nodes[i].memkb = val << 10;
+                max_memkb += b_info->vnuma_nodes[i].memkb;
+            } else if (!strcmp("vcpus", option)) {
+                split_string_into_string_list(value, ",", &cpu_spec_list);
+                len = libxl_string_list_length(&cpu_spec_list);
+
+                for (j = 0; j < len; j++) {
+                    parse_range(cpu_spec_list[j], &s, &e);
+                    for (; s <= e; s++)
+                        libxl_bitmap_set(&vcpu_parsed[i], s);
+                }
 
-                    split_string_into_string_list(value, ",", &vdist);
-                    len = libxl_string_list_length(&vdist);
+                libxl_string_list_dispose(&cpu_spec_list);
+            } else if (!strcmp("vdistances", option)) {
+                split_string_into_string_list(value, ",", &vdist);
+                len = libxl_string_list_length(&vdist);
 
-                    for (j = 0; j < len; j++) {
-                        val = parse_ulong(vdist[j]);
-                        p->distances[j] = val;
-                    }
-                    libxl_string_list_dispose(&vdist);
+                for (j = 0; j < len; j++) {
+                    val = parse_ulong(vdist[j]);
+                    b_info->vnuma_nodes[i].distances[j] = val;
                 }
-                free(option);
-                free(value);
-                free(option_untrimmed);
-                free(value_untrimmed);
+                libxl_string_list_dispose(&vdist);
             }
-        }
-    }
+        });
+
+#undef PARSE_VNUMA_SPEC
 
     /* User has specified maxvcpus= */
     if (b_info->max_vcpus != 0 &&  b_info->max_vcpus != max_vcpus) {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.