|
 |
397dc2 |
From b5aa3a33bc770714f8a68954c05ea362fcfd4d47 Mon Sep 17 00:00:00 2001
|
|
|
397dc2 |
Message-Id: <b5aa3a33bc770714f8a68954c05ea362fcfd4d47@dist-git>
|
|
|
397dc2 |
From: Daniel Henrique Barboza <danielhb413@gmail.com>
|
|
|
397dc2 |
Date: Mon, 5 Oct 2020 10:25:30 -0400
|
|
|
397dc2 |
Subject: [PATCH] virhostcpu.c: fix 'die_id' parsing for Power hosts
|
|
|
397dc2 |
|
|
|
397dc2 |
Commit 7b79ee2f78 makes assumptions about die_id parsing in
|
|
|
397dc2 |
the sysfs that aren't true for Power hosts. In both Power8
|
|
|
397dc2 |
and Power9, running 5.6 and 4.18 kernel respectively,
|
|
|
397dc2 |
'die_id' is set to -1:
|
|
|
397dc2 |
|
|
|
397dc2 |
$ cat /sys/devices/system/cpu/cpu0/topology/die_id
|
|
|
397dc2 |
-1
|
|
|
397dc2 |
|
|
|
397dc2 |
This breaks virHostCPUGetDie() parsing because it is trying to
|
|
|
397dc2 |
retrieve an unsigned integer, causing problems during VM start:
|
|
|
397dc2 |
|
|
|
397dc2 |
virFileReadValueUint:4128 : internal error: Invalid unsigned integer
|
|
|
397dc2 |
value '-1' in file '/sys/devices/system/cpu/cpu0/topology/die_id'
|
|
|
397dc2 |
|
|
|
397dc2 |
This isn't necessarily a PowerPC only behavior. Linux kernel commit
|
|
|
397dc2 |
0e344d8c70 added in the former Documentation/cputopology.txt, now
|
|
|
397dc2 |
Documentation/admin-guide/cputopology.rst, that:
|
|
|
397dc2 |
|
|
|
397dc2 |
To be consistent on all architectures, include/linux/topology.h
|
|
|
397dc2 |
provides default definitions for any of the above macros that are
|
|
|
397dc2 |
not defined by include/asm-XXX/topology.h:
|
|
|
397dc2 |
|
|
|
397dc2 |
1) topology_physical_package_id: -1
|
|
|
397dc2 |
2) topology_die_id: -1
|
|
|
397dc2 |
(...)
|
|
|
397dc2 |
|
|
|
397dc2 |
This means that it might be expected that an architecture that
|
|
|
397dc2 |
does not implement the die_id element will mark it as -1 in
|
|
|
397dc2 |
sysfs.
|
|
|
397dc2 |
|
|
|
397dc2 |
It is not required to change die_id implementation from uInt to
|
|
|
397dc2 |
Int because of that. Instead, let's change the parsing of the
|
|
|
397dc2 |
die_id in virHostCPUGetDie() to read an integer value and, in
|
|
|
397dc2 |
case it's -1, default it to zero like in case of file not found.
|
|
|
397dc2 |
This is enough to solve the issue Power hosts are experiencing.
|
|
|
397dc2 |
|
|
|
397dc2 |
Fixes: 7b79ee2f78bbf2af76df2f6466919e19ae05aeeb
|
|
|
397dc2 |
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
|
|
|
397dc2 |
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
|
|
397dc2 |
(cherry picked from commit 0137bf0dab2738d5443e2f407239856e2aa25bb3)
|
|
|
397dc2 |
|
|
|
397dc2 |
https://bugzilla.redhat.com/show_bug.cgi?id=1876742
|
|
|
397dc2 |
|
|
|
397dc2 |
Signed-off-by: Daniel Henrique Barboza <dbarboza@redhat.com>
|
|
|
397dc2 |
Message-Id: <20201005142530.3961036-1-dbarboza@redhat.com>
|
|
|
397dc2 |
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
|
|
|
397dc2 |
---
|
|
|
397dc2 |
src/util/virhostcpu.c | 21 ++++++++++++++-------
|
|
|
397dc2 |
1 file changed, 14 insertions(+), 7 deletions(-)
|
|
|
397dc2 |
|
|
|
397dc2 |
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
|
|
|
397dc2 |
index 09c959cd25..218272d7ec 100644
|
|
|
397dc2 |
--- a/src/util/virhostcpu.c
|
|
|
397dc2 |
+++ b/src/util/virhostcpu.c
|
|
|
397dc2 |
@@ -221,16 +221,23 @@ virHostCPUGetSocket(unsigned int cpu, unsigned int *socket)
|
|
|
397dc2 |
int
|
|
|
397dc2 |
virHostCPUGetDie(unsigned int cpu, unsigned int *die)
|
|
|
397dc2 |
{
|
|
|
397dc2 |
- int ret = virFileReadValueUint(die,
|
|
|
397dc2 |
- "%s/cpu/cpu%u/topology/die_id",
|
|
|
397dc2 |
- SYSFS_SYSTEM_PATH, cpu);
|
|
|
397dc2 |
+ int die_id;
|
|
|
397dc2 |
+ int ret = virFileReadValueInt(&die_id,
|
|
|
397dc2 |
+ "%s/cpu/cpu%u/topology/die_id",
|
|
|
397dc2 |
+ SYSFS_SYSTEM_PATH, cpu);
|
|
|
397dc2 |
|
|
|
397dc2 |
- /* If the file is not there, it's 0 */
|
|
|
397dc2 |
- if (ret == -2)
|
|
|
397dc2 |
- *die = 0;
|
|
|
397dc2 |
- else if (ret < 0)
|
|
|
397dc2 |
+ if (ret == -1)
|
|
|
397dc2 |
return -1;
|
|
|
397dc2 |
|
|
|
397dc2 |
+ /* If the file is not there, it's 0.
|
|
|
397dc2 |
+ * Another alternative is die_id set to -1, meaning that
|
|
|
397dc2 |
+ * the arch does not have die_id support. Set @die to
|
|
|
397dc2 |
+ * 0 in this case too. */
|
|
|
397dc2 |
+ if (ret == -2 || die_id < 0)
|
|
|
397dc2 |
+ *die = 0;
|
|
|
397dc2 |
+ else
|
|
|
397dc2 |
+ *die = die_id;
|
|
|
397dc2 |
+
|
|
|
397dc2 |
return 0;
|
|
|
397dc2 |
}
|
|
|
397dc2 |
|
|
|
397dc2 |
--
|
|
|
397dc2 |
2.28.0
|
|
|
397dc2 |
|