[PATCH v2 1/1] virhostcpu.c: fix 'die_id' parsing for Power hosts

Daniel Henrique Barboza danielhb413 at gmail.com
Tue Mar 17 00:01:34 UTC 2020


Commit 7b79ee2f78 makes assumptions about die_id parsing in
the sysfs that aren't true for Power hosts. In both Power8
and Power9, running 5.6 and 4.18 kernel respectively,
'die_id' is set to -1:

$ cat /sys/devices/system/cpu/cpu0/topology/die_id
-1

This breaks virHostCPUGetDie() parsing because it is trying to
retrieve an unsigned integer, causing problems during VM 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 can be expected that an architecture that
does not implement the die_id element will mark it as -1 in
sysfs.

Let's change the parsing being done in virHostCPUGetDie(), reading
die_id as an integer instead of unsigned int. In case die_id is -1,
default it to zero like the case of file not found.

Fixes: 7b79ee2f78bbf2af76df2f6466919e19ae05aeeb
Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
---
 src/util/virhostcpu.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index c1b39e772a..67d3e3308b 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -220,16 +220,23 @@ 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)
-        *die = 0;
-    else if (ret < 0)
+    if (ret == -1)
         return -1;
 
+    /* If the file is not there, it's 0.
+     * Another alternative is die_id set to -1, meaning that
+     * the arch does not have die_id support. Set @die to
+     * 0 in this case too. */
+    if (ret == -2 || die_id < 0)
+        *die = 0;
+    else
+        *die = die_id;
+
     return 0;
 }
 
-- 
2.24.1





More information about the libvir-list mailing list