[PATCH 1/1] virhostcpu.c: fix 'die' parsing for Power hosts
Commit 7b79ee2f78 makes assumptions about die_id parsing in
the sysfs that aren't true for Power hosts. In both Power8
and Power9 the die_id is set to -1:
$ cat /sys/devices/system/cpu/cpu0/topology/die_id
-1
This breaks the parsing being done in virHostCPUGetDie(),
which is using an uInt instead, stopping the VM to start:
virFileReadValueUint:4128 : internal error: Invalid unsigned integer
value '-1' in file '/sys/devices/system/cpu/cpu0/topology/die_id'
This isn't necessarily a PowerPC only behavior. Linux kernel commit
0e344d8c70 added in the former Documentation/cputopology.txt, now
Documentation/admin-guide/cputopology.rst, that:
---
To be consistent on all architectures, include/linux/topology.h
provides default definitions for any of the above macros that are
not defined by include/asm-XXX/topology.h:
1) topology_physical_package_id: -1
2) topology_die_id: -1
(...)
---
This means that it might be expected that an architecture that
does not implement the die_id element will mark it as -1 in
sysfs.
It is not required to change die_id implementation from uInt to
Int because of that. Instead, let's change the parsing of the
die_id in virHostCPUGetDie() to read an integer value and, in
case it's -1, default it to zero like in case of file not found.
This is enough to solve the issue Power hosts are experiencing.
Fixes: 7b79ee2f78bbf2af76df2f6466919e19ae05aeeb
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
---
src/util/virhostcpu.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index c1b39e772a..8d7841517e 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -220,15 +220,20 @@ virHostCPUGetSocket(unsigned int cpu, unsigned int *socket)
int
virHostCPUGetDie(unsigned int cpu, unsigned int *die)
{
- int ret = virFileReadValueUint(die,
- "%s/cpu/cpu%u/topology/die_id",
- SYSFS_SYSTEM_PATH, cpu);
+ int die_id;
+ int ret = virFileReadValueInt(&die_id,
+ "%s/cpu/cpu%u/topology/die_id",
+ SYSFS_SYSTEM_PATH, cpu);
- /* If the file is not there, it's 0 */
- if (ret == -2)
+ /* If the file is not there, it's 0.
+ * PowerPC hosts can report die_id = -1, which for our
+ * case here it's the same as absent file. */
+ if (ret == -2 || die_id < 0)
*die = 0;
else if (ret < 0)
return -1;
+ else
+ *die = die_id;
return 0;
}
--
2.24.1