[libvirt] [PATCH 00/10] -numa option parsing fixes & improvements
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
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@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
Instead of silently ignoring them, abort in case an invalid -numa option is provided. Signed-off-by: Eduardo Habkost <ehabkost@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
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@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
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
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
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@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
This will make it easier to refactor that code later. Signed-off-by: Eduardo Habkost <ehabkost@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
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@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
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
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
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@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
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@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
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@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
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@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
participants (3)
-
Anthony Liguori -
Eduardo Habkost -
Eric Blake