--- 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

--- 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 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'> ... Indentation is a bit off (not that it affects correct behavior). Indentation is off. 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. 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