On Fri, Oct 18, 2013 at 05:05:41PM +0200, Peter Krempa wrote: I think it would be desirable to crate a nodeinfopriv.h header for this, since it isn't intended to be part of the internal API. 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 :|

All functions from libnuma must be protected with ifdefs. Avoid this by using our own wrapper. --- src/libvirt_private.syms | 1 + src/nodeinfo.c | 13 +++++-------- src/util/virnuma.c | 14 ++++++++++++++ src/util/virnuma.h | 2 ++ 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 84c1c28..48d3da7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1520,6 +1520,7 @@ virNodeSuspendGetTargetMask; virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; virNumaGetAutoPlacementAdvice; +virNumaIsAvailable; virNumaSetupMemoryPolicy; virNumaTuneMemPlacementModeTypeFromString; virNumaTuneMemPlacementModeTypeToString; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 232b465..d4bb792 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -55,6 +55,7 @@ #include "virfile.h" #include "virtypedparam.h" #include "virstring.h" +#include "virnuma.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -977,15 +978,11 @@ int nodeGetMemoryStats(int cellNum ATTRIBUTE_UNUSED, if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0) return -1; } else { -# if WITH_NUMACTL - if (numa_available() < 0) { -# endif + if (!virNumaIsAvailable()) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("NUMA not supported on this host")); return -1; -# if WITH_NUMACTL } -# endif # if WITH_NUMACTL if (cellNum > numa_max_node()) { @@ -1600,7 +1597,7 @@ nodeCapsInitNUMA(virCapsPtr caps) int ncpus = 0; bool topology_failed = false; - if (numa_available() < 0) + if (!virNumaIsAvailable()) return nodeCapsInitNUMAFake(caps); int mask_n_bytes = max_n_cpus / 8; @@ -1672,7 +1669,7 @@ nodeGetCellsFreeMemory(unsigned long long *freeMems, int ret = -1; int maxCell; - if (numa_available() < 0) + if (!virNumaIsAvailable()) return nodeGetCellsFreeMemoryFake(freeMems, startCell, maxCells); @@ -1706,7 +1703,7 @@ nodeGetFreeMemory(void) unsigned long long freeMem = 0; int n; - if (numa_available() < 0) + if (!virNumaIsAvailable()) return nodeGetFreeMemoryFake(); diff --git a/src/util/virnuma.c b/src/util/virnuma.c index ecf7ede..11078a1 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -167,6 +167,13 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune, cleanup: return ret; } + + +bool +virNumaIsAvailable(void) +{ + return numa_available() != -1; +} #else int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, @@ -181,4 +188,11 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune, return 0; } + + +bool +virNumaIsAvailable(void) +{ + return false; +} #endif diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 9ff8e69..5f9c38b 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -55,4 +55,6 @@ char *virNumaGetAutoPlacementAdvice(unsigned short vcups, int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, virBitmapPtr nodemask); + +bool virNumaIsAvailable(void); #endif /* __VIR_NUMA_H__ */ -- 1.8.3.2

linuxNodeGetCPUStats() and linuxNodeGetMemoryStats() are static and don't need a forward declaration. --- src/nodeinfo.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index d4bb792..803856b 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -89,15 +89,6 @@ appleFreebsdNodeGetCPUCount(void) # define LINUX_NB_MEMORY_STATS_ALL 4 # define LINUX_NB_MEMORY_STATS_CELL 2 -static int linuxNodeGetCPUStats(FILE *procstat, - int cpuNum, - virNodeCPUStatsPtr params, - int *nparams); -static int linuxNodeGetMemoryStats(FILE *meminfo, - int cellNum, - virNodeMemoryStatsPtr params, - int *nparams); - /* Return the positive decimal contents of the given * DIR/cpu%u/FILE, or -1 on error. If DEFAULT_VALUE is non-negative * and the file could not be found, return that instead of an error; @@ -586,10 +577,11 @@ cleanup: # define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / sysconf(_SC_CLK_TCK)) -int linuxNodeGetCPUStats(FILE *procstat, - int cpuNum, - virNodeCPUStatsPtr params, - int *nparams) +static int +linuxNodeGetCPUStats(FILE *procstat, + int cpuNum, + virNodeCPUStatsPtr params, + int *nparams) { int ret = -1; char line[1024]; @@ -689,10 +681,11 @@ cleanup: return ret; } -int linuxNodeGetMemoryStats(FILE *meminfo, - int cellNum, - virNodeMemoryStatsPtr params, - int *nparams) +static int +linuxNodeGetMemoryStats(FILE *meminfo, + int cellNum, + virNodeMemoryStatsPtr params, + int *nparams) { int ret = -1; size_t i = 0, j = 0, k = 0; -- 1.8.3.2

