Re: [Xen-devel] [PATCH v5 24/24] xl: vNUMA support
On Tue, Feb 24, 2015 at 04:19:02PM +, Dario Faggioli wrote:
On Thu, 2015-02-12 at 19:44 +, Wei Liu wrote:
This patch includes configuration options parser and documentation.
Please find the hunk to xl.cfg.pod.5 for more information.
Signed-off-by: Wei Liu wei.l...@citrix.com
Cc: Ian Campbell ian.campb...@citrix.com
Cc: Ian Jackson ian.jack...@eu.citrix.com
This all looks pretty good to me. I only have one comment and a
question.
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ec7fb2d..f52daf9 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
[...]
+if (!strcmp(pnode, option)) {
+val = strtoul(value, endptr, 10);
+ABORT_IF_FAILED(value);
+if (val = nr_nodes) {
+fprintf(stderr,
+xl: invalid pnode number: %lu\n, val);
+exit(1);
+}
+p-pnode = val;
This is, to all the effects, a form of placement so, if this part of
vNUMA specification is present, you should disable the automatic
placement happening in libxl.
This is all it takes to do so (look inside parse_vcpu_affinity() if you
need more insights):
libxl_defbool_set(b_info-numa_placement, false);
Will fix this.
+} else if (!strcmp(size, option)) {
+val = strtoul(value, endptr, 10);
+ABORT_IF_FAILED(value);
+p-memkb = val 10;
+} else if (!strcmp(vcpus, option)) {
+libxl_string_list cpu_spec_list;
+int cpu;
+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 (cpu = s; cpu =e; cpu++)
+libxl_bitmap_set(p-vcpus, cpu);
+}
+libxl_string_list_dispose(cpu_spec_list);
I think that using vcpupin_parse() for vcpus= would allow for more
flexible syntax (i.e., things like 3-8,^5), and save some code. The
only downside is that it also accepts things like nodes:1, which we
clearly don't want in here... is that why you are not going for it?
Yes. I don't want nodes so I didn't reuse that function, and at that
point I didn't think it's critical to support ^X.
If you think this ^X syntax is important, I can check for nodes
before calling vcpupin_parse.
If you decide to use it, BTW, you may want to change its name (again!)
vcpus_parse? It's not restricted to vcpu pinning in any way, I think.
Wei.
Regards,
Dario
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 24/24] xl: vNUMA support
On Tue, 2015-02-24 at 16:31 +, Wei Liu wrote:
On Tue, Feb 24, 2015 at 04:19:02PM +, Dario Faggioli wrote:
+} else if (!strcmp(size, option)) {
+val = strtoul(value, endptr, 10);
+ABORT_IF_FAILED(value);
+p-memkb = val 10;
+} else if (!strcmp(vcpus, option)) {
+libxl_string_list cpu_spec_list;
+int cpu;
+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 (cpu = s; cpu =e; cpu++)
+libxl_bitmap_set(p-vcpus, cpu);
+}
+libxl_string_list_dispose(cpu_spec_list);
I think that using vcpupin_parse() for vcpus= would allow for more
flexible syntax (i.e., things like 3-8,^5), and save some code. The
only downside is that it also accepts things like nodes:1, which we
clearly don't want in here... is that why you are not going for it?
Yes. I don't want nodes so I didn't reuse that function, and at that
point I didn't think it's critical to support ^X.
Ok, I just wanted to be sure you were aware of the possibility. I
actually agree that supporting ^x is not that critical here.
If you think this ^X syntax is important, I can check for nodes
before calling vcpupin_parse.
I don't think it is... TBH, I'm more attracted by the code being
potentially simpler, and less duplicate parsing logic being around, but
I appreciate that having to check for node[s] not being present up
front would make things look clumsy... so I'm leaving this to you, I'm
happy either way.
vcpus_parse? It's not restricted to vcpu pinning in any way, I think.
If you go for it, yes, I like this as a name.
Regards,
Dario
signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 24/24] xl: vNUMA support
This patch includes configuration options parser and documentation.
Please find the hunk to xl.cfg.pod.5 for more information.
Signed-off-by: Wei Liu wei.l...@citrix.com
Cc: Ian Campbell ian.campb...@citrix.com
Cc: Ian Jackson ian.jack...@eu.citrix.com
---
Changes in v5:
1. New syntax for vNUMA configuration.
---
docs/man/xl.cfg.pod.5| 54 ++
tools/libxl/xl_cmdimpl.c | 139 ++-
2 files changed, 192 insertions(+), 1 deletion(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 408653f..2a27b1c 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -266,6 +266,60 @@ it will crash.
=back
+=head3 Guest Virtual NUMA Configuration
+
+=over 4
+
+=item Bvnuma=[ VNODE_SPEC, VNODE_SPEC, ... ]
+
+Specify virtual NUMA configuration with positional arguments. The
+nth BVNODE_SPECE in the list specifies the configuration of nth
+virtual node.
+
+Each BVNODE_SPEC is a list, which has a form of
+[VNODE_CONFIG_OPTION,VNODE_CONFIG_OPTION, ... ] (without quotes).
+
+For example vnuma = [ [pnode=0,size=512,vcpus=0-4,vdistances=10,20] ]
+means vnode 0 is mapped to pnode 0, has 512MB ram, has vcpus 0 to 4, the
+distance to itself is 10 and the distance to vnode 1 is 20.
+
+Each BVNODE_CONFIG_OPTION is a quoted string. Supported
+BVNODE_CONFIG_OPTIONs are:
+
+=over 4
+
+=item Bpnode=NUMBER
+
+Specify which physical node this virtual node maps to.
+
+=item Bsize=MBYTES
+
+Specify the size of this virtual node. The sum of memory size of all
+vnodes must match Bmaxmem= (or Bmemory= if Bmaxmem= is not
+specified).
+
+=item Bvcpus=CPU-STRING
+
+Specify which vcpus belong to this node. BCPU-STRING is a string
+separated by comma. You can specify range and single cpu. An example
+is vcpus=0-5,8, which means you specify vcpu 0 to vcpu 5, and vcpu
+8.
+
+=item Bvdistances=NUMBER, NUMBER, ...
+
+Specify virtual distance from this node to all nodes (including
+itself) with positional arguments. For example, vdistance=10,20
+for vnode 0 means the distance from vnode 0 to vnode 0 is 10, from
+vnode 0 to vnode 1 is 20. The number of arguments supplied must match
+the total number of vnodes.
+
+Normally you can use the values from xl info -n or numactl
+--hardware to fill in vdistance list.
+
+=back
+
+=back
+
=head3 Event Actions
=over 4
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ec7fb2d..f52daf9 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -158,7 +158,6 @@ struct domain_create {
};
-static uint32_t find_domain(const char *p) __attribute__((warn_unused_result));
static uint32_t find_domain(const char *p)
{
uint32_t domid;
@@ -989,6 +988,142 @@ static int parse_nic_config(libxl_device_nic *nic,
XLU_Config **config, char *to
return 0;
}
+static void parse_vnuma_config(const XLU_Config *config,
+ libxl_domain_build_info *b_info)
+{
+libxl_physinfo physinfo;
+uint32_t nr_nodes;
+XLU_ConfigList *vnuma;
+int i, j, len, num_vnuma;
+
+
+libxl_physinfo_init(physinfo);
+if (libxl_get_physinfo(ctx, physinfo) != 0) {
+libxl_physinfo_dispose(physinfo);
+fprintf(stderr, libxl_get_physinfo failed\n);
+exit(1);
+}
+
+nr_nodes = physinfo.nr_nodes;
+libxl_physinfo_dispose(physinfo);
+
+if (xlu_cfg_get_list(config, vnuma, vnuma, num_vnuma, 1))
+return;
+
+b_info-num_vnuma_nodes = num_vnuma;
+b_info-vnuma_nodes = xcalloc(num_vnuma, sizeof(libxl_vnode_info));
+
+for (i = 0; i b_info-num_vnuma_nodes; i++) {
+libxl_vnode_info *p = b_info-vnuma_nodes[i];
+
+libxl_vnode_info_init(p);
+libxl_cpu_bitmap_alloc(ctx, p-vcpus, b_info-max_vcpus);
+libxl_bitmap_set_none(p-vcpus);
+p-distances = xcalloc(b_info-num_vnuma_nodes,
+ sizeof(*p-distances));
+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];
+
+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;
+char *endptr;
+unsigned long val;
+
+xlu_cfg_value_get_string(config, conf_option, buf, 0);
+
+if (!buf)
