| Message ID | 463560c2fbe9c476d4a500008618f01740cd5631.1402299637.git.hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 06/09/2014 04:25 AM, Hu Tao 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> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > numa.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > + if (numa_total != ram_size) { > + error_report("qemu: total memory size for NUMA nodes (%" PRIu64 ")" > + " should equal to RAM size (" RAM_ADDR_FMT ")\n", error_report() should not include trailing \n > + numa_total, ram_size); > + exit(1); Not your fault that this file is full of exit(1), but it would be nice to have a cleanup patch someday that uses EXIT_FAILURE.
On Mon, Jun 09, 2014 at 05:02:51PM -0600, Eric Blake wrote: > On 06/09/2014 04:25 AM, Hu Tao 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> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > numa.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > > + if (numa_total != ram_size) { > > + error_report("qemu: total memory size for NUMA nodes (%" PRIu64 ")" > > + " should equal to RAM size (" RAM_ADDR_FMT ")\n", > > error_report() should not include trailing \n Thanks. > > > + numa_total, ram_size); > > + exit(1); > > Not your fault that this file is full of exit(1), but it would be nice > to have a cleanup patch someday that uses EXIT_FAILURE. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
On 06/09/2014 08:29 PM, Hu Tao wrote: > On Mon, Jun 09, 2014 at 05:02:51PM -0600, Eric Blake wrote: >> On 06/09/2014 04:25 AM, Hu Tao 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> >>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> >>> --- >>> numa.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >> >>> + if (numa_total != ram_size) { >>> + error_report("qemu: total memory size for NUMA nodes (%" PRIu64 ")" >>> + " should equal to RAM size (" RAM_ADDR_FMT ")\n", >> >> error_report() should not include trailing \n > > Thanks. Sorry for not noticing earlier, but a couple more things to fix: error_report already prefixes the error message, so s/qemu: // The grammar is a bit awkward; how about: "total memory for NUMA nodes (%" PRIu64 ") should equal RAM size (" RAM_ADDR_FMT ")"
On Mon, Jun 09, 2014 at 08:36:17PM -0600, Eric Blake wrote: > On 06/09/2014 08:29 PM, Hu Tao wrote: > > On Mon, Jun 09, 2014 at 05:02:51PM -0600, Eric Blake wrote: > >> On 06/09/2014 04:25 AM, Hu Tao 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> > >>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > >>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > >>> --- > >>> numa.c | 14 ++++++++++++++ > >>> 1 file changed, 14 insertions(+) > >>> > >> > >>> + if (numa_total != ram_size) { > >>> + error_report("qemu: total memory size for NUMA nodes (%" PRIu64 ")" > >>> + " should equal to RAM size (" RAM_ADDR_FMT ")\n", > >> > >> error_report() should not include trailing \n > > > > Thanks. > > Sorry for not noticing earlier, but a couple more things to fix: > > error_report already prefixes the error message, so s/qemu: // > > The grammar is a bit awkward; how about: > > "total memory for NUMA nodes (%" PRIu64 ") should equal RAM size (" > RAM_ADDR_FMT ")" MST's already said equal RAM size. I looked up in a dictionary and found a `equal to' example. But I was still wrong that `euqal to' is adj. form, if it is used as verb there is no following `to'. Thanks. Hu
diff --git a/numa.c b/numa.c index c419deb..b106531 100644 --- a/numa.c +++ b/numa.c @@ -26,6 +26,8 @@ #include "exec/cpu-common.h" #include "qemu/bitmap.h" #include "qom/cpu.h" +#include "qemu/error-report.h" +#include "include/exec/cpu-common.h" /* for RAM_ADDR_FMT */ static void numa_node_parse_cpus(int nodenr, const char *cpus) { @@ -126,6 +128,7 @@ void numa_add(const char *optarg) void set_numa_nodes(void) { if (nb_numa_nodes > 0) { + uint64_t numa_total; int i; if (nb_numa_nodes > MAX_NODES) { @@ -153,6 +156,17 @@ void set_numa_nodes(void) node_mem[i] = ram_size - usedmem; } + numa_total = 0; + for (i = 0; i < nb_numa_nodes; i++) { + numa_total += node_mem[i]; + } + if (numa_total != ram_size) { + error_report("qemu: total memory size for NUMA nodes (%" PRIu64 ")" + " should equal to RAM size (" RAM_ADDR_FMT ")\n", + numa_total, ram_size); + exit(1); + } + for (i = 0; i < nb_numa_nodes; i++) { if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) { break;