Avoid necessary checks for the numa library with this helper. --- src/libvirt_private.syms | 1 + src/nodeinfo.c | 37 ++++++++++++++++++++++--------------- src/util/virnuma.c | 37 +++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 2 ++ 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 48d3da7..c8959ad 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1520,6 +1520,7 @@ virNodeSuspendGetTargetMask; virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; virNumaGetAutoPlacementAdvice; +virNumaGetMaxNode; virNumaIsAvailable; virNumaSetupMemoryPolicy; virNumaTuneMemPlacementModeTypeFromString; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 803856b..27cd502 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -966,25 +966,21 @@ int nodeGetMemoryStats(int cellNum ATTRIBUTE_UNUSED, int ret; char *meminfo_path = NULL; FILE *meminfo; + int max_node; if (cellNum == VIR_NODE_MEMORY_STATS_ALL_CELLS) { if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0) return -1; } else { - if (!virNumaIsAvailable()) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("NUMA not supported on this host")); + if ((max_node = virNumaGetMaxNode()) < 0) return -1; - } -# if WITH_NUMACTL - if (cellNum > numa_max_node()) { + if (cellNum > max_node) { virReportInvalidArg(cellNum, _("cellNum in %s must be less than or equal to %d"), - __FUNCTION__, numa_max_node()); + __FUNCTION__, max_node); return -1; } -# endif if (virAsprintf(&meminfo_path, "%s/node/node%d/meminfo", SYSFS_SYSTEM_PATH, cellNum) < 0) @@ -1587,31 +1583,35 @@ nodeCapsInitNUMA(virCapsPtr caps) virCapsHostNUMACellCPUPtr cpus = NULL; int ret = -1; int max_n_cpus = NUMA_MAX_N_CPUS; + int mask_n_bytes = max_n_cpus / 8; int ncpus = 0; bool topology_failed = false; + int max_node; if (!virNumaIsAvailable()) return nodeCapsInitNUMAFake(caps); - int mask_n_bytes = max_n_cpus / 8; + if ((max_node = virNumaGetMaxNode()) < 0) + goto cleanup; + if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof(*mask)) < 0) goto cleanup; if (VIR_ALLOC_N(allonesmask, mask_n_bytes / sizeof(*mask)) < 0) goto cleanup; memset(allonesmask, 0xff, mask_n_bytes); - for (n = 0; n <= numa_max_node(); n++) { + for (n = 0; n <= max_node; n++) { size_t i; /* The first time this returns -1, ENOENT if node doesn't exist... */ if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0) { VIR_WARN("NUMA topology for cell %d of %d not available, ignoring", - n, numa_max_node()+1); + n, max_node + 1); continue; } /* second, third... times it returns an all-1's mask */ if (memcmp(mask, allonesmask, mask_n_bytes) == 0) { VIR_DEBUG("NUMA topology for cell %d of %d is all ones, ignoring", - n, numa_max_node()+1); + n, max_node + 1); continue; } @@ -1666,7 +1666,9 @@ nodeGetCellsFreeMemory(unsigned long long *freeMems, return nodeGetCellsFreeMemoryFake(freeMems, startCell, maxCells); - maxCell = numa_max_node(); + if ((maxCell = virNumaGetMaxNode()) < 0) + return 0; + if (startCell > maxCell) { virReportError(VIR_ERR_INTERNAL_ERROR, _("start cell %d out of range (0-%d)"), @@ -1694,13 +1696,16 @@ unsigned long long nodeGetFreeMemory(void) { unsigned long long freeMem = 0; + int max_node; int n; if (!virNumaIsAvailable()) return nodeGetFreeMemoryFake(); + if ((max_node = virNumaGetMaxNode()) < 0) + return 0; - for (n = 0; n <= numa_max_node(); n++) { + for (n = 0; n <= max_node; n++) { long long mem; if (numa_node_size64(n, &mem) < 0) continue; @@ -1728,7 +1733,9 @@ static unsigned long long nodeGetCellMemory(int cell) int maxCell; /* Make sure the provided cell number is valid. */ - maxCell = numa_max_node(); + if ((maxCell = virNumaGetMaxNode()) < 0) + return 0; + if (cell > maxCell) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cell %d out of range (0-%d)"), diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 11078a1..2d8d0e9 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -174,6 +174,34 @@ virNumaIsAvailable(void) { return numa_available() != -1; } + + +/** + * virNumaGetMaxNode: + * Get the highest node number available on the current system. + * (See the node numbers in /sys/devices/system/node/ ). + * + * Returns the highes NUMA node id on success, -1 on error. + */ +int +virNumaGetMaxNode(void) +{ + int ret; + + if (!virNumaIsAvailable()) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NUMA isn't available on this host")); + return -1; + } + + if ((ret = numa_max_node()) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to request maximum NUMA node id")); + return -1; + } + + return ret; +} #else int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, @@ -195,4 +223,13 @@ virNumaIsAvailable(void) { return false; } + + +int +virNumaGetMaxNode(void) +{ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NUMA isn't available on this host")); + return -1; +} #endif diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 5f9c38b..65636c3 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -57,4 +57,6 @@ int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, virBitmapPtr nodemask); bool virNumaIsAvailable(void); +int virNumaGetMaxNode(void); + #endif /* __VIR_NUMA_H__ */ -- 1.8.3.2

