[libvirt] [RFC: PATCH] nodeinfo: support kernels that lack socket information
On RHEL 5, I was getting a segfault trying to start libvirtd,
because we were failing virNodeParseSocket but not checking
for errors, and then calling CPU_SET(-1, &sock_map) as a result.
But if you don't have a topology/physical_package_id file,
then you can just assume that the cpu belongs to socket 0.
* src/nodeinfo.c (virNodeGetCpuValue): Change bool into
default_value.
(virNodeParseSocket, virNodeParseNode): Update callers.
---
I'm still testing this, but wanted to get eyes on it now as I think
it belongs in 1.0.0 if it passes my testing. Having libvirtd dump
core on RHEL 5 is not nice. I'm not quite sure when the regression
was introduced, but it was caused by our refactoring of code to
parse different sysfs files.
src/nodeinfo.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 35c5f96..ccf0682 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -79,13 +79,14 @@ static int linuxNodeGetMemoryStats(FILE *meminfo,
int *nparams);
/* Return the positive decimal contents of the given
- * DIR/cpu%u/FILE, or -1 on error. If MISSING_OK and the
- * file could not be found, return 1 instead of an error; this is
- * because some machines cannot hot-unplug cpu0, or because
- * hot-unplugging is disabled. */
+ * 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;
+ * this is useful for machines that cannot hot-unplug cpu0, or where
+ * hot-unplugging is disabled, or where the kernel is too old
+ * to support NUMA cells, etc. */
static int
virNodeGetCpuValue(const char *dir, unsigned int cpu, const char *file,
- bool missing_ok)
+ int default_value)
{
char *path;
FILE *pathfp;
@@ -100,8 +101,8 @@ virNodeGetCpuValue(const char *dir, unsigned int cpu, const char
*file,
pathfp = fopen(path, "r");
if (pathfp == NULL) {
- if (missing_ok && errno == ENOENT)
- value = 1;
+ if (default_value >= 0 && errno == ENOENT)
+ value = default_value;
else
virReportSystemError(errno, _("cannot open %s"), path);
goto cleanup;
@@ -174,7 +175,7 @@ static int
virNodeParseSocket(const char *dir, unsigned int cpu)
{
int ret = virNodeGetCpuValue(dir, cpu, "topology/physical_package_id",
- false);
+ 0);
# if defined(__powerpc__) || \
defined(__powerpc64__) || \
defined(__s390__) || \
@@ -236,7 +237,7 @@ virNodeParseNode(const char *node, int *sockets, int *cores, int
*threads)
if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
continue;
- if ((online = virNodeGetCpuValue(node, cpu, "online", true)) < 0)
+ if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
goto cleanup;
if (!online)
@@ -275,7 +276,7 @@ virNodeParseNode(const char *node, int *sockets, int *cores, int
*threads)
if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
continue;
- if ((online = virNodeGetCpuValue(node, cpu, "online", true)) < 0)
+ if ((online = virNodeGetCpuValue(node, cpu, "online", 1)) < 0)
goto cleanup;
if (!online)
@@ -297,7 +298,7 @@ virNodeParseNode(const char *node, int *sockets, int *cores, int
*threads)
/* logical cpu is equivalent to a core on s390 */
core = cpu;
# else
- core = virNodeGetCpuValue(node, cpu, "topology/core_id", false);
+ core = virNodeGetCpuValue(node, cpu, "topology/core_id", -1);
# endif
CPU_SET(core, &core_maps[sock]);
--
1.7.11.7