12:14 p.m.
This series contains the following:
* Patches 1-7 are multiple bug fixes to the current code
* Patch 8 introduce a feature that libvirt requires since a long time,
and even tries to use it today (in a way that doesn't work,
using the "-numa node,cpus=1,2,3,4" format): having non-contiguous CPU
ranges assigned to a NUMA node.
The last 2 patches I am sending as RFCs:
* Patch 9 makes the "-numa" option deprecated and introduces a
"-numa-node"
command-line and config file option.
* Patch 10 adds a small hack to the (now deprecated) "-numa" option, that
makes the "cpus=1,2,3,4" format currently used by libvirt work.
Eduardo Habkost (10):
vl.c: Fix off-by-one bug when handling "-numa node" argument
vl.c: Abort on unknown -numa option type
vl.c: Isolate code specific to "-numa node" option type
vl.c: Check for NUMA node limit inside numa_node_add()
vl.c: Extract -numa "cpus" parsing to separate function
vl.c: handle invalid NUMA CPU ranges properly
vl.c: numa_add_node(): Validate nodeid before using it
vl.c: Support multiple CPU ranges on -numa option
vl.c: Introduce QemuOpts-friendly "-numa-node" config option
vl.c: Handle legacy "-numa node,cpus=A,B,C,D" format
qemu-config.c | 25 +++++++
qemu-options.hx | 50 +++++++++++++-
vl.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++----------
3 files changed, 240 insertions(+), 39 deletions(-)
--
1.7.11.7
12:14 p.m.
New subject: [libvirt] [PATCH 01/10] vl.c: Fix off-by-one bug when handling "-numa node" argument
The numa_add() code was unconditionally adding 1 to the get_opt_name()
return value, making it point after the end of the string if no ','
separator is present.
Example of weird behavior caused by the bug:
$ qemu-img create -f qcow2 this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2 5G
Formatting 'this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2', fmt=qcow2
size=5368709120 encryption=off cluster_size=65536
$ ./x86_64-softmmu/qemu-system-x86_64 -S -monitor stdio -numa node
'this-file-image-has,cpus=5,mem=1000,in-its-name.qcow2'
QEMU 1.3.50 monitor - type 'help' for more information
(qemu) info numa
1 nodes
node 0 cpus: 0
node 0 size: 1000 MB
(qemu)
This changes the code to nove the pointer only if ',' is found.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index e5da31c..93fde36 100644
--- a/vl.c
+++ b/vl.c
@@ -1061,7 +1061,10 @@ static void numa_add(const char *optarg)
value = endvalue = 0ULL;
- optarg = get_opt_name(option, 128, optarg, ',') + 1;
+ optarg = get_opt_name(option, 128, optarg, ',');
+ if (*optarg == ',') {
+ optarg++;
+ }
if (!strcmp(option, "node")) {
if (get_param_value(option, 128, "nodeid", optarg) == 0) {
nodenr = nb_numa_nodes;
--
1.7.11.7
12:15 p.m.
New subject: [libvirt] [PATCH 02/10] vl.c: Abort on unknown -numa option type
Instead of silently ignoring them, abort in case an invalid -numa option
is provided.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/vl.c b/vl.c
index 93fde36..042cc7f 100644
--- a/vl.c
+++ b/vl.c
@@ -1101,6 +1101,9 @@ static void numa_add(const char *optarg)
bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
}
nb_numa_nodes++;
+ } else {
+ fprintf(stderr, "Invalid -numa option: %s\n", option);
+ exit(1);
}
}
--
1.7.11.7
12:15 p.m.
New subject: [libvirt] [PATCH 03/10] vl.c: Isolate code specific to "-numa node" option type
Extract the code that's specific for the "node" -numa option type (the
only one, today) to a separate function.
The extracted code will eventually become a function specific for a
"numa-node" config section, independent from the numa_add() code.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 75 +++++++++++++++++++++++++++++++++++++-------------------------------
1 file changed, 41 insertions(+), 34 deletions(-)
diff --git a/vl.c b/vl.c
index 042cc7f..94cc6fd 100644
--- a/vl.c
+++ b/vl.c
@@ -1052,7 +1052,7 @@ char *get_boot_devices_list(uint32_t *size)
return list;
}
-static void numa_add(const char *optarg)
+static void numa_node_add(const char *optarg)
{
char option[128];
char *endptr;
@@ -1061,46 +1061,53 @@ static void numa_add(const char *optarg)
value = endvalue = 0ULL;
- optarg = get_opt_name(option, 128, optarg, ',');
- if (*optarg == ',') {
- optarg++;
+ if (get_param_value(option, 128, "nodeid", optarg) == 0) {
+ nodenr = nb_numa_nodes;
+ } else {
+ nodenr = strtoull(option, NULL, 10);
}
- if (!strcmp(option, "node")) {
- if (get_param_value(option, 128, "nodeid", optarg) == 0) {
- nodenr = nb_numa_nodes;
+
+ if (get_param_value(option, 128, "mem", optarg) == 0) {
+ node_mem[nodenr] = 0;
+ } else {
+ int64_t sval;
+ sval = strtosz(option, &endptr);
+ if (sval < 0 || *endptr) {
+ fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
+ exit(1);
+ }
+ node_mem[nodenr] = sval;
+ }
+ if (get_param_value(option, 128, "cpus", optarg) != 0) {
+ value = strtoull(option, &endptr, 10);
+ if (*endptr == '-') {
+ endvalue = strtoull(endptr+1, &endptr, 10);
} else {
- nodenr = strtoull(option, NULL, 10);
+ endvalue = value;
}
- if (get_param_value(option, 128, "mem", optarg) == 0) {
- node_mem[nodenr] = 0;
- } else {
- int64_t sval;
- sval = strtosz(option, &endptr);
- if (sval < 0 || *endptr) {
- fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
- exit(1);
- }
- node_mem[nodenr] = sval;
+ if (!(endvalue < MAX_CPUMASK_BITS)) {
+ endvalue = MAX_CPUMASK_BITS - 1;
+ fprintf(stderr,
+ "A max of %d CPUs are supported in a guest\n",
+ MAX_CPUMASK_BITS);
}
- if (get_param_value(option, 128, "cpus", optarg) != 0) {
- value = strtoull(option, &endptr, 10);
- if (*endptr == '-') {
- endvalue = strtoull(endptr+1, &endptr, 10);
- } else {
- endvalue = value;
- }
- if (!(endvalue < MAX_CPUMASK_BITS)) {
- endvalue = MAX_CPUMASK_BITS - 1;
- fprintf(stderr,
- "A max of %d CPUs are supported in a guest\n",
- MAX_CPUMASK_BITS);
- }
+ bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+ }
+ nb_numa_nodes++;
+}
- bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
- }
- nb_numa_nodes++;
+static void numa_add(const char *optarg)
+{
+ char option[128];
+
+ optarg = get_opt_name(option, 128, optarg, ',');
+ if (*optarg == ',') {
+ optarg++;
+ }
+ if (!strcmp(option, "node")) {
+ numa_node_add(optarg);
} else {
fprintf(stderr, "Invalid -numa option: %s\n", option);
exit(1);
--
1.7.11.7
3:06 p.m.
New subject: [libvirt] [PATCH 03/10] vl.c: Isolate code specific to "-numa node" option type
On 01/11/2013 11:15 AM, Eduardo Habkost wrote:
Extract the code that's specific for the "node" -numa
option type (the
only one, today) to a separate function.
The extracted code will eventually become a function specific for a
"numa-node" config section, independent from the numa_add() code.
+ if (get_param_value(option, 128, "nodeid", optarg) ==
0) {
+ nodenr = nb_numa_nodes;
+ } else {
+ nodenr = strtoull(option, NULL, 10);
}
strtoull() needs additional error checking after the fact, to make sure
I didn't pass an empty string, trailing garbage, or so many digits I
triggered overflow.
+ if (get_param_value(option, 128, "cpus", optarg) != 0)
{
+ value = strtoull(option, &endptr, 10);
+ if (*endptr == '-') {
+ endvalue = strtoull(endptr+1, &endptr, 10);
} else {
- nodenr = strtoull(option, NULL, 10);
+ endvalue = value;
}
More uses of strtoull() that aren't guarding against all possible errors.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:19 p.m.
New subject: [libvirt] [PATCH 03/10] vl.c: Isolate code specific to "-numa node" option type
On Fri, Jan 11, 2013 at 02:06:05PM -0700, Eric Blake wrote:
On 01/11/2013 11:15 AM, Eduardo Habkost wrote:
> Extract the code that's specific for the "node" -numa option type
(the
> only one, today) to a separate function.
>
> The extracted code will eventually become a function specific for a
> "numa-node" config section, independent from the numa_add() code.
> + if (get_param_value(option, 128, "nodeid", optarg) == 0) {
> + nodenr = nb_numa_nodes;
> + } else {
> + nodenr = strtoull(option, NULL, 10);
> }
strtoull() needs additional error checking after the fact, to make sure
I didn't pass an empty string, trailing garbage, or so many digits I
triggered overflow.
> + if (get_param_value(option, 128, "cpus", optarg) != 0) {
> + value = strtoull(option, &endptr, 10);
> + if (*endptr == '-') {
> + endvalue = strtoull(endptr+1, &endptr, 10);
> } else {
> - nodenr = strtoull(option, NULL, 10);
> + endvalue = value;
> }
More uses of strtoull() that aren't guarding against all possible errors.
All this code gets replaced in patch 09/10. The only remaining
strtoull() call gets error-checking at patch 06/10.
--
Eduardo
12:15 p.m.
New subject: [libvirt] [PATCH 04/10] vl.c: Check for NUMA node limit inside numa_node_add()
Instead of checking the limit before calling numa_add(), check the limit
only when we already know we're going to add a new node.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index 94cc6fd..2c3bbb9 100644
--- a/vl.c
+++ b/vl.c
@@ -1061,6 +1061,11 @@ static void numa_node_add(const char *optarg)
value = endvalue = 0ULL;
+ if (nb_numa_nodes >= MAX_NODES) {
+ fprintf(stderr, "qemu: too many NUMA nodes\n");
+ exit(1);
+ }
+
if (get_param_value(option, 128, "nodeid", optarg) == 0) {
nodenr = nb_numa_nodes;
} else {
@@ -2762,10 +2767,6 @@ int main(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_numa:
- if (nb_numa_nodes >= MAX_NODES) {
- fprintf(stderr, "qemu: too many NUMA nodes\n");
- exit(1);
- }
numa_add(optarg);
break;
case QEMU_OPTION_display:
--
1.7.11.7
12:15 p.m.
New subject: [libvirt] [PATCH 05/10] vl.c: Extract -numa "cpus" parsing to separate function
This will make it easier to refactor that code later.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/vl.c b/vl.c
index 2c3bbb9..03a826e 100644
--- a/vl.c
+++ b/vl.c
@@ -1052,15 +1052,33 @@ char *get_boot_devices_list(uint32_t *size)
return list;
}
+static void numa_node_parse_cpus(int nodenr, const char *cpus)
+{
+ char *endptr;
+ unsigned long long value, endvalue;
+
+ value = strtoull(cpus, &endptr, 10);
+ if (*endptr == '-') {
+ endvalue = strtoull(endptr+1, &endptr, 10);
+ } else {
+ endvalue = value;
+ }
+
+ if (!(endvalue < MAX_CPUMASK_BITS)) {
+ endvalue = MAX_CPUMASK_BITS - 1;
+ fprintf(stderr, "A max of %d CPUs are supported in a guest\n",
+ MAX_CPUMASK_BITS);
+ }
+
+ bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+}
+
static void numa_node_add(const char *optarg)
{
char option[128];
char *endptr;
- unsigned long long value, endvalue;
int nodenr;
- value = endvalue = 0ULL;
-
if (nb_numa_nodes >= MAX_NODES) {
fprintf(stderr, "qemu: too many NUMA nodes\n");
exit(1);
@@ -1084,21 +1102,7 @@ static void numa_node_add(const char *optarg)
node_mem[nodenr] = sval;
}
if (get_param_value(option, 128, "cpus", optarg) != 0) {
- value = strtoull(option, &endptr, 10);
- if (*endptr == '-') {
- endvalue = strtoull(endptr+1, &endptr, 10);
- } else {
- endvalue = value;
- }
-
- if (!(endvalue < MAX_CPUMASK_BITS)) {
- endvalue = MAX_CPUMASK_BITS - 1;
- fprintf(stderr,
- "A max of %d CPUs are supported in a guest\n",
- MAX_CPUMASK_BITS);
- }
-
- bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+ numa_node_parse_cpus(nodenr, option);
}
nb_numa_nodes++;
}
--
1.7.11.7
12:15 p.m.
New subject: [libvirt] [PATCH 06/10] vl.c: handle invalid NUMA CPU ranges properly
Add checks for the following cases:
* Empty string: will be ignored and won't set any CPU bitmap,
parser won't abort.
* Missing end value after "-": parser will abort.
* Extra characters after a valid CPU range: parser will abort.
* "N-M" string where M < N: parser will abort.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 03a826e..19010fa 100644
--- a/vl.c
+++ b/vl.c
@@ -1057,13 +1057,30 @@ static void numa_node_parse_cpus(int nodenr, const char *cpus)
char *endptr;
unsigned long long value, endvalue;
+ /* Empty strings will be ignored, and not considered an error */
+ if (!*cpus) {
+ return;
+ }
+
value = strtoull(cpus, &endptr, 10);
if (*endptr == '-') {
- endvalue = strtoull(endptr+1, &endptr, 10);
+ endptr++;
+ if (!*endptr) {
+ goto error;
+ }
+ endvalue = strtoull(endptr, &endptr, 10);
} else {
endvalue = value;
}
+ if (*endptr != '\0') {
+ goto error;
+ }
+
+ if (endvalue < value) {
+ goto error;
+ }
+
if (!(endvalue < MAX_CPUMASK_BITS)) {
endvalue = MAX_CPUMASK_BITS - 1;
fprintf(stderr, "A max of %d CPUs are supported in a guest\n",
@@ -1071,6 +1088,11 @@ static void numa_node_parse_cpus(int nodenr, const char *cpus)
}
bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+ return;
+
+error:
+ fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
+ exit(1);
}
static void numa_node_add(const char *optarg)
--
1.7.11.7
3:32 p.m.
New subject: [libvirt] [PATCH 06/10] vl.c: handle invalid NUMA CPU ranges properly
On 01/11/2013 11:15 AM, Eduardo Habkost wrote:
Add checks for the following cases:
* Empty string: will be ignored and won't set any CPU bitmap,
parser won't abort.
* Missing end value after "-": parser will abort.
* Extra characters after a valid CPU range: parser will abort.
* "N-M" string where M < N: parser will abort.
value = strtoull(cpus, &endptr, 10);
if (*endptr == '-') {
- endvalue = strtoull(endptr+1, &endptr, 10);
+ endptr++;
+ if (!*endptr) {
+ goto error;
+ }
+ endvalue = strtoull(endptr, &endptr, 10);
} else {
endvalue = value;
}
Still missing a check for '-numa=-2' with no number on the left of '-',
as well as missing a check for overflow for -numa=999999999999999999999999
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:30 a.m.
New subject: [libvirt] [PATCH 06/10] vl.c: handle invalid NUMA CPU ranges properly
On Fri, Jan 11, 2013 at 02:32:48PM -0700, Eric Blake wrote:
On 01/11/2013 11:15 AM, Eduardo Habkost wrote:
> Add checks for the following cases:
>
> * Empty string: will be ignored and won't set any CPU bitmap,
> parser won't abort.
> * Missing end value after "-": parser will abort.
> * Extra characters after a valid CPU range: parser will abort.
> * "N-M" string where M < N: parser will abort.
> value = strtoull(cpus, &endptr, 10);
> if (*endptr == '-') {
> - endvalue = strtoull(endptr+1, &endptr, 10);
> + endptr++;
> + if (!*endptr) {
> + goto error;
> + }
> + endvalue = strtoull(endptr, &endptr, 10);
> } else {
> endvalue = value;
> }
Still missing a check for '-numa=-2' with no number on the left of '-',
as well as missing a check for overflow for -numa=999999999999999999999999
Thanks!
I will fix and submit v2 of this patch.
--
Eduardo
12:15 p.m.
New subject: [libvirt] [PATCH 07/10] vl.c: numa_add_node(): Validate nodeid before using it
Without this check, qemu-kvm will corrupt memory if a too-large nodeid
is provided in the command-line. e.g.:
-numa node,mem=...,cpus=...,nodeid=65
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/vl.c b/vl.c
index 19010fa..31175f6 100644
--- a/vl.c
+++ b/vl.c
@@ -1112,6 +1112,11 @@ static void numa_node_add(const char *optarg)
nodenr = strtoull(option, NULL, 10);
}
+ if (nodenr >= MAX_NODES) {
+ fprintf(stderr, "qemu: invalid NUMA nodeid: %d\n", nodenr);
+ exit(1);
+ }
+
if (get_param_value(option, 128, "mem", optarg) == 0) {
node_mem[nodenr] = 0;
} else {
--
1.7.11.7
12:15 p.m.
New subject: [libvirt] [PATCH 08/10] vl.c: Support multiple CPU ranges on -numa option
This allows "," or ";" to be used a separator between each CPU range.
Note that commas inside key=value command-line options have to be
escaped using ",,", so the command-line will look like:
-numa node,cpus=A,,B,,C,,D
or:
-numa node,cpus=A;B;C;D
Note that the following format, currently used by libvirt:
-numa nodes,cpus=A,B,C,D
will _not_ work yet, as "," is the option separator for the command-line
option parser, and it will require changing the -numa option parsing
code to handle "cpus" as a special case.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 31175f6..d7337c1 100644
--- a/vl.c
+++ b/vl.c
@@ -1052,7 +1052,7 @@ char *get_boot_devices_list(uint32_t *size)
return list;
}
-static void numa_node_parse_cpus(int nodenr, const char *cpus)
+static void numa_node_parse_cpu_range(int nodenr, const char *cpus)
{
char *endptr;
unsigned long long value, endvalue;
@@ -1095,6 +1095,18 @@ error:
exit(1);
}
+static void numa_node_parse_cpus(int nodenr, const char *option)
+{
+ char **parts;
+ int i;
+
+ parts = g_strsplit_set(option, ",;", 0);
+ for (i = 0; parts[i]; i++) {
+ numa_node_parse_cpu_range(nodenr, parts[i]);
+ }
+ g_strfreev(parts);
+}
+
static void numa_node_add(const char *optarg)
{
char option[128];
--
1.7.11.7
12:15 p.m.
New subject: [libvirt] [RFC 09/10] vl.c: Introduce QemuOpts-friendly "-numa-node" config option
This introduces both a "-numa-node" command-line option that is parsed
using a pure qemu_opts_parse() call, and a "numa-node" config file
section.
As the -numa option has a non-standard syntax, parse it manually and
translate it "numa-node" config options.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
qemu-config.c | 25 ++++++++++++
qemu-options.hx | 50 ++++++++++++++++++++++-
vl.c | 120 ++++++++++++++++++++++++++++++++++++++++++--------------
3 files changed, 164 insertions(+), 31 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 2188c3e..601dcda 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -647,6 +647,30 @@ static QemuOptsList qemu_object_opts = {
},
};
+static QemuOptsList qemu_numa_node_opts = {
+ .name = "numa-node",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_numa_node_opts.head),
+ .implied_opt_name = "type",
+ .desc = {
+ {
+ .name = "cpus",
+ .type = QEMU_OPT_STRING,
+ .help = "CPU range in the format N[-M]",
+ },
+ {
+ .name = "mem",
+ .type = QEMU_OPT_STRING,
+ .help = "RAM size for node, in MB",
+ },
+ {
+ .name = "nodeid",
+ .type = QEMU_OPT_NUMBER,
+ .help = "Node ID",
+ },
+ { /* end of list */ }
+ },
+};
+
static QemuOptsList *vm_config_groups[32] = {
&qemu_drive_opts,
&qemu_chardev_opts,
@@ -664,6 +688,7 @@ static QemuOptsList *vm_config_groups[32] = {
&qemu_sandbox_opts,
&qemu_add_fd_opts,
&qemu_object_opts,
+ &qemu_numa_node_opts,
NULL,
};
diff --git a/qemu-options.hx b/qemu-options.hx
index 9df0cde..a1c1a87 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -99,8 +99,54 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
STEXI
@item -numa @var{opts}
@findex -numa
-Simulate a multi node NUMA system. If mem and cpus are omitted, resources
-are split equally.
+Deprecated. See the @option{-numa-node} option.
+ETEXI
+
+DEF("numa-node", HAS_ARG, QEMU_OPTION_numa_node,
+ "-numa-node
[nodeid=@var{nodeid}][,mem=@var{size}][,cpus=@var{cpuranges}\n", QEMU_ARCH_ALL)
+STEXI
+@item -numa-node @var{opts}
+@findex -numa-node
+
+Define a NUMA node.
+
+@table @option
+@item nodeid=@var{nodeid}
+Index of the NUMA node, starting with 0. If omitted, NUMA nodes will be defined
+in the order they appear.
+
+@item mem=@var{size}
+
+Sets the RAM size of the NUMA node (in MB, of no unit is specified). If size
+of all nodes is omitted, memory is split equally.
+
+@item cpus=@var{cpus}
+
+@var{cpus} is a list of CPU indexes or CPU index ranges in the format:
+@samp{@var{start}[-@var{end}]}, separated by commas or semicolons.
+
+Note that commas used in values in key=value options have to be escaped, using
+@samp{,,}.
+
+The @option{cpus} option may appear multiple times, to assign multiple CPUs or
+CPU ranges to a node.
+
+If no node has CPU ranges assigned, CPUs will be split equally between the
+nodes.
+@end table
+
+Examples:
+
+@example
+-numa-node nodeid=1,mem=1024,cpus=0,,2,,4,,6 \
+-numa-node nodeid=0,mem=1024,cpus=1,,3,,5,,7
+@end example
+
+@example
+-numa-node mem=1024,cpus=0-3,,8-11 \
+-numa-node mem=1024,cpus=4-7,cpus=12-15
+@end example
+
ETEXI
DEF("fda", HAS_ARG, QEMU_OPTION_fda,
diff --git a/vl.c b/vl.c
index d7337c1..14bf9b6 100644
--- a/vl.c
+++ b/vl.c
@@ -1052,7 +1052,7 @@ char *get_boot_devices_list(uint32_t *size)
return list;
}
-static void numa_node_parse_cpu_range(int nodenr, const char *cpus)
+static int numa_node_parse_cpu_range(int nodenr, const char *cpus)
{
char *endptr;
unsigned long long value, endvalue;
@@ -1088,62 +1088,113 @@ static void numa_node_parse_cpu_range(int nodenr, const char
*cpus)
}
bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
- return;
+ return 0;
error:
fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
- exit(1);
+ return -1;
}
-static void numa_node_parse_cpus(int nodenr, const char *option)
+static int numa_node_parse_cpus(int nodenr, const char *option)
{
char **parts;
- int i;
+ int i, r = 0;
parts = g_strsplit_set(option, ",;", 0);
for (i = 0; parts[i]; i++) {
- numa_node_parse_cpu_range(nodenr, parts[i]);
+ r = numa_node_parse_cpu_range(nodenr, parts[i]);
+ if (r < 0) {
+ break;
+ }
}
g_strfreev(parts);
+ return r;
}
-static void numa_node_add(const char *optarg)
+static int parse_numa_node_opt(const char *name, const char *value,
+ void *opaque)
{
- char option[128];
- char *endptr;
- int nodenr;
+ uint64_t nodenr = *(uint64_t *)opaque;
+ if (!strcmp(name, "cpus")) {
+ return numa_node_parse_cpus(nodenr, value);
+ }
+ return 0;
+}
+
+static int parse_numa_node(QemuOpts *opts, void *opaque)
+{
+ uint64_t nodenr;
+ const char *memstr;
+ int64_t mem;
if (nb_numa_nodes >= MAX_NODES) {
fprintf(stderr, "qemu: too many NUMA nodes\n");
- exit(1);
+ return -1;
+ }
+
+ nodenr = qemu_opt_get_number(opts, "nodeid", nb_numa_nodes);
+
+ if (nodenr >= MAX_NODES) {
+ fprintf(stderr, "qemu: invalid NUMA nodeid: %" PRIu64 "\n",
nodenr);
+ return -1;
}
- if (get_param_value(option, 128, "nodeid", optarg) == 0) {
- nodenr = nb_numa_nodes;
+ memstr = qemu_opt_get(opts, "mem");
+ if (memstr) {
+ char *endptr;
+ mem = strtosz(memstr, &endptr);
+ if (mem < 0 || *endptr) {
+ fprintf(stderr, "qemu: invalid numa mem size: %s\n", memstr);
+ return -1;
+ }
} else {
- nodenr = strtoull(option, NULL, 10);
+ mem = 0;
}
- if (nodenr >= MAX_NODES) {
- fprintf(stderr, "qemu: invalid NUMA nodeid: %d\n", nodenr);
+ node_mem[nodenr] = mem;
+
+ /* There may be multiple "cpus" options set */
+ if (qemu_opt_foreach(opts, parse_numa_node_opt, &nodenr, 1) < 0) {
+ return -1;
+ }
+
+ nb_numa_nodes++;
+
+ return 0;
+}
+
+/* Parse legacy -numa option
+ *
+ * The option will be translated to a numa-node config option.
+ */
+static void parse_legacy_numa_node(const char *optarg)
+{
+ char option[128];
+ char value[128];
+ const char *p = optarg;
+ Error *error = NULL;
+ QemuOpts *opts = qemu_opts_create(qemu_find_opts("numa-node"),
+ NULL, 0, &error);
+ if (error) {
+ error_report("%s", error_get_pretty(error));
exit(1);
}
- if (get_param_value(option, 128, "mem", optarg) == 0) {
- node_mem[nodenr] = 0;
- } else {
- int64_t sval;
- sval = strtosz(option, &endptr);
- if (sval < 0 || *endptr) {
- fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
+ while (*p) {
+ p = get_opt_name(option, 128, p, '=');
+ if (*p == '=') {
+ p++;
+ } else {
+ fprintf(stderr, "Invalid -numa option: %s\n", optarg);
exit(1);
}
- node_mem[nodenr] = sval;
- }
- if (get_param_value(option, 128, "cpus", optarg) != 0) {
- numa_node_parse_cpus(nodenr, option);
+
+ p = get_opt_value(value, 128, p);
+ if (*p == ',') {
+ p++;
+ }
+ qemu_opt_set(opts, option, value);
}
- nb_numa_nodes++;
}
static void numa_add(const char *optarg)
@@ -1155,7 +1206,7 @@ static void numa_add(const char *optarg)
optarg++;
}
if (!strcmp(option, "node")) {
- numa_node_add(optarg);
+ parse_legacy_numa_node(optarg);
} else {
fprintf(stderr, "Invalid -numa option: %s\n", option);
exit(1);
@@ -2812,6 +2863,12 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_numa:
numa_add(optarg);
break;
+ case QEMU_OPTION_numa_node:
+ opts = qemu_opts_parse(qemu_find_opts("numa-node"), optarg,
0);
+ if (!opts) {
+ exit(1);
+ }
+ break;
case QEMU_OPTION_display:
display_type = select_display(optarg);
break;
@@ -3856,6 +3913,11 @@ int main(int argc, char **argv, char **envp)
register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
+ if (qemu_opts_foreach(qemu_find_opts("numa-node"),
+ parse_numa_node, NULL, 1) < 0) {
+ exit(1);
+ }
+
if (nb_numa_nodes > 0) {
int i;
--
1.7.11.7
12:15 p.m.
New subject: [libvirt] [RFC 10/10] vl.c: Handle legacy "-numa node, cpus=A, B, C, D" format
As libvirt already uses this format and expects it to work, add a small
hack to the legacy -numa option parsing code to make the "cpus=A,B,C,D"
format work.
Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com>
---
vl.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/vl.c b/vl.c
index 14bf9b6..cf30d44 100644
--- a/vl.c
+++ b/vl.c
@@ -1194,6 +1194,17 @@ static void parse_legacy_numa_node(const char *optarg)
p++;
}
qemu_opt_set(opts, option, value);
+
+ /* special case for "cpus", as it can contain "," */
+ if (!strcmp(option, "cpus")) {
+ while (isdigit(*p)) {
+ p = get_opt_value(value, 128, p);
+ if (*p == ',') {
+ p++;
+ }
+ qemu_opt_set(opts, "cpus", value);
+ }
+ }
}
}
--
1.7.11.7
4309
days inactive
4333
days old
15 comments
3 participants
participants (3)
-
Anthony Liguori -
Eduardo Habkost -
Eric Blake