--- src/libvirt_private.syms | 1 + src/nodeinfo.c | 21 +++++++----------- src/util/virnuma.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 3 +++ 4 files changed, 67 insertions(+), 13 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c8959ad..d7243a9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1521,6 +1521,7 @@ virDomainNumatuneMemModeTypeFromString; virDomainNumatuneMemModeTypeToString; virNumaGetAutoPlacementAdvice; virNumaGetMaxNode; +virNumaGetNodeMemory; virNumaIsAvailable; virNumaSetupMemoryPolicy; virNumaTuneMemPlacementModeTypeFromString; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 27cd502..ba9fd57 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1658,6 +1658,7 @@ nodeGetCellsFreeMemory(unsigned long long *freeMems, int startCell, int maxCells) { + unsigned long long mem; int n, lastCell, numCells; int ret = -1; int maxCell; @@ -1680,9 +1681,7 @@ nodeGetCellsFreeMemory(unsigned long long *freeMems, lastCell = maxCell; for (numCells = 0, n = startCell; n <= lastCell; n++) { - long long mem; - if (numa_node_size64(n, &mem) < 0) - mem = 0; + virNumaGetNodeMemory(n, NULL, &mem); freeMems[numCells++] = mem; } @@ -1695,6 +1694,7 @@ cleanup: unsigned long long nodeGetFreeMemory(void) { + unsigned long long mem; unsigned long long freeMem = 0; int max_node; int n; @@ -1706,9 +1706,7 @@ nodeGetFreeMemory(void) return 0; for (n = 0; n <= max_node; n++) { - long long mem; - if (numa_node_size64(n, &mem) < 0) - continue; + virNumaGetNodeMemory(n, NULL, &mem); freeMem += mem; } @@ -1720,21 +1718,19 @@ nodeGetFreeMemory(void) * 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. + * Request size of memory in a NUMA node. * * Returns 0 if unavailable, amount of memory in KiB on success. */ static unsigned long long nodeGetCellMemory(int cell) { - long long mem; + unsigned long long mem; unsigned long long memKiB = 0; int maxCell; /* Make sure the provided cell number is valid. */ if ((maxCell = virNumaGetMaxNode()) < 0) - return 0; + goto cleanup; if (cell > maxCell) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1744,8 +1740,7 @@ static unsigned long long nodeGetCellMemory(int cell) } /* Get the amount of memory(bytes) in the node */ - mem = numa_node_size64(cell, NULL); - if (mem < 0) { + if (virNumaGetNodeMemory(cell, &mem, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to query NUMA total memory for node: %d"), cell); diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 2d8d0e9..15a18e9 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -202,6 +202,45 @@ virNumaGetMaxNode(void) return ret; } + + +/** + * virNumaGetNodeMemorySize: + * @node: identifier of the requested NUMA node + * @memsize: returns the total size of memory in the NUMA node + * @memfree: returns the total free memory in a NUMA node + * + * Returns the size of the memory in one NUMA node in bytes via the @size + * argument and free memory of a node in the @free argument. The caller has to + * guarantee that @node is in range (see virNumaGetMaxNode). + * + * Returns 0 on success, -1 on error. Does not report errors. + */ +int +virNumaGetNodeMemory(int node, + unsigned long long *memsize, + unsigned long long *memfree) +{ + long long node_size; + long long node_free; + + if (memsize) + *memsize = 0; + + if (memfree) + *memfree = 0; + + if ((node_size = numa_node_size64(node, &node_free)) < 0) + return -1; + + if (memsize) + *memsize = node_size; + + if (memfree) + *memfree = node_free; + + return 0; +} #else int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, @@ -232,4 +271,20 @@ virNumaGetMaxNode(void) _("NUMA isn't available on this host")); return -1; } + + +int +virNumaGetNodeMemory(int node ATTRIBUTE_UNUSED, + unsigned long long *memsize, + unsigned long long *memfree) +{ + if (memsize) + *memsize = 0; + + if (memfree) + *memfree = 0; + + VIR_DEBUG("NUMA isn't available on this host"); + return -1; +} #endif diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 65636c3..7640d1b 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -58,5 +58,8 @@ int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, bool virNumaIsAvailable(void); int virNumaGetMaxNode(void); +int virNumaGetNodeMemory(int node, + unsigned long long *memsize, + unsigned long long *memfree); #endif /* __VIR_NUMA_H__ */ -- 1.8.3.2

The function was called in a single place only and was reporting errors that were later ignored. Use the virNumaGetNodeMemory helper to get the size of the memory in the NUMA node and remove the helper --- src/nodeinfo.c | 47 +++-------------------------------------------- 1 file changed, 3 insertions(+), 44 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index ba9fd57..c73eee4 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1523,8 +1523,6 @@ nodeGetFreeMemoryFake(void) # define MASK_CPU_ISSET(mask, cpu) \ (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1) -static unsigned long long nodeGetCellMemory(int cell); - static virBitmapPtr virNodeGetSiblingsList(const char *dir, int cpu_id) { @@ -1615,8 +1613,9 @@ nodeCapsInitNUMA(virCapsPtr caps) continue; } - /* Detect the amount of memory in the numa cell */ - memory = nodeGetCellMemory(n); + /* Detect the amount of memory in the numa cell in KiB */ + virNumaGetNodeMemory(n, &memory, NULL); + memory >>= 10; for (ncpus = 0, i = 0; i < max_n_cpus; i++) if (MASK_CPU_ISSET(mask, i)) @@ -1714,46 +1713,6 @@ nodeGetFreeMemory(void) return freeMem; } -/** - * nodeGetCellMemory - * @cell: The number of the numa cell to get memory info for. - * - * Request size of memory in a NUMA node. - * - * Returns 0 if unavailable, amount of memory in KiB on success. - */ -static unsigned long long nodeGetCellMemory(int cell) -{ - unsigned long long mem; - unsigned long long memKiB = 0; - int maxCell; - - /* Make sure the provided cell number is valid. */ - if ((maxCell = virNumaGetMaxNode()) < 0) - goto cleanup; - - 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 */ - if (virNumaGetNodeMemory(cell, &mem, NULL) < 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) { -- 1.8.3.2

--- src/nodeinfo.c | 9 ++------- src/util/virnuma.c | 24 +++++++++++++++++++++++- src/util/virnuma.h | 2 ++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index c73eee4..4f0d412 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1513,11 +1513,6 @@ nodeGetFreeMemoryFake(void) } #if WITH_NUMACTL -# if LIBNUMA_API_VERSION <= 1 -# define NUMA_MAX_N_CPUS 4096 -# else -# define NUMA_MAX_N_CPUS (numa_all_cpus_ptr->size) -# endif # define n_bits(var) (8 * sizeof(var)) # define MASK_CPU_ISSET(mask, cpu) \ @@ -1537,7 +1532,7 @@ virNodeGetSiblingsList(const char *dir, int cpu_id) if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGTH_MAX, &buf) < 0) goto cleanup; - if (virBitmapParse(buf, 0, &ret, NUMA_MAX_N_CPUS) < 0) + if (virBitmapParse(buf, 0, &ret, virNumaGetMaxCPUs()) < 0) goto cleanup; cleanup: @@ -1580,7 +1575,7 @@ nodeCapsInitNUMA(virCapsPtr caps) unsigned long long memory; virCapsHostNUMACellCPUPtr cpus = NULL; int ret = -1; - int max_n_cpus = NUMA_MAX_N_CPUS; + int max_n_cpus = virNumaGetMaxCPUs(); int mask_n_bytes = max_n_cpus / 8; int ncpus = 0; bool topology_failed = false; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 15a18e9..0530d64 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -21,10 +21,18 @@ #include <config.h> +#define NUMA_MAX_N_CPUS 4096 + #if WITH_NUMACTL # define NUMA_VERSION1_COMPATIBILITY 1 # include <numa.h> -#endif + +# if LIBNUMA_API_VERSION > 1 +# undef NUMA_MAX_N_CPUS +# define NUMA_MAX_N_CPUS (numa_all_cpus_ptr->size) +# endif + +#endif /* WITH_NUMACTL */ #include "virnuma.h" #include "vircommand.h" @@ -288,3 +296,17 @@ virNumaGetNodeMemory(int node ATTRIBUTE_UNUSED, return -1; } #endif + + +/** + * virNumaGetMaxCPUs: + * + * Get the maximum count of CPUs supportable in the host. + * + * Returns the count of CPUs supported. + */ +unsigned int +virNumaGetMaxCPUs(void) +{ + return NUMA_MAX_N_CPUS; +} diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 7640d1b..62b2c0a 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -62,4 +62,6 @@ int virNumaGetNodeMemory(int node, unsigned long long *memsize, unsigned long long *memfree); +unsigned int virNumaGetMaxCPUs(void); + #endif /* __VIR_NUMA_H__ */ -- 1.8.3.2

--- src/conf/capabilities.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1acc936..8d53370 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -263,6 +263,7 @@ virCapabilitiesAddHostMigrateTransport(virCapsPtr caps, * @caps: capabilities to extend * @num: ID number of NUMA cell * @ncpus: number of CPUs in cell + * @mem: Total size of memory in the NUMA node (in KiB) * @cpus: array of CPU definition structures, the pointer is stolen * * Registers a new NUMA cell for a host, passing in a -- 1.8.3.2

--- src/nodeinfo.c | 62 ++++++++++++++----------------------- src/util/virnuma.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 2 ++ 3 files changed, 117 insertions(+), 38 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 4f0d412..7e75611 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1513,11 +1513,6 @@ nodeGetFreeMemoryFake(void) } #if WITH_NUMACTL - -# define n_bits(var) (8 * sizeof(var)) -# define MASK_CPU_ISSET(mask, cpu) \ - (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1) - static virBitmapPtr virNodeGetSiblingsList(const char *dir, int cpu_id) { @@ -1570,14 +1565,12 @@ int nodeCapsInitNUMA(virCapsPtr caps) { int n; - unsigned long *mask = NULL; - unsigned long *allonesmask = NULL; unsigned long long memory; virCapsHostNUMACellCPUPtr cpus = NULL; + virBitmapPtr cpumap = NULL; int ret = -1; - int max_n_cpus = virNumaGetMaxCPUs(); - int mask_n_bytes = max_n_cpus / 8; int ncpus = 0; + int cpu; bool topology_failed = false; int max_node; @@ -1587,49 +1580,41 @@ nodeCapsInitNUMA(virCapsPtr caps) if ((max_node = virNumaGetMaxNode()) < 0) goto cleanup; - if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof(*mask)) < 0) - goto cleanup; - if (VIR_ALLOC_N(allonesmask, mask_n_bytes / sizeof(*mask)) < 0) - goto cleanup; - memset(allonesmask, 0xff, mask_n_bytes); - for (n = 0; n <= max_node; n++) { size_t i; - /* The first time this returns -1, ENOENT if node doesn't exist... */ - if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0) { - VIR_WARN("NUMA topology for cell %d of %d not available, ignoring", - n, max_node + 1); - continue; - } - /* second, third... times it returns an all-1's mask */ - if (memcmp(mask, allonesmask, mask_n_bytes) == 0) { - VIR_DEBUG("NUMA topology for cell %d of %d is all ones, ignoring", - n, max_node + 1); - continue; - } - /* Detect the amount of memory in the numa cell in KiB */ - virNumaGetNodeMemory(n, &memory, NULL); - memory >>= 10; + if ((ncpus = virNumaGetNodeCPUs(n, &cpumap)) < 0) { + if (ncpus == -2) + continue; - for (ncpus = 0, i = 0; i < max_n_cpus; i++) - if (MASK_CPU_ISSET(mask, i)) - ncpus++; + goto cleanup; + } if (VIR_ALLOC_N(cpus, ncpus) < 0) goto cleanup; + cpu = 0; - for (ncpus = 0, i = 0; i < max_n_cpus; i++) { - if (MASK_CPU_ISSET(mask, i)) { - if (virNodeCapsFillCPUInfo(i, cpus + ncpus++) < 0) { + for (i = 0; i < virBitmapSize(cpumap); i++) { + bool cpustate; + if (virBitmapGetBit(cpumap, i, &cpustate) < 0) + continue; + + if (cpustate) { + if (virNodeCapsFillCPUInfo(i, cpus + cpu++) < 0) { topology_failed = true; virResetLastError(); } } } + /* Detect the amount of memory in the numa cell in KiB */ + virNumaGetNodeMemory(n, &memory, NULL); + memory >>= 10; + if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, memory, cpus) < 0) goto cleanup; + + cpus = NULL; } ret = 0; @@ -1638,11 +1623,12 @@ cleanup: if (topology_failed || ret < 0) virCapabilitiesClearHostNUMACellCPUTopology(cpus, ncpus); + virBitmapFree(cpumap); + VIR_FREE(cpus); + if (ret < 0) VIR_FREE(cpus); - VIR_FREE(mask); - VIR_FREE(allonesmask); return ret; } diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 0530d64..ab46591 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -38,6 +38,8 @@ #include "vircommand.h" #include "virerror.h" #include "virlog.h" +#include "viralloc.h" +#include "virbitmap.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -249,6 +251,83 @@ virNumaGetNodeMemory(int node, return 0; } + + +/** + * virNumaGetNodeCPUs: + * @node: identifier of the requested NUMA node + * @cpus: returns a bitmap of CPUs in @node + * + * Returns count of CPUs in the selected node and sets the map of the cpus to + * @cpus. On error if the @node doesn't exist in the system this function + * returns -2 and sets @cpus to NULL. On other errors -1 is returned, @cpus + * is set to NULL and an error is reported. + */ + +# define n_bits(var) (8 * sizeof(var)) +# define MASK_CPU_ISSET(mask, cpu) \ + (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1) +int +virNumaGetNodeCPUs(int node, + virBitmapPtr *cpus) +{ + unsigned long *mask = NULL; + unsigned long *allonesmask = NULL; + virBitmapPtr cpumap = NULL; + int ncpus = 0; + int max_n_cpus = virNumaGetMaxCPUs(); + int mask_n_bytes = max_n_cpus / 8; + size_t i; + int ret = -1; + + *cpus = NULL; + + if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof(*mask)) < 0) + goto cleanup; + + if (VIR_ALLOC_N(allonesmask, mask_n_bytes / sizeof(*mask)) < 0) + goto cleanup; + + memset(allonesmask, 0xff, mask_n_bytes); + + /* The first time this returns -1, ENOENT if node doesn't exist... */ + if (numa_node_to_cpus(node, mask, mask_n_bytes) < 0) { + VIR_WARN("NUMA topology for cell %d is not available, ignoring", node); + ret = -2; + goto cleanup; + } + + /* second, third... times it returns an all-1's mask */ + if (memcmp(mask, allonesmask, mask_n_bytes) == 0) { + VIR_DEBUG("NUMA topology for cell %d is invalid, ignoring", node); + ret = -2; + goto cleanup; + } + + if (!(cpumap = virBitmapNew(max_n_cpus))) + goto cleanup; + + for (i = 0; i < max_n_cpus; i++) { + if (MASK_CPU_ISSET(mask, i)) { + ignore_value(virBitmapSetBit(cpumap, i)); + ncpus++; + } + } + + *cpus = cpumap; + cpumap = NULL; + ret = ncpus; + +cleanup: + VIR_FREE(mask); + VIR_FREE(allonesmask); + VIR_FREE(cpumap); + + return ret; +} +# undef MASK_CPU_ISSET +# undef n_bits + #else int virNumaSetupMemoryPolicy(virNumaTuneDef numatune, @@ -295,6 +374,18 @@ virNumaGetNodeMemory(int node ATTRIBUTE_UNUSED, VIR_DEBUG("NUMA isn't available on this host"); return -1; } + + +int +virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED, + virBitmapPtr *cpus) +{ + *cpus = NULL; + + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NUMA isn't available on this host")); + return -1; +} #endif diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 62b2c0a..eee0225 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -64,4 +64,6 @@ int virNumaGetNodeMemory(int node, unsigned int virNumaGetMaxCPUs(void); +int virNumaGetNodeCPUs(int node, virBitmapPtr *cpus); + #endif /* __VIR_NUMA_H__ */ -- 1.8.3.2

Now that all libnuma functions used by libvirt are wrapped in virNuma we can remove the dependancy from nodeinfo. --- src/nodeinfo.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 7e75611..87edf67 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -33,11 +33,6 @@ #include <sched.h> #include "conf/domain_conf.h" -#if WITH_NUMACTL -# define NUMA_VERSION1_COMPATIBILITY 1 -# include <numa.h> -#endif - #if defined(__FreeBSD__) || defined(__APPLE__) # include <sys/types.h> # include <sys/sysctl.h> @@ -1512,7 +1507,6 @@ nodeGetFreeMemoryFake(void) return ret; } -#if WITH_NUMACTL static virBitmapPtr virNodeGetSiblingsList(const char *dir, int cpu_id) { @@ -1693,23 +1687,3 @@ nodeGetFreeMemory(void) return freeMem; } - - -#else -int nodeCapsInitNUMA(virCapsPtr caps) { - return nodeCapsInitNUMAFake(caps); -} - -int nodeGetCellsFreeMemory(unsigned long long *freeMems, - int startCell, - int maxCells) -{ - return nodeGetCellsFreeMemoryFake(freeMems, - startCell, maxCells); -} - -unsigned long long nodeGetFreeMemory(void) -{ - return nodeGetFreeMemoryFake(); -} -#endif -- 1.8.3.2

On 10/18/2013 05:05 PM, Peter Krempa wrote: This broke the build on FreeBSD: CC libvirt_driver_la-nodeinfo.lo nodeinfo.c: In function 'virNodeGetSiblingsList': nodeinfo.c:1543: error: 'SYSFS_THREAD_SIBLINGS_LIST_LENGTH_MAX' undeclared (first use in this function) nodeinfo.c:1543: error: (Each undeclared identifier is reported only once nodeinfo.c:1543: error: for each function it appears in.) cc1: warnings being treated as errors nodeinfo.c: In function 'virNodeCapsFillCPUInfo': nodeinfo.c:1562: warning: implicit declaration of function 'virNodeGetCpuValue' nodeinfo.c:1562: warning: nested extern declaration of 'virNodeGetCpuValue' [-Wnested-externs] nodeinfo.c:1562: error: 'SYSFS_CPU_PATH' undeclared (first use in this function) make[3]: *** [libvirt_driver_la-nodeinfo.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... make[3]: Leaving directory `/root/libvirt2/libvirt/src' make[2]: *** [all] Error 2 make[2]: Leaving directory `/root/libvirt2/libvirt/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/root/libvirt2/libvirt' make: *** [all] Error 2 Jan