[libvirt] [PATCHv2 00/10] Strip libnuma functions from nodeinfo.c
As a pre-requisite for doing unit tests for topology detection code the code in nodeinfo.c needs to be cleaned up. Peter Krempa (10): nodeinfo: Rename linuxNodeInfoCPUPopulate and export it properly numa: Introduce virNumaIsAvailable and use it instead of numa_available nodeinfo: Avoid forward declarations of static functions numa: Introduce virNumaGetMaxNode and use it instead of numa_max_node numa: Introduce virNumaGetNodeMemory and use it instead of numa_node_size64 nodeinfo: Get rid of nodeGetCellMemory numa: Replace NUMA_MAX_N_CPUS macro with virNumaGetMaxCPUs() caps: Fix function docs for virCapabilitiesAddHostNUMACell numa: Add wrapper of numa_node_to_cpus and use it nodeinfo: Remove libnuma include src/conf/capabilities.c | 1 + src/libvirt_linux.syms | 2 +- src/libvirt_private.syms | 3 + src/nodeinfo.c | 222 +++++++++++++---------------------------------- src/nodeinfo.h | 5 ++ src/util/virnuma.c | 221 +++++++++++++++++++++++++++++++++++++++++++++- src/util/virnuma.h | 11 +++ tests/nodeinfotest.c | 6 +- 8 files changed, 304 insertions(+), 167 deletions(-) -- 1.8.3.2
Call it virNodeInfoLinuxPopulateCPU and use the header file to export it instead of extern definition in the test file. --- src/libvirt_linux.syms | 2 +- src/nodeinfo.c | 14 +++++--------- src/nodeinfo.h | 5 +++++ tests/nodeinfotest.c | 6 +----- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms index 3500898..bfef3ec 100644 --- a/src/libvirt_linux.syms +++ b/src/libvirt_linux.syms @@ -3,7 +3,7 @@ # # nodeinfo.h -linuxNodeInfoCPUPopulate; +virNodeInfoLinuxPopulateCPU; # util/virstatslinux.h linuxDomainInterfaceStats; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 70814c2..232b465 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -88,11 +88,6 @@ appleFreebsdNodeGetCPUCount(void) # define LINUX_NB_MEMORY_STATS_ALL 4 # define LINUX_NB_MEMORY_STATS_CELL 2 -/* NB, this is not static as we need to call it from the testsuite */ -int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - const char *sysfs_dir, - virNodeInfoPtr nodeinfo); - static int linuxNodeGetCPUStats(FILE *procstat, int cpuNum, virNodeCPUStatsPtr params, @@ -376,9 +371,10 @@ cleanup: return ret; } -int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - const char *sysfs_dir, - virNodeInfoPtr nodeinfo) +int +virNodeInfoLinuxPopulateCPU(FILE *cpuinfo, + const char *sysfs_dir, + virNodeInfoPtr nodeinfo) { char line[1024]; DIR *nodedir = NULL; @@ -872,7 +868,7 @@ int nodeGetInfo(virNodeInfoPtr nodeinfo) return -1; } - ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo); + ret = virNodeInfoLinuxPopulateCPU(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo); if (ret < 0) goto cleanup; diff --git a/src/nodeinfo.h b/src/nodeinfo.h index 413fddd..a13cf28 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -57,4 +57,9 @@ int nodeGetCPUMap(unsigned char **cpumap, unsigned int *online, unsigned int flags); + +int virNodeInfoLinuxPopulateCPU(FILE *cpuinfo, + const char *sysfs_dir, + virNodeInfoPtr nodeinfo); + #endif /* __VIR_NODEINFO_H__*/ diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c index 74f6d4d..9bb7adb 100644 --- a/tests/nodeinfotest.c +++ b/tests/nodeinfotest.c @@ -27,10 +27,6 @@ main(void) #else -extern int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - char *sysfs_dir, - virNodeInfoPtr nodeinfo); - static int linuxTestCompareFiles(const char *cpuinfofile, char *sysfs_dir, @@ -50,7 +46,7 @@ linuxTestCompareFiles(const char *cpuinfofile, goto fail; memset(&nodeinfo, 0, sizeof(nodeinfo)); - if (linuxNodeInfoCPUPopulate(cpuinfo, sysfs_dir, &nodeinfo) < 0) { + if (virNodeInfoLinuxPopulateCPU(cpuinfo, sysfs_dir, &nodeinfo) < 0) { if (virTestGetDebug()) { virErrorPtr error = virSaveLastError(); if (error && error->code != VIR_ERR_OK) -- 1.8.3.2
On Fri, Oct 18, 2013 at 05:05:41PM +0200, Peter Krempa wrote:
Call it virNodeInfoLinuxPopulateCPU and use the header file to export it instead of extern definition in the test file. --- src/libvirt_linux.syms | 2 +- src/nodeinfo.c | 14 +++++--------- src/nodeinfo.h | 5 +++++ tests/nodeinfotest.c | 6 +----- 4 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms index 3500898..bfef3ec 100644 --- a/src/libvirt_linux.syms +++ b/src/libvirt_linux.syms @@ -3,7 +3,7 @@ #
# nodeinfo.h -linuxNodeInfoCPUPopulate; +virNodeInfoLinuxPopulateCPU;
# util/virstatslinux.h linuxDomainInterfaceStats; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 70814c2..232b465 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -88,11 +88,6 @@ appleFreebsdNodeGetCPUCount(void) # define LINUX_NB_MEMORY_STATS_ALL 4 # define LINUX_NB_MEMORY_STATS_CELL 2
-/* NB, this is not static as we need to call it from the testsuite */ -int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - const char *sysfs_dir, - virNodeInfoPtr nodeinfo); - static int linuxNodeGetCPUStats(FILE *procstat, int cpuNum, virNodeCPUStatsPtr params, @@ -376,9 +371,10 @@ cleanup: return ret; }
-int linuxNodeInfoCPUPopulate(FILE *cpuinfo, - const char *sysfs_dir, - virNodeInfoPtr nodeinfo) +int +virNodeInfoLinuxPopulateCPU(FILE *cpuinfo, + const char *sysfs_dir, + virNodeInfoPtr nodeinfo) { char line[1024]; DIR *nodedir = NULL; @@ -872,7 +868,7 @@ int nodeGetInfo(virNodeInfoPtr nodeinfo) return -1; }
- ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo); + ret = virNodeInfoLinuxPopulateCPU(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo); if (ret < 0) goto cleanup;
diff --git a/src/nodeinfo.h b/src/nodeinfo.h index 413fddd..a13cf28 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -57,4 +57,9 @@ int nodeGetCPUMap(unsigned char **cpumap, unsigned int *online, unsigned int flags);
+ +int virNodeInfoLinuxPopulateCPU(FILE *cpuinfo, + const char *sysfs_dir, + virNodeInfoPtr nodeinfo);
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 :|
On 10/28/13 13:13, Daniel P. Berrange wrote:
On Fri, Oct 18, 2013 at 05:05:41PM +0200, Peter Krempa wrote:
Call it virNodeInfoLinuxPopulateCPU and use the header file to export it instead of extern definition in the test file. --- src/libvirt_linux.syms | 2 +- src/nodeinfo.c | 14 +++++--------- src/nodeinfo.h | 5 +++++ tests/nodeinfotest.c | 6 +----- 4 files changed, 12 insertions(+), 15 deletions(-)
...
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.
Hmm, I will send a new version of this one adding the new header when finishing the refactor of nodeinfo.c I've pushed the rest of the patches that were ACKed. Thanks. Peter
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
On Fri, Oct 18, 2013 at 05:05:42PM +0200, Peter Krempa wrote:
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(-)
ACK 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 :|
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
On Fri, Oct 18, 2013 at 05:05:43PM +0200, Peter Krempa wrote:
linuxNodeGetCPUStats() and linuxNodeGetMemoryStats() are static and don't need a forward declaration. --- src/nodeinfo.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-)
ACK 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 :|
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
On Fri, Oct 18, 2013 at 05:05:44PM +0200, Peter Krempa wrote:
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(-)
ACK 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 :|
--- 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
On Fri, Oct 18, 2013 at 05:05:45PM +0200, Peter Krempa wrote:
--- 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(-)
ACK 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 :|
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
On Fri, Oct 18, 2013 at 05:05:46PM +0200, Peter Krempa wrote:
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(-)
ACK 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 :|
--- 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
On Fri, Oct 18, 2013 at 05:05:47PM +0200, Peter Krempa wrote:
--- src/nodeinfo.c | 9 ++------- src/util/virnuma.c | 24 +++++++++++++++++++++++- src/util/virnuma.h | 2 ++ 3 files changed, 27 insertions(+), 8 deletions(-)
ACK 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 :|
--- 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
On Fri, Oct 18, 2013 at 05:05:48PM +0200, Peter Krempa wrote:
--- 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
ACK 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 :|
--- 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
On Fri, Oct 18, 2013 at 05:05:49PM +0200, Peter Krempa wrote:
--- src/nodeinfo.c | 62 ++++++++++++++----------------------- src/util/virnuma.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virnuma.h | 2 ++ 3 files changed, 117 insertions(+), 38 deletions(-)
ACK 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 :|
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 Fri, Oct 18, 2013 at 05:05:50PM +0200, Peter Krempa wrote:
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(-)
ACK 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 10/18/2013 05:05 PM, Peter Krempa wrote:
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
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
On 11/04/13 13:36, Ján Tomko wrote:
On 10/18/2013 05:05 PM, Peter Krempa wrote:
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(-)
...
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
Hmmm, nodeinfo.c is a convoluted mess of #ifdefs and platform specific code. While factoring out the numa code I missed that this would enable building some of that code on freebsd while some required functions are protected by #ifdef __linux__. I'll try to finish the split of nodeinfo.c into platform specific subfiles. This should fix this issue.
Jan
Thanks for pointing this out. Peter
On 11/04/2013 05:36 AM, Ján Tomko wrote:
On 10/18/2013 05:05 PM, Peter Krempa wrote:
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(-)
This broke the build on FreeBSD:
CC libvirt_driver_la-nodeinfo.lo nodeinfo.c: In function 'virNodeGetSiblingsList':
I also assume (but haven't actually bisected) that this series is responsible for a break in mingw: CC libvirt_driver_la-nodeinfo.lo ../../src/nodeinfo.c: In function 'virNodeGetSiblingsList': ../../src/nodeinfo.c:1543:30: error: 'SYSFS_THREAD_SIBLINGS_LIST_LENGTH_MAX' undeclared (first use in this function) if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGTH_MAX, &buf) < 0) ^ ../../src/nodeinfo.c:1543:30: note: each undeclared identifier is reported only once for each function it appears in ../../src/nodeinfo.c: In function 'virNodeCapsFillCPUInfo': ../../src/nodeinfo.c:1562:5: error: implicit declaration of function 'virNodeGetCpuValue' [-Werror=implicit-function-declaration] if ((tmp = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id, ^ ../../src/nodeinfo.c:1562:5: error: nested extern declaration of 'virNodeGetCpuValue' [-Werror=nested-externs] ../../src/nodeinfo.c:1562:35: error: 'SYSFS_CPU_PATH' undeclared (first use in this function) if ((tmp = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id, ^ cc1: all warnings being treated as errors -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Ján Tomko -
Peter Krempa