* [Qemu-devel] [PATCH 1/6] vl.c: fix style issue
2014-02-17 15:49 [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Igor Mammedov
@ 2014-02-17 15:49 ` Igor Mammedov
2014-02-17 15:49 ` [Qemu-devel] [PATCH 2/6] NUMA: move numa related code to new file numa.c Igor Mammedov
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2014-02-17 15:49 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, andre.przywara, stefanha, mst, peter.maydell, armbru,
ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini, afaerber,
gaowanlong, rth
add missing curly brackets to 'if' statement
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
vl.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/vl.c b/vl.c
index 316de54..9e60e5c 100644
--- a/vl.c
+++ b/vl.c
@@ -4150,8 +4150,9 @@ int main(int argc, char **argv, char **envp)
* and distribute the available memory equally across all nodes
*/
for (i = 0; i < nb_numa_nodes; i++) {
- if (node_mem[i] != 0)
+ if (node_mem[i] != 0) {
break;
+ }
}
if (i == nb_numa_nodes) {
uint64_t usedmem = 0;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 2/6] NUMA: move numa related code to new file numa.c
2014-02-17 15:49 [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Igor Mammedov
2014-02-17 15:49 ` [Qemu-devel] [PATCH 1/6] vl.c: fix style issue Igor Mammedov
@ 2014-02-17 15:49 ` Igor Mammedov
2014-02-26 17:00 ` Eduardo Habkost
2014-02-17 15:49 ` [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size Igor Mammedov
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2014-02-17 15:49 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, andre.przywara, stefanha, mst, peter.maydell, armbru,
ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini, afaerber,
gaowanlong, rth
From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
Makefile.target | 2 +-
cpus.c | 14 ----
include/sysemu/cpus.h | 1 -
include/sysemu/sysemu.h | 3 +
numa.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++
vl.c | 140 +-----------------------------------
6 files changed, 188 insertions(+), 155 deletions(-)
create mode 100644 numa.c
diff --git a/Makefile.target b/Makefile.target
index af6ac7e..0197c17 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -109,7 +109,7 @@ endif #CONFIG_BSD_USER
#########################################################
# System emulator target
ifdef CONFIG_SOFTMMU
-obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o
+obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
obj-y += qtest.o
obj-y += hw/
obj-$(CONFIG_FDT) += device_tree.o
diff --git a/cpus.c b/cpus.c
index 945d85b..891d062 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1299,20 +1299,6 @@ static void tcg_exec_all(void)
exit_request = 0;
}
-void set_numa_modes(void)
-{
- CPUState *cpu;
- int i;
-
- CPU_FOREACH(cpu) {
- for (i = 0; i < nb_numa_nodes; i++) {
- if (test_bit(cpu->cpu_index, node_cpumask[i])) {
- cpu->numa_node = i;
- }
- }
- }
-}
-
void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
{
/* XXX: implement xxx_cpu_list for targets that still miss it */
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 6502488..4f79081 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -23,7 +23,6 @@ extern int smp_threads;
#define smp_threads 1
#endif
-void set_numa_modes(void);
void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg);
#endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 495dae8..2509649 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -136,6 +136,9 @@ extern QEMUClockType rtc_clock;
extern int nb_numa_nodes;
extern uint64_t node_mem[MAX_NODES];
extern unsigned long *node_cpumask[MAX_NODES];
+void numa_add(const char *optarg);
+void set_numa_nodes(void);
+void set_numa_modes(void);
#define MAX_OPTION_ROMS 16
typedef struct QEMUOptionRom {
diff --git a/numa.c b/numa.c
new file mode 100644
index 0000000..7845036
--- /dev/null
+++ b/numa.c
@@ -0,0 +1,183 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2013 Fujitsu Ltd.
+ * Author: Wanlong Gao <gaowanlong@cn.fujitsu.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "sysemu/sysemu.h"
+
+static void numa_node_parse_cpus(int nodenr, const char *cpus)
+{
+ char *endptr;
+ unsigned long long value, endvalue;
+
+ /* Empty CPU range strings will be considered valid, they will simply
+ * not set any bit in the CPU bitmap.
+ */
+ if (!*cpus) {
+ return;
+ }
+
+ if (parse_uint(cpus, &value, &endptr, 10) < 0) {
+ goto error;
+ }
+ if (*endptr == '-') {
+ if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
+ goto error;
+ }
+ } else if (*endptr == '\0') {
+ endvalue = value;
+ } else {
+ goto error;
+ }
+
+ if (endvalue >= MAX_CPUMASK_BITS) {
+ endvalue = MAX_CPUMASK_BITS - 1;
+ fprintf(stderr,
+ "qemu: NUMA: A max of %d VCPUs are supported\n",
+ MAX_CPUMASK_BITS);
+ }
+
+ if (endvalue < value) {
+ goto error;
+ }
+
+ bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+ return;
+
+error:
+ fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
+ exit(1);
+}
+
+void numa_add(const char *optarg)
+{
+ char option[128];
+ char *endptr;
+ unsigned long long nodenr;
+
+ optarg = get_opt_name(option, 128, optarg, ',');
+ if (*optarg == ',') {
+ optarg++;
+ }
+ if (!strcmp(option, "node")) {
+
+ 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 {
+ if (parse_uint_full(option, &nodenr, 10) < 0) {
+ fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
+ exit(1);
+ }
+ }
+
+ if (nodenr >= MAX_NODES) {
+ fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
+ 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);
+ exit(1);
+ }
+ node_mem[nodenr] = sval;
+ }
+ if (get_param_value(option, 128, "cpus", optarg) != 0) {
+ numa_node_parse_cpus(nodenr, option);
+ }
+ nb_numa_nodes++;
+ } else {
+ fprintf(stderr, "Invalid -numa option: %s\n", option);
+ exit(1);
+ }
+}
+
+void set_numa_nodes(void)
+{
+ if (nb_numa_nodes > 0) {
+ int i;
+
+ if (nb_numa_nodes > MAX_NODES) {
+ nb_numa_nodes = MAX_NODES;
+ }
+
+ /* If no memory size if given for any node, assume the default case
+ * and distribute the available memory equally across all nodes
+ */
+ for (i = 0; i < nb_numa_nodes; i++) {
+ if (node_mem[i] != 0) {
+ break;
+ }
+ }
+ if (i == nb_numa_nodes) {
+ uint64_t usedmem = 0;
+
+ /* On Linux, the each node's border has to be 8MB aligned,
+ * the final node gets the rest.
+ */
+ for (i = 0; i < nb_numa_nodes - 1; i++) {
+ node_mem[i] = (ram_size / nb_numa_nodes) & ~((1 << 23UL) - 1);
+ usedmem += node_mem[i];
+ }
+ node_mem[i] = ram_size - usedmem;
+ }
+
+ for (i = 0; i < nb_numa_nodes; i++) {
+ if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) {
+ break;
+ }
+ }
+ /* assigning the VCPUs round-robin is easier to implement, guest OSes
+ * must cope with this anyway, because there are BIOSes out there in
+ * real machines which also use this scheme.
+ */
+ if (i == nb_numa_nodes) {
+ for (i = 0; i < max_cpus; i++) {
+ set_bit(i, node_cpumask[i % nb_numa_nodes]);
+ }
+ }
+ }
+}
+
+void set_numa_modes(void)
+{
+ CPUState *cpu;
+ int i;
+
+ CPU_FOREACH(cpu) {
+ for (i = 0; i < nb_numa_nodes; i++) {
+ if (test_bit(cpu->cpu_index, node_cpumask[i])) {
+ cpu->numa_node = i;
+ }
+ }
+ }
+}
diff --git a/vl.c b/vl.c
index 9e60e5c..0adac0c 100644
--- a/vl.c
+++ b/vl.c
@@ -1211,102 +1211,6 @@ char *get_boot_devices_list(size_t *size)
return list;
}
-static void numa_node_parse_cpus(int nodenr, const char *cpus)
-{
- char *endptr;
- unsigned long long value, endvalue;
-
- /* Empty CPU range strings will be considered valid, they will simply
- * not set any bit in the CPU bitmap.
- */
- if (!*cpus) {
- return;
- }
-
- if (parse_uint(cpus, &value, &endptr, 10) < 0) {
- goto error;
- }
- if (*endptr == '-') {
- if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
- goto error;
- }
- } else if (*endptr == '\0') {
- endvalue = value;
- } else {
- goto error;
- }
-
- if (endvalue >= MAX_CPUMASK_BITS) {
- endvalue = MAX_CPUMASK_BITS - 1;
- fprintf(stderr,
- "qemu: NUMA: A max of %d VCPUs are supported\n",
- MAX_CPUMASK_BITS);
- }
-
- if (endvalue < value) {
- goto error;
- }
-
- 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_add(const char *optarg)
-{
- char option[128];
- char *endptr;
- unsigned long long nodenr;
-
- optarg = get_opt_name(option, 128, optarg, ',');
- if (*optarg == ',') {
- optarg++;
- }
- if (!strcmp(option, "node")) {
-
- 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 {
- if (parse_uint_full(option, &nodenr, 10) < 0) {
- fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
- exit(1);
- }
- }
-
- if (nodenr >= MAX_NODES) {
- fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
- 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);
- exit(1);
- }
- node_mem[nodenr] = sval;
- }
- if (get_param_value(option, 128, "cpus", optarg) != 0) {
- numa_node_parse_cpus(nodenr, option);
- }
- nb_numa_nodes++;
- } else {
- fprintf(stderr, "Invalid -numa option: %s\n", option);
- exit(1);
- }
-}
-
static QemuOptsList qemu_smp_opts = {
.name = "smp-opts",
.implied_opt_name = "cpus",
@@ -4139,49 +4043,7 @@ int main(int argc, char **argv, char **envp)
register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
- if (nb_numa_nodes > 0) {
- int i;
-
- if (nb_numa_nodes > MAX_NODES) {
- nb_numa_nodes = MAX_NODES;
- }
-
- /* If no memory size if given for any node, assume the default case
- * and distribute the available memory equally across all nodes
- */
- for (i = 0; i < nb_numa_nodes; i++) {
- if (node_mem[i] != 0) {
- break;
- }
- }
- if (i == nb_numa_nodes) {
- uint64_t usedmem = 0;
-
- /* On Linux, the each node's border has to be 8MB aligned,
- * the final node gets the rest.
- */
- for (i = 0; i < nb_numa_nodes - 1; i++) {
- node_mem[i] = (ram_size / nb_numa_nodes) & ~((1 << 23UL) - 1);
- usedmem += node_mem[i];
- }
- node_mem[i] = ram_size - usedmem;
- }
-
- for (i = 0; i < nb_numa_nodes; i++) {
- if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) {
- break;
- }
- }
- /* assigning the VCPUs round-robin is easier to implement, guest OSes
- * must cope with this anyway, because there are BIOSes out there in
- * real machines which also use this scheme.
- */
- if (i == nb_numa_nodes) {
- for (i = 0; i < max_cpus; i++) {
- set_bit(i, node_cpumask[i % nb_numa_nodes]);
- }
- }
- }
+ set_numa_nodes();
if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) {
exit(1);
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 2/6] NUMA: move numa related code to new file numa.c
2014-02-17 15:49 ` [Qemu-devel] [PATCH 2/6] NUMA: move numa related code to new file numa.c Igor Mammedov
@ 2014-02-26 17:00 ` Eduardo Habkost
0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2014-02-26 17:00 UTC (permalink / raw)
To: Igor Mammedov, gaowanlong
Cc: peter.maydell, aliguori, andre.przywara, armbru, mst, qemu-devel,
agraf, qemu-ppc, stefanha, pbonzini, lcapitulino, afaerber, rth
On Mon, Feb 17, 2014 at 04:49:43PM +0100, Igor Mammedov wrote:
[...]
> diff --git a/numa.c b/numa.c
> new file mode 100644
> index 0000000..7845036
> --- /dev/null
> +++ b/numa.c
> @@ -0,0 +1,183 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2013 Fujitsu Ltd.
> + * Author: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> + *
IANAL, but don't you need to keep the copyright assignment from the file
from which you are copying the code? There are only 2 lines in this file
that were not copied from existing code at vl.c.
(The fact that the only copyright notice is (c) 2003-2008 Fabrice
Bellard (and -numa was introduced in 2009) may make proper copyright
assignment difficult, though.)
Copyright questions aside, the code movement looks good, so:
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
[...]
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size
2014-02-17 15:49 [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Igor Mammedov
2014-02-17 15:49 ` [Qemu-devel] [PATCH 1/6] vl.c: fix style issue Igor Mammedov
2014-02-17 15:49 ` [Qemu-devel] [PATCH 2/6] NUMA: move numa related code to new file numa.c Igor Mammedov
@ 2014-02-17 15:49 ` Igor Mammedov
2014-02-26 17:15 ` Eduardo Habkost
2014-02-17 15:49 ` [Qemu-devel] [PATCH 4/6] NUMA: Add numa_info structure to contain numa nodes info Igor Mammedov
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2014-02-17 15:49 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, andre.przywara, stefanha, mst, peter.maydell, armbru,
ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini, afaerber,
gaowanlong, rth
From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
If the total number of the assigned numa nodes memory is not
equal to the assigned ram size, it will write the wrong data
to ACPI table, then the guest will ignore the wrong ACPI table
and recognize all memory to one node. It's buggy, we should
check it to ensure that we write the right data to ACPI table.
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
numa.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/numa.c b/numa.c
index 7845036..d12a4f2 100644
--- a/numa.c
+++ b/numa.c
@@ -151,6 +151,16 @@ void set_numa_nodes(void)
node_mem[i] = ram_size - usedmem;
}
+ uint64_t numa_total = 0;
+ for (i = 0; i < nb_numa_nodes; i++) {
+ numa_total += node_mem[i];
+ }
+ if (numa_total != ram_size) {
+ fprintf(stderr, "qemu: numa nodes total memory size "
+ "should equal ram_size\n");
+ exit(1);
+ }
+
for (i = 0; i < nb_numa_nodes; i++) {
if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) {
break;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size
2014-02-17 15:49 ` [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size Igor Mammedov
@ 2014-02-26 17:15 ` Eduardo Habkost
2014-02-26 17:28 ` Paolo Bonzini
0 siblings, 1 reply; 17+ messages in thread
From: Eduardo Habkost @ 2014-02-26 17:15 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, aliguori, andre.przywara, armbru, mst, qemu-devel,
agraf, qemu-ppc, stefanha, pbonzini, lcapitulino, afaerber,
gaowanlong, rth
On Mon, Feb 17, 2014 at 04:49:44PM +0100, Igor Mammedov wrote:
> From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>
> If the total number of the assigned numa nodes memory is not
> equal to the assigned ram size, it will write the wrong data
> to ACPI table, then the guest will ignore the wrong ACPI table
> and recognize all memory to one node. It's buggy, we should
> check it to ensure that we write the right data to ACPI table.
>
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> numa.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/numa.c b/numa.c
> index 7845036..d12a4f2 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -151,6 +151,16 @@ void set_numa_nodes(void)
> node_mem[i] = ram_size - usedmem;
> }
>
> + uint64_t numa_total = 0;
I was going to point out that variable declarations in the middle of
blocks goes against coding style (at least I was told so), but my patch
to amend CODING_STYLE to document it was ignored for 2 weeks, already.
So, I am not sure we really have that rule.
(Personally I am not against declaring variables in the middle of
blocks, I think it makes the code more readable, and it is perfectly
valid C99 code.)
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> + for (i = 0; i < nb_numa_nodes; i++) {
> + numa_total += node_mem[i];
> + }
> + if (numa_total != ram_size) {
> + fprintf(stderr, "qemu: numa nodes total memory size "
> + "should equal ram_size\n");
> + exit(1);
> + }
> +
> for (i = 0; i < nb_numa_nodes; i++) {
> if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) {
> break;
> --
> 1.7.1
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size
2014-02-26 17:15 ` Eduardo Habkost
@ 2014-02-26 17:28 ` Paolo Bonzini
0 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2014-02-26 17:28 UTC (permalink / raw)
To: Eduardo Habkost, Igor Mammedov
Cc: peter.maydell, aliguori, andre.przywara, armbru, mst, qemu-devel,
agraf, qemu-ppc, stefanha, lcapitulino, afaerber, gaowanlong, rth
Il 26/02/2014 18:15, Eduardo Habkost ha scritto:
> On Mon, Feb 17, 2014 at 04:49:44PM +0100, Igor Mammedov wrote:
>> From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>>
>> If the total number of the assigned numa nodes memory is not
>> equal to the assigned ram size, it will write the wrong data
>> to ACPI table, then the guest will ignore the wrong ACPI table
>> and recognize all memory to one node. It's buggy, we should
>> check it to ensure that we write the right data to ACPI table.
>>
>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> numa.c | 10 ++++++++++
>> 1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/numa.c b/numa.c
>> index 7845036..d12a4f2 100644
>> --- a/numa.c
>> +++ b/numa.c
>> @@ -151,6 +151,16 @@ void set_numa_nodes(void)
>> node_mem[i] = ram_size - usedmem;
>> }
>>
>> + uint64_t numa_total = 0;
>
> I was going to point out that variable declarations in the middle of
> blocks goes against coding style (at least I was told so), but my patch
> to amend CODING_STYLE to document it was ignored for 2 weeks, already.
> So, I am not sure we really have that rule.
>
> (Personally I am not against declaring variables in the middle of
> blocks, I think it makes the code more readable, and it is perfectly
> valid C99 code.)
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
I agree, but I fixed it already.
Paolo
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 4/6] NUMA: Add numa_info structure to contain numa nodes info
2014-02-17 15:49 [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Igor Mammedov
` (2 preceding siblings ...)
2014-02-17 15:49 ` [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size Igor Mammedov
@ 2014-02-17 15:49 ` Igor Mammedov
2014-02-26 16:42 ` Michael S. Tsirkin
2014-02-17 15:49 ` [Qemu-devel] [PATCH 5/6] NUMA: convert -numa option to use OptsVisitor Igor Mammedov
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2014-02-17 15:49 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, andre.przywara, stefanha, mst, peter.maydell, armbru,
ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini, afaerber,
gaowanlong, rth
From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Add the numa_info structure to contain the numa nodes memory,
VCPUs information and the future added numa nodes host memory
policies.
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Andre Przywara <andre.przywara@amd.com>
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/i386/pc.c | 14 +++++++++-----
hw/ppc/spapr.c | 11 ++++++-----
include/sysemu/sysemu.h | 8 ++++++--
monitor.c | 2 +-
numa.c | 23 ++++++++++++-----------
vl.c | 7 +++----
6 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e715a33..42182f9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -674,14 +674,14 @@ static FWCfgState *bochs_bios_init(void)
unsigned int apic_id = x86_cpu_apic_id_from_index(i);
assert(apic_id < apic_id_limit);
for (j = 0; j < nb_numa_nodes; j++) {
- if (test_bit(i, node_cpumask[j])) {
+ if (test_bit(i, numa_info[j].node_cpu)) {
numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
break;
}
}
}
for (i = 0; i < nb_numa_nodes; i++) {
- numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]);
+ numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem);
}
fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
(1 + apic_id_limit + nb_numa_nodes) *
@@ -1077,8 +1077,12 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
guest_info->apic_id_limit = pc_apic_id_limit(max_cpus);
guest_info->apic_xrupt_override = kvm_allows_irq0_override();
guest_info->numa_nodes = nb_numa_nodes;
- guest_info->node_mem = g_memdup(node_mem, guest_info->numa_nodes *
- sizeof *guest_info->node_mem);
+ guest_info->node_mem = g_malloc0(guest_info->numa_nodes *
+ sizeof *guest_info->node_mem);
+ for (i = 0; i < nb_numa_nodes; i++) {
+ guest_info->node_mem[i] = numa_info[i].node_mem;
+ }
+
guest_info->node_cpu = g_malloc0(guest_info->apic_id_limit *
sizeof *guest_info->node_cpu);
@@ -1086,7 +1090,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
unsigned int apic_id = x86_cpu_apic_id_from_index(i);
assert(apic_id < guest_info->apic_id_limit);
for (j = 0; j < nb_numa_nodes; j++) {
- if (test_bit(i, node_cpumask[j])) {
+ if (test_bit(i, numa_info[j].node_cpu)) {
guest_info->node_cpu[apic_id] = j;
break;
}
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 93d02c1..6fd2d95 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -531,8 +531,8 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
int i, off;
/* memory node(s) */
- if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
- node0_size = node_mem[0];
+ if (nb_numa_nodes > 1 && numa_info[0].node_mem < ram_size) {
+ node0_size = numa_info[0].node_mem;
} else {
node0_size = ram_size;
}
@@ -570,7 +570,7 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
if (mem_start >= ram_size) {
node_size = 0;
} else {
- node_size = node_mem[i];
+ node_size = numa_info[i].node_mem;
if (node_size > ram_size - mem_start) {
node_size = ram_size - mem_start;
}
@@ -697,7 +697,8 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
/* Update the RMA size if necessary */
if (spapr->vrma_adjust) {
- hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
+ hwaddr node0_size = (nb_numa_nodes > 1) ? numa_info[0].node_mem :
+ ram_size;
spapr->rma_size = kvmppc_rma_size(node0_size, spapr->htab_shift);
}
}
@@ -1115,7 +1116,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
MemoryRegion *sysmem = get_system_memory();
MemoryRegion *ram = g_new(MemoryRegion, 1);
hwaddr rma_alloc_size;
- hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
+ hwaddr node0_size = (nb_numa_nodes > 1) ? numa_info[0].node_mem : ram_size;
uint32_t initrd_base = 0;
long kernel_size = 0, initrd_size = 0;
long load_limit, rtas_limit, fw_size;
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 2509649..d873b42 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -9,6 +9,7 @@
#include "qapi-types.h"
#include "qemu/notify.h"
#include "qemu/main-loop.h"
+#include "qemu/bitmap.h"
/* vl.c */
@@ -134,8 +135,11 @@ extern QEMUClockType rtc_clock;
#define MAX_NODES 64
#define MAX_CPUMASK_BITS 255
extern int nb_numa_nodes;
-extern uint64_t node_mem[MAX_NODES];
-extern unsigned long *node_cpumask[MAX_NODES];
+typedef struct node_info {
+ uint64_t node_mem;
+ DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
+} NodeInfo;
+extern NodeInfo numa_info[MAX_NODES];
void numa_add(const char *optarg);
void set_numa_nodes(void);
void set_numa_modes(void);
diff --git a/monitor.c b/monitor.c
index 690c152..0284735 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2004,7 +2004,7 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
}
monitor_printf(mon, "\n");
monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
- node_mem[i] >> 20);
+ numa_info[i].node_mem >> 20);
}
}
diff --git a/numa.c b/numa.c
index d12a4f2..c3eca78 100644
--- a/numa.c
+++ b/numa.c
@@ -61,7 +61,7 @@ static void numa_node_parse_cpus(int nodenr, const char *cpus)
goto error;
}
- bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
+ bitmap_set(numa_info[nodenr].node_cpu, value, endvalue - value + 1);
return;
error:
@@ -101,7 +101,7 @@ void numa_add(const char *optarg)
}
if (get_param_value(option, 128, "mem", optarg) == 0) {
- node_mem[nodenr] = 0;
+ numa_info[nodenr].node_mem = 0;
} else {
int64_t sval;
sval = strtosz(option, &endptr);
@@ -109,7 +109,7 @@ void numa_add(const char *optarg)
fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
exit(1);
}
- node_mem[nodenr] = sval;
+ numa_info[nodenr].node_mem = sval;
}
if (get_param_value(option, 128, "cpus", optarg) != 0) {
numa_node_parse_cpus(nodenr, option);
@@ -134,7 +134,7 @@ void set_numa_nodes(void)
* and distribute the available memory equally across all nodes
*/
for (i = 0; i < nb_numa_nodes; i++) {
- if (node_mem[i] != 0) {
+ if (numa_info[i].node_mem != 0) {
break;
}
}
@@ -145,15 +145,16 @@ void set_numa_nodes(void)
* the final node gets the rest.
*/
for (i = 0; i < nb_numa_nodes - 1; i++) {
- node_mem[i] = (ram_size / nb_numa_nodes) & ~((1 << 23UL) - 1);
- usedmem += node_mem[i];
+ numa_info[i].node_mem = (ram_size / nb_numa_nodes) &
+ ~((1 << 23UL) - 1);
+ usedmem += numa_info[i].node_mem;
}
- node_mem[i] = ram_size - usedmem;
+ numa_info[i].node_mem = ram_size - usedmem;
}
uint64_t numa_total = 0;
for (i = 0; i < nb_numa_nodes; i++) {
- numa_total += node_mem[i];
+ numa_total += numa_info[i].node_mem;
}
if (numa_total != ram_size) {
fprintf(stderr, "qemu: numa nodes total memory size "
@@ -162,7 +163,7 @@ void set_numa_nodes(void)
}
for (i = 0; i < nb_numa_nodes; i++) {
- if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) {
+ if (!bitmap_empty(numa_info[i].node_cpu, MAX_CPUMASK_BITS)) {
break;
}
}
@@ -172,7 +173,7 @@ void set_numa_nodes(void)
*/
if (i == nb_numa_nodes) {
for (i = 0; i < max_cpus; i++) {
- set_bit(i, node_cpumask[i % nb_numa_nodes]);
+ set_bit(i, numa_info[i % nb_numa_nodes].node_cpu);
}
}
}
@@ -185,7 +186,7 @@ void set_numa_modes(void)
CPU_FOREACH(cpu) {
for (i = 0; i < nb_numa_nodes; i++) {
- if (test_bit(cpu->cpu_index, node_cpumask[i])) {
+ if (test_bit(cpu->cpu_index, numa_info[i].node_cpu)) {
cpu->numa_node = i;
}
}
diff --git a/vl.c b/vl.c
index 0adac0c..915f8b7 100644
--- a/vl.c
+++ b/vl.c
@@ -196,8 +196,7 @@ static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
QTAILQ_HEAD_INITIALIZER(fw_boot_order);
int nb_numa_nodes;
-uint64_t node_mem[MAX_NODES];
-unsigned long *node_cpumask[MAX_NODES];
+NodeInfo numa_info[MAX_NODES];
uint8_t qemu_uuid[16];
bool qemu_uuid_set;
@@ -2787,8 +2786,8 @@ int main(int argc, char **argv, char **envp)
translation = BIOS_ATA_TRANSLATION_AUTO;
for (i = 0; i < MAX_NODES; i++) {
- node_mem[i] = 0;
- node_cpumask[i] = bitmap_new(MAX_CPUMASK_BITS);
+ numa_info[i].node_mem = 0;
+ bitmap_zero(numa_info[i].node_cpu, MAX_CPUMASK_BITS);
}
nb_numa_nodes = 0;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 4/6] NUMA: Add numa_info structure to contain numa nodes info
2014-02-17 15:49 ` [Qemu-devel] [PATCH 4/6] NUMA: Add numa_info structure to contain numa nodes info Igor Mammedov
@ 2014-02-26 16:42 ` Michael S. Tsirkin
0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2014-02-26 16:42 UTC (permalink / raw)
To: Igor Mammedov
Cc: agraf, andre.przywara, stefanha, peter.maydell, qemu-devel,
armbru, ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini,
afaerber, gaowanlong, rth
On Mon, Feb 17, 2014 at 04:49:45PM +0100, Igor Mammedov wrote:
> From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>
> Add the numa_info structure to contain the numa nodes memory,
> VCPUs information and the future added numa nodes host memory
> policies.
>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Andre Przywara <andre.przywara@amd.com>
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
For pc.c changes here:
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/i386/pc.c | 14 +++++++++-----
> hw/ppc/spapr.c | 11 ++++++-----
> include/sysemu/sysemu.h | 8 ++++++--
> monitor.c | 2 +-
> numa.c | 23 ++++++++++++-----------
> vl.c | 7 +++----
> 6 files changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e715a33..42182f9 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -674,14 +674,14 @@ static FWCfgState *bochs_bios_init(void)
> unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> assert(apic_id < apic_id_limit);
> for (j = 0; j < nb_numa_nodes; j++) {
> - if (test_bit(i, node_cpumask[j])) {
> + if (test_bit(i, numa_info[j].node_cpu)) {
> numa_fw_cfg[apic_id + 1] = cpu_to_le64(j);
> break;
> }
> }
> }
> for (i = 0; i < nb_numa_nodes; i++) {
> - numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]);
> + numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem);
> }
> fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
> (1 + apic_id_limit + nb_numa_nodes) *
> @@ -1077,8 +1077,12 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
> guest_info->apic_id_limit = pc_apic_id_limit(max_cpus);
> guest_info->apic_xrupt_override = kvm_allows_irq0_override();
> guest_info->numa_nodes = nb_numa_nodes;
> - guest_info->node_mem = g_memdup(node_mem, guest_info->numa_nodes *
> - sizeof *guest_info->node_mem);
> + guest_info->node_mem = g_malloc0(guest_info->numa_nodes *
> + sizeof *guest_info->node_mem);
> + for (i = 0; i < nb_numa_nodes; i++) {
> + guest_info->node_mem[i] = numa_info[i].node_mem;
> + }
> +
> guest_info->node_cpu = g_malloc0(guest_info->apic_id_limit *
> sizeof *guest_info->node_cpu);
>
> @@ -1086,7 +1090,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
> unsigned int apic_id = x86_cpu_apic_id_from_index(i);
> assert(apic_id < guest_info->apic_id_limit);
> for (j = 0; j < nb_numa_nodes; j++) {
> - if (test_bit(i, node_cpumask[j])) {
> + if (test_bit(i, numa_info[j].node_cpu)) {
> guest_info->node_cpu[apic_id] = j;
> break;
> }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 93d02c1..6fd2d95 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -531,8 +531,8 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
> int i, off;
>
> /* memory node(s) */
> - if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
> - node0_size = node_mem[0];
> + if (nb_numa_nodes > 1 && numa_info[0].node_mem < ram_size) {
> + node0_size = numa_info[0].node_mem;
> } else {
> node0_size = ram_size;
> }
> @@ -570,7 +570,7 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
> if (mem_start >= ram_size) {
> node_size = 0;
> } else {
> - node_size = node_mem[i];
> + node_size = numa_info[i].node_mem;
> if (node_size > ram_size - mem_start) {
> node_size = ram_size - mem_start;
> }
> @@ -697,7 +697,8 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
>
> /* Update the RMA size if necessary */
> if (spapr->vrma_adjust) {
> - hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> + hwaddr node0_size = (nb_numa_nodes > 1) ? numa_info[0].node_mem :
> + ram_size;
> spapr->rma_size = kvmppc_rma_size(node0_size, spapr->htab_shift);
> }
> }
> @@ -1115,7 +1116,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> hwaddr rma_alloc_size;
> - hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> + hwaddr node0_size = (nb_numa_nodes > 1) ? numa_info[0].node_mem : ram_size;
> uint32_t initrd_base = 0;
> long kernel_size = 0, initrd_size = 0;
> long load_limit, rtas_limit, fw_size;
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 2509649..d873b42 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -9,6 +9,7 @@
> #include "qapi-types.h"
> #include "qemu/notify.h"
> #include "qemu/main-loop.h"
> +#include "qemu/bitmap.h"
>
> /* vl.c */
>
> @@ -134,8 +135,11 @@ extern QEMUClockType rtc_clock;
> #define MAX_NODES 64
> #define MAX_CPUMASK_BITS 255
> extern int nb_numa_nodes;
> -extern uint64_t node_mem[MAX_NODES];
> -extern unsigned long *node_cpumask[MAX_NODES];
> +typedef struct node_info {
> + uint64_t node_mem;
> + DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
> +} NodeInfo;
> +extern NodeInfo numa_info[MAX_NODES];
> void numa_add(const char *optarg);
> void set_numa_nodes(void);
> void set_numa_modes(void);
> diff --git a/monitor.c b/monitor.c
> index 690c152..0284735 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2004,7 +2004,7 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
> }
> monitor_printf(mon, "\n");
> monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
> - node_mem[i] >> 20);
> + numa_info[i].node_mem >> 20);
> }
> }
>
> diff --git a/numa.c b/numa.c
> index d12a4f2..c3eca78 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -61,7 +61,7 @@ static void numa_node_parse_cpus(int nodenr, const char *cpus)
> goto error;
> }
>
> - bitmap_set(node_cpumask[nodenr], value, endvalue-value+1);
> + bitmap_set(numa_info[nodenr].node_cpu, value, endvalue - value + 1);
> return;
>
> error:
> @@ -101,7 +101,7 @@ void numa_add(const char *optarg)
> }
>
> if (get_param_value(option, 128, "mem", optarg) == 0) {
> - node_mem[nodenr] = 0;
> + numa_info[nodenr].node_mem = 0;
> } else {
> int64_t sval;
> sval = strtosz(option, &endptr);
> @@ -109,7 +109,7 @@ void numa_add(const char *optarg)
> fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
> exit(1);
> }
> - node_mem[nodenr] = sval;
> + numa_info[nodenr].node_mem = sval;
> }
> if (get_param_value(option, 128, "cpus", optarg) != 0) {
> numa_node_parse_cpus(nodenr, option);
> @@ -134,7 +134,7 @@ void set_numa_nodes(void)
> * and distribute the available memory equally across all nodes
> */
> for (i = 0; i < nb_numa_nodes; i++) {
> - if (node_mem[i] != 0) {
> + if (numa_info[i].node_mem != 0) {
> break;
> }
> }
> @@ -145,15 +145,16 @@ void set_numa_nodes(void)
> * the final node gets the rest.
> */
> for (i = 0; i < nb_numa_nodes - 1; i++) {
> - node_mem[i] = (ram_size / nb_numa_nodes) & ~((1 << 23UL) - 1);
> - usedmem += node_mem[i];
> + numa_info[i].node_mem = (ram_size / nb_numa_nodes) &
> + ~((1 << 23UL) - 1);
> + usedmem += numa_info[i].node_mem;
> }
> - node_mem[i] = ram_size - usedmem;
> + numa_info[i].node_mem = ram_size - usedmem;
> }
>
> uint64_t numa_total = 0;
> for (i = 0; i < nb_numa_nodes; i++) {
> - numa_total += node_mem[i];
> + numa_total += numa_info[i].node_mem;
> }
> if (numa_total != ram_size) {
> fprintf(stderr, "qemu: numa nodes total memory size "
> @@ -162,7 +163,7 @@ void set_numa_nodes(void)
> }
>
> for (i = 0; i < nb_numa_nodes; i++) {
> - if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) {
> + if (!bitmap_empty(numa_info[i].node_cpu, MAX_CPUMASK_BITS)) {
> break;
> }
> }
> @@ -172,7 +173,7 @@ void set_numa_nodes(void)
> */
> if (i == nb_numa_nodes) {
> for (i = 0; i < max_cpus; i++) {
> - set_bit(i, node_cpumask[i % nb_numa_nodes]);
> + set_bit(i, numa_info[i % nb_numa_nodes].node_cpu);
> }
> }
> }
> @@ -185,7 +186,7 @@ void set_numa_modes(void)
>
> CPU_FOREACH(cpu) {
> for (i = 0; i < nb_numa_nodes; i++) {
> - if (test_bit(cpu->cpu_index, node_cpumask[i])) {
> + if (test_bit(cpu->cpu_index, numa_info[i].node_cpu)) {
> cpu->numa_node = i;
> }
> }
> diff --git a/vl.c b/vl.c
> index 0adac0c..915f8b7 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -196,8 +196,7 @@ static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
> QTAILQ_HEAD_INITIALIZER(fw_boot_order);
>
> int nb_numa_nodes;
> -uint64_t node_mem[MAX_NODES];
> -unsigned long *node_cpumask[MAX_NODES];
> +NodeInfo numa_info[MAX_NODES];
>
> uint8_t qemu_uuid[16];
> bool qemu_uuid_set;
> @@ -2787,8 +2786,8 @@ int main(int argc, char **argv, char **envp)
> translation = BIOS_ATA_TRANSLATION_AUTO;
>
> for (i = 0; i < MAX_NODES; i++) {
> - node_mem[i] = 0;
> - node_cpumask[i] = bitmap_new(MAX_CPUMASK_BITS);
> + numa_info[i].node_mem = 0;
> + bitmap_zero(numa_info[i].node_cpu, MAX_CPUMASK_BITS);
> }
>
> nb_numa_nodes = 0;
> --
> 1.7.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 5/6] NUMA: convert -numa option to use OptsVisitor
2014-02-17 15:49 [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Igor Mammedov
` (3 preceding siblings ...)
2014-02-17 15:49 ` [Qemu-devel] [PATCH 4/6] NUMA: Add numa_info structure to contain numa nodes info Igor Mammedov
@ 2014-02-17 15:49 ` Igor Mammedov
2014-02-17 16:36 ` Eric Blake
2014-02-17 15:49 ` [Qemu-devel] [PATCH 6/6] NUMA: expand MAX_NODES from 64 to 128 Igor Mammedov
2014-02-26 16:42 ` [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Michael S. Tsirkin
6 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2014-02-17 15:49 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, andre.przywara, stefanha, mst, peter.maydell, armbru,
ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini, afaerber,
gaowanlong, rth
From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/sysemu/sysemu.h | 3 +-
numa.c | 147 +++++++++++++++++++++++------------------------
qapi-schema.json | 32 ++++++++++
vl.c | 11 +++-
4 files changed, 115 insertions(+), 78 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d873b42..20b05a3 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -140,9 +140,10 @@ typedef struct node_info {
DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
} NodeInfo;
extern NodeInfo numa_info[MAX_NODES];
-void numa_add(const char *optarg);
void set_numa_nodes(void);
void set_numa_modes(void);
+extern QemuOptsList qemu_numa_opts;
+int numa_init_func(QemuOpts *opts, void *opaque);
#define MAX_OPTION_ROMS 16
typedef struct QEMUOptionRom {
diff --git a/numa.c b/numa.c
index c3eca78..40c28b3 100644
--- a/numa.c
+++ b/numa.c
@@ -24,101 +24,96 @@
*/
#include "sysemu/sysemu.h"
-
-static void numa_node_parse_cpus(int nodenr, const char *cpus)
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/dealloc-visitor.h"
+QemuOptsList qemu_numa_opts = {
+ .name = "numa",
+ .implied_opt_name = "type",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head),
+ .desc = { { 0 } } /* validated with OptsVisitor */
+};
+
+static int numa_node_parse(NumaNodeOptions *node, QemuOpts *opts)
{
- char *endptr;
- unsigned long long value, endvalue;
-
- /* Empty CPU range strings will be considered valid, they will simply
- * not set any bit in the CPU bitmap.
- */
- if (!*cpus) {
- return;
- }
+ uint16_t nodenr;
+ uint16List *cpus = NULL;
- if (parse_uint(cpus, &value, &endptr, 10) < 0) {
- goto error;
- }
- if (*endptr == '-') {
- if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
- goto error;
- }
- } else if (*endptr == '\0') {
- endvalue = value;
+ if (node->has_nodeid) {
+ nodenr = node->nodeid;
} else {
- goto error;
+ nodenr = nb_numa_nodes;
}
- if (endvalue >= MAX_CPUMASK_BITS) {
- endvalue = MAX_CPUMASK_BITS - 1;
- fprintf(stderr,
- "qemu: NUMA: A max of %d VCPUs are supported\n",
- MAX_CPUMASK_BITS);
+ if (nodenr >= MAX_NODES) {
+ fprintf(stderr, "qemu: Max number of NUMA nodes reached: %"
+ PRIu16 "\n", nodenr);
+ return -1;
}
- if (endvalue < value) {
- goto error;
+ for (cpus = node->cpus; cpus; cpus = cpus->next) {
+ if (cpus->value > MAX_CPUMASK_BITS) {
+ fprintf(stderr, "qemu: cpu number %" PRIu16 " is bigger than %d",
+ cpus->value, MAX_CPUMASK_BITS);
+ continue;
+ }
+ bitmap_set(numa_info[nodenr].node_cpu, cpus->value, 1);
}
- bitmap_set(numa_info[nodenr].node_cpu, value, endvalue - value + 1);
- return;
+ if (node->has_mem) {
+ uint64_t mem_size = node->mem;
+ const char *mem_str = qemu_opt_get(opts, "mem");
+ /* Fix up legacy suffix-less format */
+ if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
+ mem_size <<= 20;
+ }
+ numa_info[nodenr].node_mem = mem_size;
+ }
-error:
- fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
- exit(1);
+ return 0;
}
-void numa_add(const char *optarg)
+int numa_init_func(QemuOpts *opts, void *opaque)
{
- char option[128];
- char *endptr;
- unsigned long long nodenr;
-
- optarg = get_opt_name(option, 128, optarg, ',');
- if (*optarg == ',') {
- optarg++;
+ NumaOptions *object = NULL;
+ Error *err = NULL;
+ int ret = 0;
+
+ {
+ OptsVisitor *ov = opts_visitor_new(opts);
+ visit_type_NumaOptions(opts_get_visitor(ov), &object, NULL, &err);
+ opts_visitor_cleanup(ov);
}
- if (!strcmp(option, "node")) {
-
- 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 {
- if (parse_uint_full(option, &nodenr, 10) < 0) {
- fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
- exit(1);
- }
- }
-
- if (nodenr >= MAX_NODES) {
- fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
- exit(1);
- }
+ if (error_is_set(&err)) {
+ fprintf(stderr, "qemu: %s\n", error_get_pretty(err));
+ error_free(err);
+ ret = -1;
+ goto error;
+ }
- if (get_param_value(option, 128, "mem", optarg) == 0) {
- numa_info[nodenr].node_mem = 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);
- }
- numa_info[nodenr].node_mem = sval;
- }
- if (get_param_value(option, 128, "cpus", optarg) != 0) {
- numa_node_parse_cpus(nodenr, option);
+ switch (object->kind) {
+ case NUMA_OPTIONS_KIND_NODE:
+ ret = numa_node_parse(object->node, opts);
+ if (ret) {
+ goto error;
}
nb_numa_nodes++;
- } else {
- fprintf(stderr, "Invalid -numa option: %s\n", option);
- exit(1);
+ break;
+ default:
+ fprintf(stderr, "qemu: Invalid NUMA options type.\n");
+ ret = -1;
}
+
+error:
+ if (object) {
+ QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
+ visit_type_NumaOptions(qapi_dealloc_get_visitor(dv),
+ &object, NULL, NULL);
+ qapi_dealloc_visitor_cleanup(dv);
+ }
+
+ return ret;
}
void set_numa_nodes(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index 7cfb5e5..8d05c6b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4420,3 +4420,35 @@
# Since: 1.7
##
{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
+
+##
+# @NumaOptions
+#
+# A discriminated record of NUMA options. (for OptsVisitor)
+#
+# Since 2.0
+##
+{ 'union': 'NumaOptions',
+ 'data': {
+ 'node': 'NumaNodeOptions' }}
+
+##
+# @NumaNodeOptions
+#
+# Create a guest NUMA node. (for OptsVisitor)
+#
+# @nodeid: #optional NUMA node ID (increase by 1 from 0 if omitted)
+#
+# @cpus: #optional VCPUs belong to this node (assign VCPUS round-robin
+# if omitted)
+#
+# @mem: #optional memory size of this node (go halves total memory to
+# each node if omitted)
+#
+# Since: 2.0
+##
+{ 'type': 'NumaNodeOptions',
+ 'data': {
+ '*nodeid': 'uint16',
+ '*cpus': ['uint16'],
+ '*mem': 'size' }}
diff --git a/vl.c b/vl.c
index 915f8b7..e070649 100644
--- a/vl.c
+++ b/vl.c
@@ -2765,6 +2765,7 @@ int main(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_tpmdev_opts);
qemu_add_opts(&qemu_realtime_opts);
qemu_add_opts(&qemu_msg_opts);
+ qemu_add_opts(&qemu_numa_opts);
runstate_init();
@@ -2952,7 +2953,10 @@ int main(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_numa:
- numa_add(optarg);
+ opts = qemu_opts_parse(qemu_find_opts("numa"), optarg, 1);
+ if (!opts) {
+ exit(1);
+ }
break;
case QEMU_OPTION_display:
display_type = select_display(optarg);
@@ -4042,6 +4046,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"), numa_init_func,
+ NULL, 1) != 0) {
+ exit(1);
+ }
+
set_numa_nodes();
if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) {
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 5/6] NUMA: convert -numa option to use OptsVisitor
2014-02-17 15:49 ` [Qemu-devel] [PATCH 5/6] NUMA: convert -numa option to use OptsVisitor Igor Mammedov
@ 2014-02-17 16:36 ` Eric Blake
2014-02-26 14:40 ` [Qemu-devel] [PATCH v2 " Igor Mammedov
0 siblings, 1 reply; 17+ messages in thread
From: Eric Blake @ 2014-02-17 16:36 UTC (permalink / raw)
To: Igor Mammedov, qemu-devel
Cc: agraf, andre.przywara, stefanha, mst, peter.maydell, armbru,
ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini, afaerber,
gaowanlong, rth
[-- Attachment #1: Type: text/plain, Size: 972 bytes --]
On 02/17/2014 08:49 AM, Igor Mammedov wrote:
> From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -4420,3 +4420,35 @@
> +##
> +# @NumaNodeOptions
> +#
> +# Create a guest NUMA node. (for OptsVisitor)
> +#
> +# @nodeid: #optional NUMA node ID (increase by 1 from 0 if omitted)
> +#
> +# @cpus: #optional VCPUs belong to this node (assign VCPUS round-robin
s/belong/belonging/
> +# if omitted)
> +#
> +# @mem: #optional memory size of this node (go halves total memory to
> +# each node if omitted)
Awkward grammar. I suggest:
#optional memory size of this node (equally divide total memory among
nodes if omitted)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v2 5/6] NUMA: convert -numa option to use OptsVisitor
2014-02-17 16:36 ` Eric Blake
@ 2014-02-26 14:40 ` Igor Mammedov
2014-02-26 18:08 ` Eduardo Habkost
0 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2014-02-26 14:40 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, hutao, armbru, lcapitulino, aliguori, pbonzini,
gaowanlong
From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
- altered doc comment according to Eric Blake <eblake@redhat.com>
suggestions.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/sysemu/sysemu.h | 3 +-
numa.c | 147 +++++++++++++++++++++++------------------------
qapi-schema.json | 32 ++++++++++
vl.c | 11 +++-
4 files changed, 115 insertions(+), 78 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d873b42..20b05a3 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -140,9 +140,10 @@ typedef struct node_info {
DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
} NodeInfo;
extern NodeInfo numa_info[MAX_NODES];
-void numa_add(const char *optarg);
void set_numa_nodes(void);
void set_numa_modes(void);
+extern QemuOptsList qemu_numa_opts;
+int numa_init_func(QemuOpts *opts, void *opaque);
#define MAX_OPTION_ROMS 16
typedef struct QEMUOptionRom {
diff --git a/numa.c b/numa.c
index c3eca78..40c28b3 100644
--- a/numa.c
+++ b/numa.c
@@ -24,101 +24,96 @@
*/
#include "sysemu/sysemu.h"
-
-static void numa_node_parse_cpus(int nodenr, const char *cpus)
+#include "qapi-visit.h"
+#include "qapi/opts-visitor.h"
+#include "qapi/dealloc-visitor.h"
+QemuOptsList qemu_numa_opts = {
+ .name = "numa",
+ .implied_opt_name = "type",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head),
+ .desc = { { 0 } } /* validated with OptsVisitor */
+};
+
+static int numa_node_parse(NumaNodeOptions *node, QemuOpts *opts)
{
- char *endptr;
- unsigned long long value, endvalue;
-
- /* Empty CPU range strings will be considered valid, they will simply
- * not set any bit in the CPU bitmap.
- */
- if (!*cpus) {
- return;
- }
+ uint16_t nodenr;
+ uint16List *cpus = NULL;
- if (parse_uint(cpus, &value, &endptr, 10) < 0) {
- goto error;
- }
- if (*endptr == '-') {
- if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
- goto error;
- }
- } else if (*endptr == '\0') {
- endvalue = value;
+ if (node->has_nodeid) {
+ nodenr = node->nodeid;
} else {
- goto error;
+ nodenr = nb_numa_nodes;
}
- if (endvalue >= MAX_CPUMASK_BITS) {
- endvalue = MAX_CPUMASK_BITS - 1;
- fprintf(stderr,
- "qemu: NUMA: A max of %d VCPUs are supported\n",
- MAX_CPUMASK_BITS);
+ if (nodenr >= MAX_NODES) {
+ fprintf(stderr, "qemu: Max number of NUMA nodes reached: %"
+ PRIu16 "\n", nodenr);
+ return -1;
}
- if (endvalue < value) {
- goto error;
+ for (cpus = node->cpus; cpus; cpus = cpus->next) {
+ if (cpus->value > MAX_CPUMASK_BITS) {
+ fprintf(stderr, "qemu: cpu number %" PRIu16 " is bigger than %d",
+ cpus->value, MAX_CPUMASK_BITS);
+ continue;
+ }
+ bitmap_set(numa_info[nodenr].node_cpu, cpus->value, 1);
}
- bitmap_set(numa_info[nodenr].node_cpu, value, endvalue - value + 1);
- return;
+ if (node->has_mem) {
+ uint64_t mem_size = node->mem;
+ const char *mem_str = qemu_opt_get(opts, "mem");
+ /* Fix up legacy suffix-less format */
+ if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
+ mem_size <<= 20;
+ }
+ numa_info[nodenr].node_mem = mem_size;
+ }
-error:
- fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
- exit(1);
+ return 0;
}
-void numa_add(const char *optarg)
+int numa_init_func(QemuOpts *opts, void *opaque)
{
- char option[128];
- char *endptr;
- unsigned long long nodenr;
-
- optarg = get_opt_name(option, 128, optarg, ',');
- if (*optarg == ',') {
- optarg++;
+ NumaOptions *object = NULL;
+ Error *err = NULL;
+ int ret = 0;
+
+ {
+ OptsVisitor *ov = opts_visitor_new(opts);
+ visit_type_NumaOptions(opts_get_visitor(ov), &object, NULL, &err);
+ opts_visitor_cleanup(ov);
}
- if (!strcmp(option, "node")) {
-
- 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 {
- if (parse_uint_full(option, &nodenr, 10) < 0) {
- fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
- exit(1);
- }
- }
-
- if (nodenr >= MAX_NODES) {
- fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
- exit(1);
- }
+ if (error_is_set(&err)) {
+ fprintf(stderr, "qemu: %s\n", error_get_pretty(err));
+ error_free(err);
+ ret = -1;
+ goto error;
+ }
- if (get_param_value(option, 128, "mem", optarg) == 0) {
- numa_info[nodenr].node_mem = 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);
- }
- numa_info[nodenr].node_mem = sval;
- }
- if (get_param_value(option, 128, "cpus", optarg) != 0) {
- numa_node_parse_cpus(nodenr, option);
+ switch (object->kind) {
+ case NUMA_OPTIONS_KIND_NODE:
+ ret = numa_node_parse(object->node, opts);
+ if (ret) {
+ goto error;
}
nb_numa_nodes++;
- } else {
- fprintf(stderr, "Invalid -numa option: %s\n", option);
- exit(1);
+ break;
+ default:
+ fprintf(stderr, "qemu: Invalid NUMA options type.\n");
+ ret = -1;
}
+
+error:
+ if (object) {
+ QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
+ visit_type_NumaOptions(qapi_dealloc_get_visitor(dv),
+ &object, NULL, NULL);
+ qapi_dealloc_visitor_cleanup(dv);
+ }
+
+ return ret;
}
void set_numa_nodes(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index 7cfb5e5..75095a9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4420,3 +4420,35 @@
# Since: 1.7
##
{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
+
+##
+# @NumaOptions
+#
+# A discriminated record of NUMA options. (for OptsVisitor)
+#
+# Since 2.0
+##
+{ 'union': 'NumaOptions',
+ 'data': {
+ 'node': 'NumaNodeOptions' }}
+
+##
+# @NumaNodeOptions
+#
+# Create a guest NUMA node. (for OptsVisitor)
+#
+# @nodeid: #optional NUMA node ID (increase by 1 from 0 if omitted)
+#
+# @cpus: #optional VCPUs belonging to this node (assign VCPUS round-robin
+# if omitted)
+#
+# @mem: #optional memory size of this node (equally divide total memory among
+# nodes if omitted)
+#
+# Since: 2.0
+##
+{ 'type': 'NumaNodeOptions',
+ 'data': {
+ '*nodeid': 'uint16',
+ '*cpus': ['uint16'],
+ '*mem': 'size' }}
diff --git a/vl.c b/vl.c
index 915f8b7..e070649 100644
--- a/vl.c
+++ b/vl.c
@@ -2765,6 +2765,7 @@ int main(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_tpmdev_opts);
qemu_add_opts(&qemu_realtime_opts);
qemu_add_opts(&qemu_msg_opts);
+ qemu_add_opts(&qemu_numa_opts);
runstate_init();
@@ -2952,7 +2953,10 @@ int main(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_numa:
- numa_add(optarg);
+ opts = qemu_opts_parse(qemu_find_opts("numa"), optarg, 1);
+ if (!opts) {
+ exit(1);
+ }
break;
case QEMU_OPTION_display:
display_type = select_display(optarg);
@@ -4042,6 +4046,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"), numa_init_func,
+ NULL, 1) != 0) {
+ exit(1);
+ }
+
set_numa_nodes();
if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) {
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/6] NUMA: convert -numa option to use OptsVisitor
2014-02-26 14:40 ` [Qemu-devel] [PATCH v2 " Igor Mammedov
@ 2014-02-26 18:08 ` Eduardo Habkost
0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2014-02-26 18:08 UTC (permalink / raw)
To: Igor Mammedov
Cc: hutao, qemu-devel, lcapitulino, gaowanlong, aliguori, pbonzini,
armbru
On Wed, Feb 26, 2014 at 03:40:58PM +0100, Igor Mammedov wrote:
> From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
> - altered doc comment according to Eric Blake <eblake@redhat.com>
> suggestions.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
I didn't know QemuOptsVisitor finally supports ranges when parsing
integer lists. Nice.
I also confirm that it keeps compatibility with previous behavior when
the size suffix is omitted (defaulting to "M").
Tested-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> include/sysemu/sysemu.h | 3 +-
> numa.c | 147 +++++++++++++++++++++++------------------------
> qapi-schema.json | 32 ++++++++++
> vl.c | 11 +++-
> 4 files changed, 115 insertions(+), 78 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index d873b42..20b05a3 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -140,9 +140,10 @@ typedef struct node_info {
> DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
> } NodeInfo;
> extern NodeInfo numa_info[MAX_NODES];
> -void numa_add(const char *optarg);
> void set_numa_nodes(void);
> void set_numa_modes(void);
> +extern QemuOptsList qemu_numa_opts;
> +int numa_init_func(QemuOpts *opts, void *opaque);
>
> #define MAX_OPTION_ROMS 16
> typedef struct QEMUOptionRom {
> diff --git a/numa.c b/numa.c
> index c3eca78..40c28b3 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -24,101 +24,96 @@
> */
>
> #include "sysemu/sysemu.h"
> -
> -static void numa_node_parse_cpus(int nodenr, const char *cpus)
> +#include "qapi-visit.h"
> +#include "qapi/opts-visitor.h"
> +#include "qapi/dealloc-visitor.h"
> +QemuOptsList qemu_numa_opts = {
> + .name = "numa",
> + .implied_opt_name = "type",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head),
> + .desc = { { 0 } } /* validated with OptsVisitor */
> +};
> +
> +static int numa_node_parse(NumaNodeOptions *node, QemuOpts *opts)
> {
> - char *endptr;
> - unsigned long long value, endvalue;
> -
> - /* Empty CPU range strings will be considered valid, they will simply
> - * not set any bit in the CPU bitmap.
> - */
> - if (!*cpus) {
> - return;
> - }
> + uint16_t nodenr;
> + uint16List *cpus = NULL;
>
> - if (parse_uint(cpus, &value, &endptr, 10) < 0) {
> - goto error;
> - }
> - if (*endptr == '-') {
> - if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
> - goto error;
> - }
> - } else if (*endptr == '\0') {
> - endvalue = value;
> + if (node->has_nodeid) {
> + nodenr = node->nodeid;
> } else {
> - goto error;
> + nodenr = nb_numa_nodes;
> }
>
> - if (endvalue >= MAX_CPUMASK_BITS) {
> - endvalue = MAX_CPUMASK_BITS - 1;
> - fprintf(stderr,
> - "qemu: NUMA: A max of %d VCPUs are supported\n",
> - MAX_CPUMASK_BITS);
> + if (nodenr >= MAX_NODES) {
> + fprintf(stderr, "qemu: Max number of NUMA nodes reached: %"
> + PRIu16 "\n", nodenr);
> + return -1;
> }
>
> - if (endvalue < value) {
> - goto error;
> + for (cpus = node->cpus; cpus; cpus = cpus->next) {
> + if (cpus->value > MAX_CPUMASK_BITS) {
> + fprintf(stderr, "qemu: cpu number %" PRIu16 " is bigger than %d",
> + cpus->value, MAX_CPUMASK_BITS);
> + continue;
> + }
> + bitmap_set(numa_info[nodenr].node_cpu, cpus->value, 1);
> }
>
> - bitmap_set(numa_info[nodenr].node_cpu, value, endvalue - value + 1);
> - return;
> + if (node->has_mem) {
> + uint64_t mem_size = node->mem;
> + const char *mem_str = qemu_opt_get(opts, "mem");
> + /* Fix up legacy suffix-less format */
> + if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
> + mem_size <<= 20;
> + }
> + numa_info[nodenr].node_mem = mem_size;
> + }
>
> -error:
> - fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
> - exit(1);
> + return 0;
> }
>
> -void numa_add(const char *optarg)
> +int numa_init_func(QemuOpts *opts, void *opaque)
> {
> - char option[128];
> - char *endptr;
> - unsigned long long nodenr;
> -
> - optarg = get_opt_name(option, 128, optarg, ',');
> - if (*optarg == ',') {
> - optarg++;
> + NumaOptions *object = NULL;
> + Error *err = NULL;
> + int ret = 0;
> +
> + {
> + OptsVisitor *ov = opts_visitor_new(opts);
> + visit_type_NumaOptions(opts_get_visitor(ov), &object, NULL, &err);
> + opts_visitor_cleanup(ov);
> }
> - if (!strcmp(option, "node")) {
> -
> - 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 {
> - if (parse_uint_full(option, &nodenr, 10) < 0) {
> - fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
> - exit(1);
> - }
> - }
> -
> - if (nodenr >= MAX_NODES) {
> - fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
> - exit(1);
> - }
> + if (error_is_set(&err)) {
> + fprintf(stderr, "qemu: %s\n", error_get_pretty(err));
> + error_free(err);
> + ret = -1;
> + goto error;
> + }
>
> - if (get_param_value(option, 128, "mem", optarg) == 0) {
> - numa_info[nodenr].node_mem = 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);
> - }
> - numa_info[nodenr].node_mem = sval;
> - }
> - if (get_param_value(option, 128, "cpus", optarg) != 0) {
> - numa_node_parse_cpus(nodenr, option);
> + switch (object->kind) {
> + case NUMA_OPTIONS_KIND_NODE:
> + ret = numa_node_parse(object->node, opts);
> + if (ret) {
> + goto error;
> }
> nb_numa_nodes++;
> - } else {
> - fprintf(stderr, "Invalid -numa option: %s\n", option);
> - exit(1);
> + break;
> + default:
> + fprintf(stderr, "qemu: Invalid NUMA options type.\n");
> + ret = -1;
> }
> +
> +error:
> + if (object) {
> + QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
> + visit_type_NumaOptions(qapi_dealloc_get_visitor(dv),
> + &object, NULL, NULL);
> + qapi_dealloc_visitor_cleanup(dv);
> + }
> +
> + return ret;
> }
>
> void set_numa_nodes(void)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7cfb5e5..75095a9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4420,3 +4420,35 @@
> # Since: 1.7
> ##
> { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> +
> +##
> +# @NumaOptions
> +#
> +# A discriminated record of NUMA options. (for OptsVisitor)
> +#
> +# Since 2.0
> +##
> +{ 'union': 'NumaOptions',
> + 'data': {
> + 'node': 'NumaNodeOptions' }}
> +
> +##
> +# @NumaNodeOptions
> +#
> +# Create a guest NUMA node. (for OptsVisitor)
> +#
> +# @nodeid: #optional NUMA node ID (increase by 1 from 0 if omitted)
> +#
> +# @cpus: #optional VCPUs belonging to this node (assign VCPUS round-robin
> +# if omitted)
> +#
> +# @mem: #optional memory size of this node (equally divide total memory among
> +# nodes if omitted)
> +#
> +# Since: 2.0
> +##
> +{ 'type': 'NumaNodeOptions',
> + 'data': {
> + '*nodeid': 'uint16',
> + '*cpus': ['uint16'],
> + '*mem': 'size' }}
> diff --git a/vl.c b/vl.c
> index 915f8b7..e070649 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2765,6 +2765,7 @@ int main(int argc, char **argv, char **envp)
> qemu_add_opts(&qemu_tpmdev_opts);
> qemu_add_opts(&qemu_realtime_opts);
> qemu_add_opts(&qemu_msg_opts);
> + qemu_add_opts(&qemu_numa_opts);
>
> runstate_init();
>
> @@ -2952,7 +2953,10 @@ int main(int argc, char **argv, char **envp)
> }
> break;
> case QEMU_OPTION_numa:
> - numa_add(optarg);
> + opts = qemu_opts_parse(qemu_find_opts("numa"), optarg, 1);
> + if (!opts) {
> + exit(1);
> + }
> break;
> case QEMU_OPTION_display:
> display_type = select_display(optarg);
> @@ -4042,6 +4046,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"), numa_init_func,
> + NULL, 1) != 0) {
> + exit(1);
> + }
> +
> set_numa_nodes();
>
> if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) {
> --
> 1.7.1
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 6/6] NUMA: expand MAX_NODES from 64 to 128
2014-02-17 15:49 [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Igor Mammedov
` (4 preceding siblings ...)
2014-02-17 15:49 ` [Qemu-devel] [PATCH 5/6] NUMA: convert -numa option to use OptsVisitor Igor Mammedov
@ 2014-02-17 15:49 ` Igor Mammedov
2014-02-26 18:40 ` Eduardo Habkost
2014-02-26 16:42 ` [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Michael S. Tsirkin
6 siblings, 1 reply; 17+ messages in thread
From: Igor Mammedov @ 2014-02-17 15:49 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, andre.przywara, stefanha, mst, peter.maydell, armbru,
ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini, afaerber,
gaowanlong, rth
From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
libnuma choosed 128 for MAX_NODES, so we follow libnuma here.
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/sysemu/sysemu.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 20b05a3..4c94cf5 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -132,7 +132,7 @@ extern size_t boot_splash_filedata_size;
extern uint8_t qemu_extra_params_fw[2];
extern QEMUClockType rtc_clock;
-#define MAX_NODES 64
+#define MAX_NODES 128
#define MAX_CPUMASK_BITS 255
extern int nb_numa_nodes;
typedef struct node_info {
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 6/6] NUMA: expand MAX_NODES from 64 to 128
2014-02-17 15:49 ` [Qemu-devel] [PATCH 6/6] NUMA: expand MAX_NODES from 64 to 128 Igor Mammedov
@ 2014-02-26 18:40 ` Eduardo Habkost
0 siblings, 0 replies; 17+ messages in thread
From: Eduardo Habkost @ 2014-02-26 18:40 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.maydell, aliguori, andre.przywara, armbru, mst, qemu-devel,
agraf, qemu-ppc, stefanha, pbonzini, lcapitulino, afaerber,
gaowanlong, rth
On Mon, Feb 17, 2014 at 04:49:47PM +0100, Igor Mammedov wrote:
> From: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>
> libnuma choosed 128 for MAX_NODES, so we follow libnuma here.
>
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
--
Eduardo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor
2014-02-17 15:49 [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Igor Mammedov
` (5 preceding siblings ...)
2014-02-17 15:49 ` [Qemu-devel] [PATCH 6/6] NUMA: expand MAX_NODES from 64 to 128 Igor Mammedov
@ 2014-02-26 16:42 ` Michael S. Tsirkin
2014-02-26 16:53 ` Paolo Bonzini
6 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2014-02-26 16:42 UTC (permalink / raw)
To: Igor Mammedov
Cc: agraf, andre.przywara, stefanha, peter.maydell, qemu-devel,
armbru, ehabkost, lcapitulino, qemu-ppc, aliguori, pbonzini,
afaerber, gaowanlong, rth
On Mon, Feb 17, 2014 at 04:49:41PM +0100, Igor Mammedov wrote:
> ... while at it move most of numa related code to
> a dedicated file so that it won't to clatter vl.c.
>
> git tree for testing:
> https://github.com/imammedo/qemu/commits/numa_prep_v1
>
> Igor Mammedov (1):
> vl.c: fix style issue
Just to clarify - Paolo, you intend to merge this?
I acked pc.c changes.
> Wanlong Gao (5):
> NUMA: move numa related code to new file numa.c
> NUMA: check if the total numa memory size is equal to ram_size
> NUMA: Add numa_info structure to contain numa nodes info
> NUMA: convert -numa option to use OptsVisitor
> NUMA: expand MAX_NODES from 64 to 128
>
> Makefile.target | 2 +-
> cpus.c | 14 ----
> hw/i386/pc.c | 14 ++-
> hw/ppc/spapr.c | 11 ++-
> include/sysemu/cpus.h | 1 -
> include/sysemu/sysemu.h | 14 +++-
> monitor.c | 2 +-
> numa.c | 189 +++++++++++++++++++++++++++++++++++++++++++++++
> qapi-schema.json | 32 ++++++++
> vl.c | 155 +++-----------------------------------
> 10 files changed, 262 insertions(+), 172 deletions(-)
> create mode 100644 numa.c
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor
2014-02-26 16:42 ` [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor Michael S. Tsirkin
@ 2014-02-26 16:53 ` Paolo Bonzini
0 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2014-02-26 16:53 UTC (permalink / raw)
To: Michael S. Tsirkin, Igor Mammedov
Cc: agraf, andre.przywara, stefanha, peter.maydell, qemu-devel,
armbru, ehabkost, lcapitulino, qemu-ppc, aliguori, afaerber,
gaowanlong, rth
Il 26/02/2014 17:42, Michael S. Tsirkin ha scritto:
>> > ... while at it move most of numa related code to
>> > a dedicated file so that it won't to clatter vl.c.
>> >
>> > git tree for testing:
>> > https://github.com/imammedo/qemu/commits/numa_prep_v1
>> >
>> > Igor Mammedov (1):
>> > vl.c: fix style issue
> Just to clarify - Paolo, you intend to merge this?
Some cleanups could go in 2.0 but the meat will have to wait, so there's
no need to hurry. I haven't yet talked with all involved people about
the merging plan but here is a possibility:
* during soft/hard freeze we'll finish development of the memdev
backend, to include the -mem-path replacement. I already have some
patches for this, that I intend to post to the list sometime soon.
* at the beginning of the 2.1 cycle I will post a pull request with Hu
Tao's NUMA policies, which includes all the memdev backend.
* memory hotplug would go through your tree so it should wait for NUMA
policies to be in. Of course, Igor can also post it soon, so that it's
also mostly ready at the beginning of the 2.1 cycle.
> I acked pc.c changes.
Thanks!
Paolo
^ permalink raw reply [flat|nested] 17+ messages in thread