[libvirt] [PATCH] Add NUMA memory information to virsh capabilities output.
Hi, This is a follow up to a previous proposed patch that added NUMA memory and CPU thread siblings information to virsh capabilities outout (https://www.redhat.com/archives/libvir-list/2012-November/msg00449.html) Since in a recent commit the thread siblings information was added (http://libvirt.org/git/?p=libvirt.git;a=commit;h=828820e2d). I have modified my patch to just add the NUMA memory information. An example of the output from virsh capabilities looks like the following text: <topology> <cells num='2'> <cell id='0'> <memory unit='KiB'>67073420</memory> <cpus num='16'> <cpu id='0' socket_id='0' core_id='0' siblings='0,16'/> <cpu id='1' socket_id='0' core_id='1' siblings='1,17'/> ... ... </cpus> </cell> Dusty Mabe Dusty Mabe (1): Add NUMA memory information to virsh capabilities output. docs/schemas/capability.rng | 10 ++++ src/conf/capabilities.c | 8 +++ src/conf/capabilities.h | 2 + src/nodeinfo.c | 64 +++++++++++++++++++++- src/test/test_driver.c | 2 +- src/xen/xend_internal.c | 2 +- tests/capabilityschemadata/caps-test3.xml | 88 +++++++++++++++++++++++++++++++ 7 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 tests/capabilityschemadata/caps-test3.xml -- 1.7.11.7
--- docs/schemas/capability.rng | 10 ++++ src/conf/capabilities.c | 8 +++ src/conf/capabilities.h | 2 + src/nodeinfo.c | 64 +++++++++++++++++++++- src/test/test_driver.c | 2 +- src/xen/xend_internal.c | 2 +- tests/capabilityschemadata/caps-test3.xml | 88 +++++++++++++++++++++++++++++++ 7 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 tests/capabilityschemadata/caps-test3.xml diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 53fb04a..e0396aa 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -177,6 +177,10 @@ </attribute> <optional> + <ref name='memory'/> + </optional> + + <optional> <element name='cpus'> <attribute name='num'> <ref name='unsignedInt'/> @@ -189,6 +193,12 @@ </element> </define> + <define name='memory'> + <element name='memory'> + <ref name='scaledInteger'/> + </element> + </define> + <define name='cpu'> <element name='cpu'> <attribute name='id'> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index a0e597b..9824f0c 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -273,6 +273,7 @@ int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + unsigned long long mem, virCapsHostNUMACellCPUPtr cpus) { virCapsHostNUMACellPtr cell; @@ -286,6 +287,7 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps, cell->ncpus = ncpus; cell->num = num; + cell->mem = mem; cell->cpus = cpus; caps->host.numaCell[caps->host.nnumaCell++] = cell; @@ -712,6 +714,12 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells); for (i = 0; i < ncells; i++) { virBufferAsprintf(xml, " <cell id='%d'>\n", cells[i]->num); + + /* Print out the numacell memory total if it is available */ + if (cells[i]->mem) + virBufferAsprintf(xml, " <memory unit='KiB'>%llu</memory>\n", + cells[i]->mem); + virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); for (j = 0; j < cells[i]->ncpus; j++) { virBufferAsprintf(xml, " <cpu id='%d'", diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index cc01765..6c67fb3 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -99,6 +99,7 @@ typedef virCapsHostNUMACell *virCapsHostNUMACellPtr; struct _virCapsHostNUMACell { int num; int ncpus; + unsigned long long mem; /* in kibibytes */ virCapsHostNUMACellCPUPtr cpus; }; @@ -210,6 +211,7 @@ extern int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + unsigned long long mem, virCapsHostNUMACellCPUPtr cpus); diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 1622322..f9e173b 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -102,6 +102,7 @@ static int linuxNodeGetMemoryStats(FILE *meminfo, int cellNum, virNodeMemoryStatsPtr params, int *nparams); +static unsigned long long nodeGetCellMemory(int cell); /* Return the positive decimal contents of the given * DIR/cpu%u/FILE, or -1 on error. If DEFAULT_VALUE is non-negative @@ -1531,6 +1532,7 @@ nodeCapsInitNUMA(virCapsPtr caps) int n; unsigned long *mask = NULL; unsigned long *allonesmask = NULL; + unsigned long long memory; virCapsHostNUMACellCPUPtr cpus = NULL; int ret = -1; int max_n_cpus = NUMA_MAX_N_CPUS; @@ -1562,6 +1564,11 @@ nodeCapsInitNUMA(virCapsPtr caps) continue; } + /* Detect the amount of memory in the numa cell */ + memory = nodeGetCellMemory(n); + if (memory == 0) + goto cleanup; + for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) if (MASK_CPU_ISSET(mask, i)) ncpus++; @@ -1578,7 +1585,7 @@ nodeCapsInitNUMA(virCapsPtr caps) } } - if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0) + if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, memory, cpus) < 0) goto cleanup; } @@ -1665,6 +1672,54 @@ cleanup: return freeMem; } +/** + * nodeGetCellMemory + * @cell: The number of the numa cell to get memory info for. + * + * Will call the numa_node_size64() function from libnuma to get + * the amount of total memory in bytes. It is then converted to + * KiB and returned. + * + * Returns 0 on failure, amount of memory in KiB on success. + */ +static unsigned long long nodeGetCellMemory(int cell) +{ + long long mem; + unsigned long long memKiB = 0; + int maxCell; + + if (numa_available() < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("NUMA not supported on this host")); + goto cleanup; + } + + /* Make sure the provided cell number is valid. */ + maxCell = numa_max_node(); + if (cell > maxCell) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cell %d out of range (0-%d)"), + cell, maxCell); + goto cleanup; + } + + /* Get the amount of memory(bytes) in the node */ + mem = numa_node_size64(cell, NULL); + if (mem < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to query NUMA total memory for node: %d"), + cell); + goto cleanup; + } + + /* Convert the memory from bytes to KiB */ + memKiB = mem >> 10; + +cleanup: + return memKiB; +} + + #else int nodeCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; @@ -1686,4 +1741,11 @@ unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED) _("NUMA memory information not available on this platform")); return 0; } + +static unsigned long long nodeGetCellMemory(int cell) +{ + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("NUMA memory information not available on this platform")); + return 0; +} #endif diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 30ce8e7..0037a6f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -184,7 +184,7 @@ testBuildCapabilities(virConnectPtr conn) { if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus, - cpu_cells) < 0) + 0, cpu_cells) < 0) goto no_memory; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 27b87fc..4995d63 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1141,7 +1141,7 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) } virBitmapFree(cpuset); - if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuInfo) < 0) + if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, 0, cpuInfo) < 0) goto memory_error; cpuInfo = NULL; } diff --git a/tests/capabilityschemadata/caps-test3.xml b/tests/capabilityschemadata/caps-test3.xml new file mode 100644 index 0000000..e6c56c5 --- /dev/null +++ b/tests/capabilityschemadata/caps-test3.xml @@ -0,0 +1,88 @@ +<capabilities> + + <host> + <uuid>35383339-3134-5553-4531-30314e394a50</uuid> + <cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <vendor>Intel</vendor> + <topology sockets='1' cores='6' threads='2'/> + <feature name='rdtscp'/> + <feature name='pdpe1gb'/> + <feature name='dca'/> + <feature name='pdcm'/> + <feature name='xtpr'/> + <feature name='tm2'/> + <feature name='est'/> + <feature name='smx'/> + <feature name='vmx'/> + <feature name='ds_cpl'/> + <feature name='monitor'/> + <feature name='dtes64'/> + <feature name='pclmuldq'/> + <feature name='pbe'/> + <feature name='tm'/> + <feature name='ht'/> + <feature name='ss'/> + <feature name='acpi'/> + <feature name='ds'/> + <feature name='vme'/> + </cpu> + <power_management> + <suspend_disk/> + </power_management> + <migration_features> + <live/> + <uri_transports> + <uri_transport>tcp</uri_transport> + </uri_transports> + </migration_features> + <topology> + <cells num='2'> + <cell id='0'> + <memory unit='KiB'>12572412</memory> + <cpus num='12'> + <cpu id='0'/> + <cpu id='2'/> + <cpu id='4'/> + <cpu id='6'/> + <cpu id='8'/> + <cpu id='10'/> + <cpu id='12'/> + <cpu id='14'/> + <cpu id='16'/> + <cpu id='18'/> + <cpu id='20'/> + <cpu id='22'/> + </cpus> + </cell> + <cell id='1'> + <memory unit='KiB'>12582908</memory> + <cpus num='12'> + <cpu id='1'/> + <cpu id='3'/> + <cpu id='5'/> + <cpu id='7'/> + <cpu id='9'/> + <cpu id='11'/> + <cpu id='13'/> + <cpu id='15'/> + <cpu id='17'/> + <cpu id='19'/> + <cpu id='21'/> + <cpu id='23'/> + </cpus> + </cell> + </cells> + </topology> + <secmodel> + <model>none</model> + <doi>0</doi> + </secmodel> + <secmodel> + <model>dac</model> + <doi>0</doi> + </secmodel> + </host> + +</capabilities> -- 1.7.11.7
On Thu, Feb 14, 2013 at 12:52:38PM -0500, Dusty Mabe wrote:
--- docs/schemas/capability.rng | 10 ++++ src/conf/capabilities.c | 8 +++ src/conf/capabilities.h | 2 + src/nodeinfo.c | 64 +++++++++++++++++++++- src/test/test_driver.c | 2 +- src/xen/xend_internal.c | 2 +- tests/capabilityschemadata/caps-test3.xml | 88 +++++++++++++++++++++++++++++++ 7 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 tests/capabilityschemadata/caps-test3.xml
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 53fb04a..e0396aa 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -177,6 +177,10 @@ </attribute>
<optional> + <ref name='memory'/> + </optional> + + <optional> <element name='cpus'> <attribute name='num'> <ref name='unsignedInt'/> @@ -189,6 +193,12 @@ </element> </define>
+ <define name='memory'> + <element name='memory'> + <ref name='scaledInteger'/> + </element> + </define> + <define name='cpu'> <element name='cpu'> <attribute name='id'> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index a0e597b..9824f0c 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -273,6 +273,7 @@ int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + unsigned long long mem, virCapsHostNUMACellCPUPtr cpus) { virCapsHostNUMACellPtr cell; @@ -286,6 +287,7 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps,
cell->ncpus = ncpus; cell->num = num; + cell->mem = mem; cell->cpus = cpus;
caps->host.numaCell[caps->host.nnumaCell++] = cell; @@ -712,6 +714,12 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells); for (i = 0; i < ncells; i++) { virBufferAsprintf(xml, " <cell id='%d'>\n", cells[i]->num); + + /* Print out the numacell memory total if it is available */ + if (cells[i]->mem) + virBufferAsprintf(xml, " <memory unit='KiB'>%llu</memory>\n", + cells[i]->mem); + virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); for (j = 0; j < cells[i]->ncpus; j++) { virBufferAsprintf(xml, " <cpu id='%d'", diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index cc01765..6c67fb3 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -99,6 +99,7 @@ typedef virCapsHostNUMACell *virCapsHostNUMACellPtr; struct _virCapsHostNUMACell { int num; int ncpus; + unsigned long long mem; /* in kibibytes */ virCapsHostNUMACellCPUPtr cpus; };
@@ -210,6 +211,7 @@ extern int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + unsigned long long mem, virCapsHostNUMACellCPUPtr cpus);
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 1622322..f9e173b 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -102,6 +102,7 @@ static int linuxNodeGetMemoryStats(FILE *meminfo, int cellNum, virNodeMemoryStatsPtr params, int *nparams); +static unsigned long long nodeGetCellMemory(int cell);
/* Return the positive decimal contents of the given * DIR/cpu%u/FILE, or -1 on error. If DEFAULT_VALUE is non-negative @@ -1531,6 +1532,7 @@ nodeCapsInitNUMA(virCapsPtr caps) int n; unsigned long *mask = NULL; unsigned long *allonesmask = NULL; + unsigned long long memory; virCapsHostNUMACellCPUPtr cpus = NULL; int ret = -1; int max_n_cpus = NUMA_MAX_N_CPUS; @@ -1562,6 +1564,11 @@ nodeCapsInitNUMA(virCapsPtr caps) continue; }
+ /* Detect the amount of memory in the numa cell */ + memory = nodeGetCellMemory(n); + if (memory == 0) + goto cleanup; + for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) if (MASK_CPU_ISSET(mask, i)) ncpus++; @@ -1578,7 +1585,7 @@ nodeCapsInitNUMA(virCapsPtr caps) } }
- if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0) + if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, memory, cpus) < 0) goto cleanup; }
@@ -1665,6 +1672,54 @@ cleanup: return freeMem; }
+/** + * nodeGetCellMemory + * @cell: The number of the numa cell to get memory info for. + * + * Will call the numa_node_size64() function from libnuma to get + * the amount of total memory in bytes. It is then converted to + * KiB and returned. + * + * Returns 0 on failure, amount of memory in KiB on success. + */ +static unsigned long long nodeGetCellMemory(int cell) +{ + long long mem; + unsigned long long memKiB = 0; + int maxCell; + + if (numa_available() < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("NUMA not supported on this host")); + goto cleanup; + }
This check isn't needed since you're calling this from nodeInitNUMA which has already done exactly this check
+ + /* Make sure the provided cell number is valid. */ + maxCell = numa_max_node(); + if (cell > maxCell) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cell %d out of range (0-%d)"), + cell, maxCell); + goto cleanup; + } + + /* Get the amount of memory(bytes) in the node */ + mem = numa_node_size64(cell, NULL); + if (mem < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to query NUMA total memory for node: %d"), + cell); + goto cleanup; + } + + /* Convert the memory from bytes to KiB */ + memKiB = mem >> 10;
What do people think about bytes vs kb for the capabilities XML. I can go either way really. Bytes is more precise, but I doubt apps will care about bytes when we're talking multiple GB of RAM.
+ +cleanup: + return memKiB; +} + +
ACK to the patch with the small change made. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Mar 6, 2013 at 5:07 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Thu, Feb 14, 2013 at 12:52:38PM -0500, Dusty Mabe wrote:
+ if (numa_available() < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("NUMA not supported on this host")); + goto cleanup; + }
This check isn't needed since you're calling this from nodeInitNUMA which has already done exactly this check
Dan, Thanks for the comments! I have removed this check from the code. Should I generate a new patch email with the finalized code? I assume that I should do a git pull --rebase first. Please let me know if that is not the case. Dusty
On Thu, Mar 07, 2013 at 12:13:30AM -0500, Dusty Mabe wrote:
On Wed, Mar 6, 2013 at 5:07 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Thu, Feb 14, 2013 at 12:52:38PM -0500, Dusty Mabe wrote:
+ if (numa_available() < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("NUMA not supported on this host")); + goto cleanup; + }
This check isn't needed since you're calling this from nodeInitNUMA which has already done exactly this check
Thanks for the comments! I have removed this check from the code. Should I generate a new patch email with the finalized code? I assume that I should do a git pull --rebase first. Please let me know if that is not the case.
Yep, just rebase to latest git master & repost. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--- docs/schemas/capability.rng | 10 ++++ src/conf/capabilities.c | 8 +++ src/conf/capabilities.h | 2 + src/nodeinfo.c | 58 +++++++++++++++++++- src/test/test_driver.c | 2 +- src/xen/xend_internal.c | 2 +- tests/capabilityschemadata/caps-test3.xml | 88 +++++++++++++++++++++++++++++++ 7 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/capabilityschemadata/caps-test3.xml diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 8f3fbd9..b479070 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -177,6 +177,10 @@ </attribute> <optional> + <ref name='memory'/> + </optional> + + <optional> <element name='cpus'> <attribute name='num'> <ref name='unsignedInt'/> @@ -189,6 +193,12 @@ </element> </define> + <define name='memory'> + <element name='memory'> + <ref name='scaledInteger'/> + </element> + </define> + <define name='cpu'> <element name='cpu'> <attribute name='id'> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index a0e597b..9824f0c 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -273,6 +273,7 @@ int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + unsigned long long mem, virCapsHostNUMACellCPUPtr cpus) { virCapsHostNUMACellPtr cell; @@ -286,6 +287,7 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps, cell->ncpus = ncpus; cell->num = num; + cell->mem = mem; cell->cpus = cpus; caps->host.numaCell[caps->host.nnumaCell++] = cell; @@ -712,6 +714,12 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells); for (i = 0; i < ncells; i++) { virBufferAsprintf(xml, " <cell id='%d'>\n", cells[i]->num); + + /* Print out the numacell memory total if it is available */ + if (cells[i]->mem) + virBufferAsprintf(xml, " <memory unit='KiB'>%llu</memory>\n", + cells[i]->mem); + virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); for (j = 0; j < cells[i]->ncpus; j++) { virBufferAsprintf(xml, " <cpu id='%d'", diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index cc01765..6c67fb3 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -99,6 +99,7 @@ typedef virCapsHostNUMACell *virCapsHostNUMACellPtr; struct _virCapsHostNUMACell { int num; int ncpus; + unsigned long long mem; /* in kibibytes */ virCapsHostNUMACellCPUPtr cpus; }; @@ -210,6 +211,7 @@ extern int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, + unsigned long long mem, virCapsHostNUMACellCPUPtr cpus); diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 1622322..a75f73f 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -102,6 +102,7 @@ static int linuxNodeGetMemoryStats(FILE *meminfo, int cellNum, virNodeMemoryStatsPtr params, int *nparams); +static unsigned long long nodeGetCellMemory(int cell); /* Return the positive decimal contents of the given * DIR/cpu%u/FILE, or -1 on error. If DEFAULT_VALUE is non-negative @@ -1531,6 +1532,7 @@ nodeCapsInitNUMA(virCapsPtr caps) int n; unsigned long *mask = NULL; unsigned long *allonesmask = NULL; + unsigned long long memory; virCapsHostNUMACellCPUPtr cpus = NULL; int ret = -1; int max_n_cpus = NUMA_MAX_N_CPUS; @@ -1562,6 +1564,11 @@ nodeCapsInitNUMA(virCapsPtr caps) continue; } + /* Detect the amount of memory in the numa cell */ + memory = nodeGetCellMemory(n); + if (memory == 0) + goto cleanup; + for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) if (MASK_CPU_ISSET(mask, i)) ncpus++; @@ -1578,7 +1585,7 @@ nodeCapsInitNUMA(virCapsPtr caps) } } - if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0) + if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, memory, cpus) < 0) goto cleanup; } @@ -1665,6 +1672,48 @@ cleanup: return freeMem; } +/** + * nodeGetCellMemory + * @cell: The number of the numa cell to get memory info for. + * + * Will call the numa_node_size64() function from libnuma to get + * the amount of total memory in bytes. It is then converted to + * KiB and returned. + * + * Returns 0 on failure, amount of memory in KiB on success. + */ +static unsigned long long nodeGetCellMemory(int cell) +{ + long long mem; + unsigned long long memKiB = 0; + int maxCell; + + /* Make sure the provided cell number is valid. */ + maxCell = numa_max_node(); + if (cell > maxCell) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cell %d out of range (0-%d)"), + cell, maxCell); + goto cleanup; + } + + /* Get the amount of memory(bytes) in the node */ + mem = numa_node_size64(cell, NULL); + if (mem < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to query NUMA total memory for node: %d"), + cell); + goto cleanup; + } + + /* Convert the memory from bytes to KiB */ + memKiB = mem >> 10; + +cleanup: + return memKiB; +} + + #else int nodeCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; @@ -1686,4 +1735,11 @@ unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED) _("NUMA memory information not available on this platform")); return 0; } + +static unsigned long long nodeGetCellMemory(int cell) +{ + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("NUMA memory information not available on this platform")); + return 0; +} #endif diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 30ce8e7..0037a6f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -184,7 +184,7 @@ testBuildCapabilities(virConnectPtr conn) { if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus, - cpu_cells) < 0) + 0, cpu_cells) < 0) goto no_memory; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 27b87fc..4995d63 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1141,7 +1141,7 @@ sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) } virBitmapFree(cpuset); - if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuInfo) < 0) + if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, 0, cpuInfo) < 0) goto memory_error; cpuInfo = NULL; } diff --git a/tests/capabilityschemadata/caps-test3.xml b/tests/capabilityschemadata/caps-test3.xml new file mode 100644 index 0000000..e6c56c5 --- /dev/null +++ b/tests/capabilityschemadata/caps-test3.xml @@ -0,0 +1,88 @@ +<capabilities> + + <host> + <uuid>35383339-3134-5553-4531-30314e394a50</uuid> + <cpu> + <arch>x86_64</arch> + <model>Westmere</model> + <vendor>Intel</vendor> + <topology sockets='1' cores='6' threads='2'/> + <feature name='rdtscp'/> + <feature name='pdpe1gb'/> + <feature name='dca'/> + <feature name='pdcm'/> + <feature name='xtpr'/> + <feature name='tm2'/> + <feature name='est'/> + <feature name='smx'/> + <feature name='vmx'/> + <feature name='ds_cpl'/> + <feature name='monitor'/> + <feature name='dtes64'/> + <feature name='pclmuldq'/> + <feature name='pbe'/> + <feature name='tm'/> + <feature name='ht'/> + <feature name='ss'/> + <feature name='acpi'/> + <feature name='ds'/> + <feature name='vme'/> + </cpu> + <power_management> + <suspend_disk/> + </power_management> + <migration_features> + <live/> + <uri_transports> + <uri_transport>tcp</uri_transport> + </uri_transports> + </migration_features> + <topology> + <cells num='2'> + <cell id='0'> + <memory unit='KiB'>12572412</memory> + <cpus num='12'> + <cpu id='0'/> + <cpu id='2'/> + <cpu id='4'/> + <cpu id='6'/> + <cpu id='8'/> + <cpu id='10'/> + <cpu id='12'/> + <cpu id='14'/> + <cpu id='16'/> + <cpu id='18'/> + <cpu id='20'/> + <cpu id='22'/> + </cpus> + </cell> + <cell id='1'> + <memory unit='KiB'>12582908</memory> + <cpus num='12'> + <cpu id='1'/> + <cpu id='3'/> + <cpu id='5'/> + <cpu id='7'/> + <cpu id='9'/> + <cpu id='11'/> + <cpu id='13'/> + <cpu id='15'/> + <cpu id='17'/> + <cpu id='19'/> + <cpu id='21'/> + <cpu id='23'/> + </cpus> + </cell> + </cells> + </topology> + <secmodel> + <model>none</model> + <doi>0</doi> + </secmodel> + <secmodel> + <model>dac</model> + <doi>0</doi> + </secmodel> + </host> + +</capabilities> -- 1.7.11.7
On Thu, Mar 7, 2013 at 11:03 AM, Dusty Mabe <dustymabe@gmail.com> wrote:
--- docs/schemas/capability.rng | 10 ++++ src/conf/capabilities.c | 8 +++ src/conf/capabilities.h | 2 + src/nodeinfo.c | 58 +++++++++++++++++++- src/test/test_driver.c | 2 +- src/xen/xend_internal.c | 2 +- tests/capabilityschemadata/caps-test3.xml | 88 +++++++++++++++++++++++++++++++ 7 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/capabilityschemadata/caps-test3.xml
Let me know if there is anything more I need to do for this! Thanks for all of your help, Dusty
On 03/07/2013 09:03 AM, Dusty Mabe wrote:
---
The commit message should mention the XML changes being made. I'm modifying it to capture this snippet of your testcase: <topology> <cells num='2'> <cell id='0'> <memory unit='KiB'>12572412</memory> <cpus num='12'> ...
docs/schemas/capability.rng | 10 ++++ src/conf/capabilities.c | 8 +++ src/conf/capabilities.h | 2 + src/nodeinfo.c | 58 +++++++++++++++++++- src/test/test_driver.c | 2 +- src/xen/xend_internal.c | 2 +- tests/capabilityschemadata/caps-test3.xml | 88 +++++++++++++++++++++++++++++++ 7 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/capabilityschemadata/caps-test3.xml
+ <define name='memory'> + <element name='memory'> + <ref name='scaledInteger'/>
Indentation is a bit off (not that it affects correct behavior).
@@ -712,6 +714,12 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells); for (i = 0; i < ncells; i++) { virBufferAsprintf(xml, " <cell id='%d'>\n", cells[i]->num); + + /* Print out the numacell memory total if it is available */ + if (cells[i]->mem) + virBufferAsprintf(xml, " <memory unit='KiB'>%llu</memory>\n",
Indentation is off.
+++ b/src/nodeinfo.c @@ -102,6 +102,7 @@ static int linuxNodeGetMemoryStats(FILE *meminfo, int cellNum, virNodeMemoryStatsPtr params, int *nparams); +static unsigned long long nodeGetCellMemory(int cell);
Generally, a forward declaration of a static function indicates that the file is not topologically sorted correctly. But given that the two implementations are inside #ifdef, and you were copying from other examples, it's not worth changing.
@@ -1562,6 +1564,11 @@ nodeCapsInitNUMA(virCapsPtr caps) continue; }
+ /* Detect the amount of memory in the numa cell */ + memory = nodeGetCellMemory(n); + if (memory == 0) + goto cleanup;
Why give up on the rest of useful information? This should only go to cleanup on failure, not on lack of information, so that the cpu capabilities aren't lost (if there IS a kernel where memory information cannot be obtained, we don't want to regress and output less information than before). The alternative is to have nodeGetCellMemory take an unsigned long long * argument, and have an int return (0 for success, including unknown; -1 for complete failure which warrants ditching any capability return); but I thought that was overkill since we already special-cased the xml output to skip memory if it was 0. ACK once this is squashed in, so I pushed it. Thanks again for the patch, and for persisting with addressing our feedback, even if it took a while. diff --git i/docs/schemas/capability.rng w/docs/schemas/capability.rng index b479070..106ca73 100644 --- i/docs/schemas/capability.rng +++ w/docs/schemas/capability.rng @@ -195,7 +195,7 @@ <define name='memory'> <element name='memory'> - <ref name='scaledInteger'/> + <ref name='scaledInteger'/> </element> </define> diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c index 9824f0c..d53d5a3 100644 --- i/src/conf/capabilities.c +++ w/src/conf/capabilities.c @@ -1,7 +1,7 @@ /* * capabilities.c: hypervisor capabilities * - * Copyright (C) 2006-2008, 2010-2011 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2011, 2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -717,8 +717,9 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, /* Print out the numacell memory total if it is available */ if (cells[i]->mem) - virBufferAsprintf(xml, " <memory unit='KiB'>%llu</memory>\n", - cells[i]->mem); + virBufferAsprintf(xml, + " <memory unit='KiB'>%llu</memory>\n", + cells[i]->mem); virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); for (j = 0; j < cells[i]->ncpus; j++) { diff --git i/src/nodeinfo.c w/src/nodeinfo.c index a75f73f..b80e389 100644 --- i/src/nodeinfo.c +++ w/src/nodeinfo.c @@ -1,7 +1,7 @@ /* * nodeinfo.c: Helper routines for OS specific node information * - * Copyright (C) 2006-2008, 2010-2012 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1566,8 +1566,6 @@ nodeCapsInitNUMA(virCapsPtr caps) /* Detect the amount of memory in the numa cell */ memory = nodeGetCellMemory(n); - if (memory == 0) - goto cleanup; for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) if (MASK_CPU_ISSET(mask, i)) @@ -1680,7 +1678,7 @@ cleanup: * the amount of total memory in bytes. It is then converted to * KiB and returned. * - * Returns 0 on failure, amount of memory in KiB on success. + * Returns 0 if unavailable, amount of memory in KiB on success. */ static unsigned long long nodeGetCellMemory(int cell) { -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Dusty Mabe -
Eric Blake