[libvirt] [PATCH RFC]:tolerate numa_node_size64 < 0 because nodeid might start from 1 instead of 0
In nodeGetFreeMemory/nodeGetCellsFreeMemory, they will calculate the free memory of every nodes. They assumed that nodeid of NUMA machine must be continuous and start from 0. But here is a counter-example: # numactl -H available: 1 nodes (1) node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 node 1 size: 16340 MB node 1 free: 11065 MB test results before this patch: #virsh freecell error: internal error Failed to query NUMA free memory #virsh freecell 0 error: internal error Failed to query NUMA free memory for node: 0 after this patch: # virsh freecell Total: 15772580 KiB # virsh freecell 0 0: 0 KiB --- libvirt/src/nodeinfo.c.orig 2013-07-08 04:25:11.970351101 -0500 +++ libvirt/src/nodeinfo.c 2013-07-08 09:00:30.834471495 -0500 @@ -1717,10 +1717,6 @@ nodeGetCellsFreeMemory(unsigned long lon for (numCells = 0, n = startCell; n <= lastCell; n++) { long long mem; if (numa_node_size64(n, &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to query NUMA free memory for node: %d"), - n); - goto cleanup; } freeMems[numCells++] = mem; } @@ -1743,9 +1739,6 @@ nodeGetFreeMemory(void) for (n = 0; n <= numa_max_node(); n++) { long long mem; if (numa_node_size64(n, &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; } freeMem += mem; }
Hi, no one cares about it? On 7/8/2013 10:03 PM, hejia hejia wrote:
In nodeGetFreeMemory/nodeGetCellsFreeMemory, they will calculate the free memory of every nodes. They assumed that nodeid of NUMA machine must be continuous and start from 0. But here is a counter-example:
# numactl -H available: 1 nodes (1) node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 node 1 size: 16340 MB node 1 free: 11065 MB
test results before this patch: #virsh freecell error: internal error Failed to query NUMA free memory #virsh freecell 0 error: internal error Failed to query NUMA free memory for node: 0
after this patch: # virsh freecell Total: 15772580 KiB # virsh freecell 0 0: 0 KiB
--- libvirt/src/nodeinfo.c.orig 2013-07-08 04:25:11.970351101 -0500 +++ libvirt/src/nodeinfo.c 2013-07-08 09:00:30.834471495 -0500 @@ -1717,10 +1717,6 @@ nodeGetCellsFreeMemory(unsigned long lon for (numCells = 0, n = startCell; n <= lastCell; n++) { long long mem; if (numa_node_size64(n, &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to query NUMA free memory for node: %d"), - n); - goto cleanup; } freeMems[numCells++] = mem; } @@ -1743,9 +1739,6 @@ nodeGetFreeMemory(void) for (n = 0; n <= numa_max_node(); n++) { long long mem; if (numa_node_size64(n, &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; } freeMem += mem; }
Thanks, In my opinion, the best solution is let numa_node_size64 return the details of error, not merely -1. But this is out of libvirt's scope:( On 7/11/2013 4:06 PM, Peter Krempa wrote:
On 07/11/13 09:54, hejianet wrote:
Hi, no one cares about it?
Hi,
I have this on my to-do list, but I'll need to test it and didn't manage to find time for this. I'll try to do it this week.
Peter
On 07/08/13 16:03, hejia hejia wrote:
In nodeGetFreeMemory/nodeGetCellsFreeMemory, they will calculate the free memory of every nodes. They assumed that nodeid of NUMA machine must be continuous and start from 0. But here is a counter-example:
# numactl -H available: 1 nodes (1) node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 node 1 size: 16340 MB node 1 free: 11065 MB
test results before this patch: #virsh freecell error: internal error Failed to query NUMA free memory #virsh freecell 0 error: internal error Failed to query NUMA free memory for node: 0
after this patch: # virsh freecell Total: 15772580 KiB # virsh freecell 0 0: 0 KiB
--- libvirt/src/nodeinfo.c.orig 2013-07-08 04:25:11.970351101 -0500 +++ libvirt/src/nodeinfo.c 2013-07-08 09:00:30.834471495 -0500 @@ -1717,10 +1717,6 @@ nodeGetCellsFreeMemory(unsigned long lon for (numCells = 0, n = startCell; n <= lastCell; n++) { long long mem; if (numa_node_size64(n, &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to query NUMA free memory for node: %d"), - n);
You leave an empty block here
- goto cleanup; } freeMems[numCells++] = mem; } @@ -1743,9 +1739,6 @@ nodeGetFreeMemory(void) for (n = 0; n <= numa_max_node(); n++) { long long mem; if (numa_node_size64(n, &mem) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup;
and here too.
} freeMem += mem; }
I'll be re-posting this patch with the problems fixed soon. Peter
participants (3)
-
hejia hejia -
hejianet
-
Peter Krempa