[libvirt PATCH 0/6] qemu: Invalidate capabilities when host CPU changes
The host CPU related info stored in the capabilities cache is no longer valid after the host CPU changes. This is not a frequent situation in real world, but it can easily happen in nested scenarios when a disk image is started with various CPUs. https://bugzilla.redhat.com/show_bug.cgi?id=1778819 Jiri Denemark (6): util: Define g_autoptr callback for FILE hostcpu: Introduce virHostCPUGetSignature hostcpu: Implement virHostCPUGetSignature for x86 hostcpu: Implement virHostCPUGetSignature for ppc64 hostcpu: Implement virHostCPUGetSignature for s390 qemu: Invalidate capabilities when host CPU changes src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 24 ++++ src/qemu/qemu_capspriv.h | 1 + src/util/virfile.h | 1 + src/util/virhostcpu.c | 111 ++++++++++++++++++ src/util/virhostcpu.h | 2 + src/util/virhostcpupriv.h | 4 + tests/qemucapsprobe.c | 2 +- .../linux-ppc64-deconf-cpus.signature | 1 + .../linux-ppc64-subcores1.signature | 1 + .../linux-ppc64-subcores2.signature | 1 + .../linux-ppc64-subcores3.signature | 1 + .../linux-s390x-with-frequency.signature | 1 + .../linux-x86_64-test1.signature | 1 + .../linux-x86_64-test2.signature | 1 + .../linux-x86_64-test3.signature | 1 + .../linux-x86_64-test4.signature | 1 + .../linux-x86_64-test5.signature | 1 + .../linux-x86_64-test6.signature | 1 + .../linux-x86_64-test7.signature | 1 + .../linux-x86_64-test8.signature | 1 + .../linux-x86_64-with-die.signature | 1 + tests/virhostcputest.c | 42 ++++++- 23 files changed, 201 insertions(+), 2 deletions(-) create mode 100644 tests/virhostcpudata/linux-ppc64-deconf-cpus.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores1.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores2.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores3.signature create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test1.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test2.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test3.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test4.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test5.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test6.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test7.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test8.signature create mode 100644 tests/virhostcpudata/linux-x86_64-with-die.signature -- 2.26.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virfile.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virfile.h b/src/util/virfile.h index 0a520a7522..7a92364a5c 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -89,6 +89,7 @@ static inline void virForceCloseHelper(int *fd) */ #define VIR_AUTOCLOSE __attribute__((cleanup(virForceCloseHelper))) int +G_DEFINE_AUTOPTR_CLEANUP_FUNC(FILE, fclose); /* Opaque type for managing a wrapper around a fd. */ struct _virFileWrapperFd; -- 2.26.2
The purpose of this function is to give a short description that would be change when a host CPU is replaced with a different model. This is currently implemented by reading /proc/cpuinfo. It should be implemented for all architectures for which the QEMU driver stores host CPU data in the capabilities cache. In other words for archs that support host-model CPUs. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt_private.syms | 2 ++ src/util/virhostcpu.c | 37 ++++++++++++++++++++++++++++++++++ src/util/virhostcpu.h | 2 ++ src/util/virhostcpupriv.h | 4 ++++ tests/virhostcputest.c | 42 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 935ef7303b..2bebb62b4f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2217,9 +2217,11 @@ virHostCPUGetMSR; virHostCPUGetOnline; virHostCPUGetOnlineBitmap; virHostCPUGetPresentBitmap; +virHostCPUGetSignature; virHostCPUGetStats; virHostCPUGetThreadsPerSubcore; virHostCPUHasBitmap; +virHostCPUReadSignature; virHostCPUStatsAssign; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 721d959d46..bfef022f64 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1416,3 +1416,40 @@ virHostCPUGetTscInfo(void) #endif /* HAVE_LINUX_KVM_H && defined(KVM_GET_MSRS) && \ (defined(__i386__) || defined(__x86_64__)) && \ (defined(__linux__) || defined(__FreeBSD__)) */ + +int +virHostCPUReadSignature(virArch arch G_GNUC_UNUSED, + FILE *cpuinfo G_GNUC_UNUSED, + char **signature G_GNUC_UNUSED) +{ + return 0; +} + +#ifdef __linux__ + +int +virHostCPUGetSignature(char **signature) +{ + g_autoptr(FILE) cpuinfo = NULL; + + *signature = NULL; + + if (!(cpuinfo = fopen(CPUINFO_PATH, "r"))) { + virReportSystemError(errno, _("Failed to open cpuinfo file '%s'"), + CPUINFO_PATH); + return -1; + } + + return virHostCPUReadSignature(virArchFromHost(), cpuinfo, signature); +} + +#else + +int +virHostCPUGetSignature(char **signature) +{ + *signature = NULL; + return 0; +} + +#endif /* __linux__ */ diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index 9be2e51a38..48b1431ca4 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -79,3 +79,5 @@ int virHostCPUGetMSR(unsigned long index, uint64_t *msr); virHostCPUTscInfoPtr virHostCPUGetTscInfo(void); + +int virHostCPUGetSignature(char **signature); diff --git a/src/util/virhostcpupriv.h b/src/util/virhostcpupriv.h index afb415f9ea..f7b1e7c93c 100644 --- a/src/util/virhostcpupriv.h +++ b/src/util/virhostcpupriv.h @@ -42,3 +42,7 @@ int virHostCPUGetStatsLinux(FILE *procstat, virNodeCPUStatsPtr params, int *nparams); #endif + +int virHostCPUReadSignature(virArch arch, + FILE *cpuinfo, + char **signature); diff --git a/tests/virhostcputest.c b/tests/virhostcputest.c index 70a723098b..62bacddefb 100644 --- a/tests/virhostcputest.c +++ b/tests/virhostcputest.c @@ -1,6 +1,7 @@ #include <config.h> #include <unistd.h> +#include <fcntl.h> #include "testutils.h" #include "internal.h" @@ -193,6 +194,38 @@ linuxTestHostCPU(const void *opaque) return result; } + +static int +hostCPUSignature(const void *opaque) +{ + const struct linuxTestHostCPUData *data = opaque; + const char *arch = virArchToString(data->arch); + g_autofree char *cpuinfo = NULL; + g_autofree char *expected = NULL; + g_autofree char *signature = NULL; + g_autoptr(FILE) f = NULL; + + cpuinfo = g_strdup_printf("%s/virhostcpudata/linux-%s-%s.cpuinfo", + abs_srcdir, arch, data->testName); + expected = g_strdup_printf("%s/virhostcpudata/linux-%s-%s.signature", + abs_srcdir, arch, data->testName); + + if (!(f = fopen(cpuinfo, "r"))) { + virReportSystemError(errno, + "Failed to open cpuinfo file '%s'", cpuinfo); + return -1; + } + + if (virHostCPUReadSignature(data->arch, f, &signature) < 0) + return -1; + + if (!signature && !virFileExists(expected)) + return 0; + + return virTestCompareToFile(signature, expected); +} + + struct nodeCPUStatsData { const char *name; int ncpus; @@ -268,10 +301,17 @@ mymain(void) if (virInitialize() < 0) return EXIT_FAILURE; - for (i = 0; i < G_N_ELEMENTS(nodeData); i++) + for (i = 0; i < G_N_ELEMENTS(nodeData); i++) { + g_autofree char *sigTest = NULL; + if (virTestRun(nodeData[i].testName, linuxTestHostCPU, &nodeData[i]) != 0) ret = -1; + sigTest = g_strdup_printf("%s CPU signature", nodeData[i].testName); + if (virTestRun(sigTest, hostCPUSignature, &nodeData[i]) != 0) + ret = -1; + } + # define DO_TEST_CPU_STATS(name, ncpus, shouldFail) \ do { \ static struct nodeCPUStatsData data = { name, ncpus, shouldFail}; \ -- 2.26.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virhostcpu.c | 50 +++++++++++++++++-- .../linux-x86_64-test1.signature | 1 + .../linux-x86_64-test2.signature | 1 + .../linux-x86_64-test3.signature | 1 + .../linux-x86_64-test4.signature | 1 + .../linux-x86_64-test5.signature | 1 + .../linux-x86_64-test6.signature | 1 + .../linux-x86_64-test7.signature | 1 + .../linux-x86_64-test8.signature | 1 + .../linux-x86_64-with-die.signature | 1 + 10 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 tests/virhostcpudata/linux-x86_64-test1.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test2.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test3.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test4.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test5.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test6.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test7.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test8.signature create mode 100644 tests/virhostcpudata/linux-x86_64-with-die.signature diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index bfef022f64..851c0015f7 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1418,10 +1418,54 @@ virHostCPUGetTscInfo(void) (defined(__linux__) || defined(__FreeBSD__)) */ int -virHostCPUReadSignature(virArch arch G_GNUC_UNUSED, - FILE *cpuinfo G_GNUC_UNUSED, - char **signature G_GNUC_UNUSED) +virHostCPUReadSignature(virArch arch, + FILE *cpuinfo, + char **signature) { + size_t lineLen = 1024; + g_autofree char *line = g_new0(char, lineLen); + g_autofree char *vendor = NULL; + g_autofree char *name = NULL; + g_autofree char *family = NULL; + g_autofree char *model = NULL; + g_autofree char *stepping = NULL; + + if (!ARCH_IS_X86(arch)) + return 0; + + while (fgets(line, lineLen, cpuinfo)) { + g_auto(GStrv) parts = g_strsplit(line, ": ", 2); + + if (g_strv_length(parts) != 2) + continue; + + g_strstrip(parts[0]); + g_strstrip(parts[1]); + + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "model name")) { + if (!name) + name = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "cpu family")) { + if (!family) + family = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "model")) { + if (!model) + model = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "stepping")) { + if (!stepping) + stepping = g_steal_pointer(&parts[1]); + } + + if (vendor && name && family && model && stepping) { + *signature = g_strdup_printf("%s, %s, family: %s, model: %s, stepping: %s", + vendor, name, family, model, stepping); + return 0; + } + } + return 0; } diff --git a/tests/virhostcpudata/linux-x86_64-test1.signature b/tests/virhostcpudata/linux-x86_64-test1.signature new file mode 100644 index 0000000000..953337a4cb --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test1.signature @@ -0,0 +1 @@ +GenuineIntel, Intel(R) Xeon(TM) CPU 2.80GHz, family: 15, model: 4, stepping: 8 diff --git a/tests/virhostcpudata/linux-x86_64-test2.signature b/tests/virhostcpudata/linux-x86_64-test2.signature new file mode 100644 index 0000000000..bbeb084944 --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test2.signature @@ -0,0 +1 @@ +GenuineIntel, Intel(R) Core(TM)2 Duo CPU T9600 @ 2.80GHz, family: 6, model: 23, stepping: 10 diff --git a/tests/virhostcpudata/linux-x86_64-test3.signature b/tests/virhostcpudata/linux-x86_64-test3.signature new file mode 100644 index 0000000000..b94bbc1701 --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test3.signature @@ -0,0 +1 @@ +AuthenticAMD, AMD Opteron(tm) Processor 6172, family: 16, model: 9, stepping: 1 diff --git a/tests/virhostcpudata/linux-x86_64-test4.signature b/tests/virhostcpudata/linux-x86_64-test4.signature new file mode 100644 index 0000000000..9237332d59 --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test4.signature @@ -0,0 +1 @@ +GenuineIntel, Intel(R) Xeon(R) CPU E7- 8837 @ 2.67GHz, family: 6, model: 47, stepping: 2 diff --git a/tests/virhostcpudata/linux-x86_64-test5.signature b/tests/virhostcpudata/linux-x86_64-test5.signature new file mode 100644 index 0000000000..dbaba8ee08 --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test5.signature @@ -0,0 +1 @@ +GenuineIntel, Intel(R) Xeon(R) CPU E5320 @ 1.86GHz, family: 6, model: 15, stepping: 7 diff --git a/tests/virhostcpudata/linux-x86_64-test6.signature b/tests/virhostcpudata/linux-x86_64-test6.signature new file mode 100644 index 0000000000..e5d8476e06 --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test6.signature @@ -0,0 +1 @@ +GenuineIntel, Intel(R) Xeon(R) CPU E5640 @ 2.67GHz, family: 6, model: 44, stepping: 2 diff --git a/tests/virhostcpudata/linux-x86_64-test7.signature b/tests/virhostcpudata/linux-x86_64-test7.signature new file mode 100644 index 0000000000..a2e91b270b --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test7.signature @@ -0,0 +1 @@ +AuthenticAMD, AMD Opteron(tm) Processor 6174, family: 16, model: 9, stepping: 1 diff --git a/tests/virhostcpudata/linux-x86_64-test8.signature b/tests/virhostcpudata/linux-x86_64-test8.signature new file mode 100644 index 0000000000..e873053f3b --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-test8.signature @@ -0,0 +1 @@ +AuthenticAMD, AMD Opteron(tm) Processor 6282 SE, family: 21, model: 1, stepping: 2 diff --git a/tests/virhostcpudata/linux-x86_64-with-die.signature b/tests/virhostcpudata/linux-x86_64-with-die.signature new file mode 100644 index 0000000000..ce0545cd38 --- /dev/null +++ b/tests/virhostcpudata/linux-x86_64-with-die.signature @@ -0,0 +1 @@ +GenuineIntel, QEMU Virtual CPU version 2.5+, family: 6, model: 6, stepping: 3 -- 2.26.2
On a Monday in 2020, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virhostcpu.c | 50 +++++++++++++++++-- .../linux-x86_64-test1.signature | 1 + .../linux-x86_64-test2.signature | 1 + .../linux-x86_64-test3.signature | 1 + .../linux-x86_64-test4.signature | 1 + .../linux-x86_64-test5.signature | 1 + .../linux-x86_64-test6.signature | 1 + .../linux-x86_64-test7.signature | 1 + .../linux-x86_64-test8.signature | 1 + .../linux-x86_64-with-die.signature | 1 + 10 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 tests/virhostcpudata/linux-x86_64-test1.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test2.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test3.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test4.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test5.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test6.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test7.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test8.signature create mode 100644 tests/virhostcpudata/linux-x86_64-with-die.signature
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index bfef022f64..851c0015f7 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1418,10 +1418,54 @@ virHostCPUGetTscInfo(void) (defined(__linux__) || defined(__FreeBSD__)) */
int -virHostCPUReadSignature(virArch arch G_GNUC_UNUSED, - FILE *cpuinfo G_GNUC_UNUSED, - char **signature G_GNUC_UNUSED) +virHostCPUReadSignature(virArch arch, + FILE *cpuinfo, + char **signature) { + size_t lineLen = 1024; + g_autofree char *line = g_new0(char, lineLen); + g_autofree char *vendor = NULL; + g_autofree char *name = NULL; + g_autofree char *family = NULL; + g_autofree char *model = NULL; + g_autofree char *stepping = NULL; + + if (!ARCH_IS_X86(arch)) + return 0; + + while (fgets(line, lineLen, cpuinfo)) { + g_auto(GStrv) parts = g_strsplit(line, ": ", 2);
Indendation
+ + if (g_strv_length(parts) != 2) + continue; + + g_strstrip(parts[0]); + g_strstrip(parts[1]); + + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "model name")) { + if (!name) + name = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "cpu family")) { + if (!family) + family = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "model")) { + if (!model) + model = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "stepping")) { + if (!stepping) + stepping = g_steal_pointer(&parts[1]); + } + + if (vendor && name && family && model && stepping) { + *signature = g_strdup_printf("%s, %s, family: %s, model: %s, stepping: %s", + vendor, name, family, model, stepping); + return 0; + }
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virhostcpu.c | 58 ++++++++++++------- .../linux-ppc64-deconf-cpus.signature | 1 + .../linux-ppc64-subcores1.signature | 1 + .../linux-ppc64-subcores2.signature | 1 + .../linux-ppc64-subcores3.signature | 1 + 5 files changed, 41 insertions(+), 21 deletions(-) create mode 100644 tests/virhostcpudata/linux-ppc64-deconf-cpus.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores1.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores2.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores3.signature diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 851c0015f7..0ce895cb39 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1429,8 +1429,9 @@ virHostCPUReadSignature(virArch arch, g_autofree char *family = NULL; g_autofree char *model = NULL; g_autofree char *stepping = NULL; + g_autofree char *revision = NULL; - if (!ARCH_IS_X86(arch)) + if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch)) return 0; while (fgets(line, lineLen, cpuinfo)) { @@ -1442,27 +1443,42 @@ virHostCPUReadSignature(virArch arch, g_strstrip(parts[0]); g_strstrip(parts[1]); - if (STREQ(parts[0], "vendor_id")) { - if (!vendor) - vendor = g_steal_pointer(&parts[1]); - } else if (STREQ(parts[0], "model name")) { - if (!name) - name = g_steal_pointer(&parts[1]); - } else if (STREQ(parts[0], "cpu family")) { - if (!family) - family = g_steal_pointer(&parts[1]); - } else if (STREQ(parts[0], "model")) { - if (!model) - model = g_steal_pointer(&parts[1]); - } else if (STREQ(parts[0], "stepping")) { - if (!stepping) - stepping = g_steal_pointer(&parts[1]); - } + if (ARCH_IS_X86(arch)) { + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "model name")) { + if (!name) + name = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "cpu family")) { + if (!family) + family = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "model")) { + if (!model) + model = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "stepping")) { + if (!stepping) + stepping = g_steal_pointer(&parts[1]); + } - if (vendor && name && family && model && stepping) { - *signature = g_strdup_printf("%s, %s, family: %s, model: %s, stepping: %s", - vendor, name, family, model, stepping); - return 0; + if (vendor && name && family && model && stepping) { + *signature = g_strdup_printf("%s, %s, family: %s, model: %s, stepping: %s", + vendor, name, family, model, stepping); + return 0; + } + } else if (ARCH_IS_PPC64(arch)) { + if (STREQ(parts[0], "cpu")) { + if (!name) + name = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "revision")) { + if (!revision) + revision = g_steal_pointer(&parts[1]); + } + + if (name && revision) { + *signature = g_strdup_printf("%s, rev %s", name, revision); + return 0; + } } } diff --git a/tests/virhostcpudata/linux-ppc64-deconf-cpus.signature b/tests/virhostcpudata/linux-ppc64-deconf-cpus.signature new file mode 100644 index 0000000000..7adf19db2f --- /dev/null +++ b/tests/virhostcpudata/linux-ppc64-deconf-cpus.signature @@ -0,0 +1 @@ +POWER8E (raw), altivec supported, rev 2.1 (pvr 004b 0201) \ No newline at end of file diff --git a/tests/virhostcpudata/linux-ppc64-subcores1.signature b/tests/virhostcpudata/linux-ppc64-subcores1.signature new file mode 100644 index 0000000000..7adf19db2f --- /dev/null +++ b/tests/virhostcpudata/linux-ppc64-subcores1.signature @@ -0,0 +1 @@ +POWER8E (raw), altivec supported, rev 2.1 (pvr 004b 0201) \ No newline at end of file diff --git a/tests/virhostcpudata/linux-ppc64-subcores2.signature b/tests/virhostcpudata/linux-ppc64-subcores2.signature new file mode 100644 index 0000000000..7adf19db2f --- /dev/null +++ b/tests/virhostcpudata/linux-ppc64-subcores2.signature @@ -0,0 +1 @@ +POWER8E (raw), altivec supported, rev 2.1 (pvr 004b 0201) \ No newline at end of file diff --git a/tests/virhostcpudata/linux-ppc64-subcores3.signature b/tests/virhostcpudata/linux-ppc64-subcores3.signature new file mode 100644 index 0000000000..7adf19db2f --- /dev/null +++ b/tests/virhostcpudata/linux-ppc64-subcores3.signature @@ -0,0 +1 @@ +POWER8E (raw), altivec supported, rev 2.1 (pvr 004b 0201) \ No newline at end of file -- 2.26.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virhostcpu.c | 16 +++++++++++++++- .../linux-s390x-with-frequency.signature | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 0ce895cb39..0caf7959ef 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1430,8 +1430,9 @@ virHostCPUReadSignature(virArch arch, g_autofree char *model = NULL; g_autofree char *stepping = NULL; g_autofree char *revision = NULL; + g_autofree char *proc = NULL; - if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch)) + if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch) && !ARCH_IS_S390(arch)) return 0; while (fgets(line, lineLen, cpuinfo)) { @@ -1479,6 +1480,19 @@ virHostCPUReadSignature(virArch arch, *signature = g_strdup_printf("%s, rev %s", name, revision); return 0; } + } else if (ARCH_IS_S390(arch)) { + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "processor 0")) { + if (!proc) + proc = g_steal_pointer(&parts[1]); + } + + if (vendor && proc) { + *signature = g_strdup_printf("%s, %s", vendor, proc); + return 0; + } } } diff --git a/tests/virhostcpudata/linux-s390x-with-frequency.signature b/tests/virhostcpudata/linux-s390x-with-frequency.signature new file mode 100644 index 0000000000..6369870682 --- /dev/null +++ b/tests/virhostcpudata/linux-s390x-with-frequency.signature @@ -0,0 +1 @@ +IBM/S390, version = 00, identification = 145F07, machine = 2964 \ No newline at end of file -- 2.26.2
On 5/18/20 2:56 PM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virhostcpu.c | 16 +++++++++++++++- .../linux-s390x-with-frequency.signature | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 0ce895cb39..0caf7959ef 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1430,8 +1430,9 @@ virHostCPUReadSignature(virArch arch, g_autofree char *model = NULL; g_autofree char *stepping = NULL; g_autofree char *revision = NULL; + g_autofree char *proc = NULL;
- if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch)) + if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch) && !ARCH_IS_S390(arch)) return 0;
while (fgets(line, lineLen, cpuinfo)) { @@ -1479,6 +1480,19 @@ virHostCPUReadSignature(virArch arch, *signature = g_strdup_printf("%s, rev %s", name, revision); return 0; } + } else if (ARCH_IS_S390(arch)) { + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "processor 0")) { + if (!proc) + proc = g_steal_pointer(&parts[1]); + }
To catch scenarios of moving from LPAR to nested kvm or the other way around facilties must be added here as well.
+ + if (vendor && proc) { + *signature = g_strdup_printf("%s, %s", vendor, proc); + return 0; + } } }
diff --git a/tests/virhostcpudata/linux-s390x-with-frequency.signature b/tests/virhostcpudata/linux-s390x-with-frequency.signature new file mode 100644 index 0000000000..6369870682 --- /dev/null +++ b/tests/virhostcpudata/linux-s390x-with-frequency.signature @@ -0,0 +1 @@ +IBM/S390, version = 00, identification = 145F07, machine = 2964 \ No newline at end of file
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 20.05.20 14:20, Boris Fiuczynski wrote:
On 5/18/20 2:56 PM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virhostcpu.c | 16 +++++++++++++++- .../linux-s390x-with-frequency.signature | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 0ce895cb39..0caf7959ef 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1430,8 +1430,9 @@ virHostCPUReadSignature(virArch arch, g_autofree char *model = NULL; g_autofree char *stepping = NULL; g_autofree char *revision = NULL; + g_autofree char *proc = NULL; - if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch)) + if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch) && !ARCH_IS_S390(arch)) return 0; while (fgets(line, lineLen, cpuinfo)) { @@ -1479,6 +1480,19 @@ virHostCPUReadSignature(virArch arch, *signature = g_strdup_printf("%s, rev %s", name, revision); return 0; } + } else if (ARCH_IS_S390(arch)) { + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "processor 0")) { + if (!proc) + proc = g_steal_pointer(&parts[1]); + }
To catch scenarios of moving from LPAR to nested kvm or the other way around facilties must be added here as well.
Yes. This would also cover the case when a firmware update adds features.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/util/virhostcpu.c | 21 ++++++++++++++++++- .../linux-s390x-with-frequency.signature | 1 + 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index a25a4fa333..39bbcf8ca8 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1430,8 +1430,10 @@ virHostCPUReadSignature(virArch arch, g_autofree char *model = NULL; g_autofree char *stepping = NULL; g_autofree char *revision = NULL; + g_autofree char *proc = NULL; + g_autofree char *facilities = NULL; - if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch)) + if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch) && !ARCH_IS_S390(arch)) return 0; while (fgets(line, lineLen, cpuinfo)) { @@ -1479,6 +1481,23 @@ virHostCPUReadSignature(virArch arch, *signature = g_strdup_printf("%s, rev %s", name, revision); return 0; } + } else if (ARCH_IS_S390(arch)) { + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "processor 0")) { + if (!proc) + proc = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "facilities")) { + if (!facilities) + facilities = g_steal_pointer(&parts[1]); + } + + if (vendor && proc && facilities) { + *signature = g_strdup_printf("%s, %s, facilities: %s", + vendor, proc, facilities); + return 0; + } } } diff --git a/tests/virhostcpudata/linux-s390x-with-frequency.signature b/tests/virhostcpudata/linux-s390x-with-frequency.signature new file mode 100644 index 0000000000..70bb28a154 --- /dev/null +++ b/tests/virhostcpudata/linux-s390x-with-frequency.signature @@ -0,0 +1 @@ +IBM/S390, version = 00, identification = 145F07, machine = 2964, facilities: 0 1 2 3 4 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 30 31 32 33 34 35 36 37 40 41 42 43 44 45 46 47 48 49 50 51 52 53 55 57 64 65 66 67 68 69 70 71 72 73 75 76 77 78 80 128 129 131 132 142 143 \ No newline at end of file -- 2.26.2
Thanks Jiri! Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> On 5/22/20 11:15 AM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/util/virhostcpu.c | 21 ++++++++++++++++++- .../linux-s390x-with-frequency.signature | 1 + 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index a25a4fa333..39bbcf8ca8 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1430,8 +1430,10 @@ virHostCPUReadSignature(virArch arch, g_autofree char *model = NULL; g_autofree char *stepping = NULL; g_autofree char *revision = NULL; + g_autofree char *proc = NULL; + g_autofree char *facilities = NULL;
- if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch)) + if (!ARCH_IS_X86(arch) && !ARCH_IS_PPC64(arch) && !ARCH_IS_S390(arch)) return 0;
while (fgets(line, lineLen, cpuinfo)) { @@ -1479,6 +1481,23 @@ virHostCPUReadSignature(virArch arch, *signature = g_strdup_printf("%s, rev %s", name, revision); return 0; } + } else if (ARCH_IS_S390(arch)) { + if (STREQ(parts[0], "vendor_id")) { + if (!vendor) + vendor = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "processor 0")) { + if (!proc) + proc = g_steal_pointer(&parts[1]); + } else if (STREQ(parts[0], "facilities")) { + if (!facilities) + facilities = g_steal_pointer(&parts[1]); + } + + if (vendor && proc && facilities) { + *signature = g_strdup_printf("%s, %s, facilities: %s", + vendor, proc, facilities); + return 0; + } } }
diff --git a/tests/virhostcpudata/linux-s390x-with-frequency.signature b/tests/virhostcpudata/linux-s390x-with-frequency.signature new file mode 100644 index 0000000000..70bb28a154 --- /dev/null +++ b/tests/virhostcpudata/linux-s390x-with-frequency.signature @@ -0,0 +1 @@ +IBM/S390, version = 00, identification = 145F07, machine = 2964, facilities: 0 1 2 3 4 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 30 31 32 33 34 35 36 37 40 41 42 43 44 45 46 47 48 49 50 51 52 53 55 57 64 65 66 67 68 69 70 71 72 73 75 76 77 78 80 128 129 131 132 142 143 \ No newline at end of file
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
The host CPU related info stored in the capabilities cache is no longer valid after the host CPU changes. This is not a frequent situation in real world, but it can easily happen in nested scenarios when a disk image is started with various CPUs. https://bugzilla.redhat.com/show_bug.cgi?id=1778819 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_capspriv.h | 1 + tests/qemucapsprobe.c | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d9b339cbfb..f12769635a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -669,6 +669,7 @@ struct _virQEMUCaps { unsigned int kvmVersion; unsigned int libvirtVersion; unsigned int microcodeVersion; + char *hostCPUSignature; char *package; char *kernelVersion; @@ -1908,6 +1909,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) ret->version = qemuCaps->version; ret->kvmVersion = qemuCaps->kvmVersion; ret->microcodeVersion = qemuCaps->microcodeVersion; + ret->hostCPUSignature = g_strdup(qemuCaps->hostCPUSignature); ret->package = g_strdup(qemuCaps->package); ret->kernelVersion = g_strdup(qemuCaps->kernelVersion); @@ -1964,6 +1966,7 @@ void virQEMUCapsDispose(void *obj) VIR_FREE(qemuCaps->package); VIR_FREE(qemuCaps->kernelVersion); VIR_FREE(qemuCaps->binary); + VIR_FREE(qemuCaps->hostCPUSignature); VIR_FREE(qemuCaps->gicCapabilities); @@ -4093,6 +4096,7 @@ struct _virQEMUCapsCachePriv { virArch hostArch; unsigned int microcodeVersion; char *kernelVersion; + char *hostCPUSignature; /* cache whether /dev/kvm is usable as runUid:runGuid */ virTristateBool kvmUsable; @@ -4109,6 +4113,7 @@ virQEMUCapsCachePrivFree(void *privData) VIR_FREE(priv->libDir); VIR_FREE(priv->kernelVersion); + VIR_FREE(priv->hostCPUSignature); VIR_FREE(priv); } @@ -4286,6 +4291,8 @@ virQEMUCapsLoadCache(virArch hostArch, goto cleanup; } + qemuCaps->hostCPUSignature = virXPathString("string(./hostCPUSignature)", ctxt); + if (virXPathBoolean("boolean(./package)", ctxt) > 0) { qemuCaps->package = virXPathString("string(./package)", ctxt); if (!qemuCaps->package) @@ -4587,6 +4594,8 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps) virBufferAsprintf(&buf, "<microcodeVersion>%u</microcodeVersion>\n", qemuCaps->microcodeVersion); + virBufferEscapeString(&buf, "<hostCPUSignature>%s</hostCPUSignature>\n", + qemuCaps->hostCPUSignature); if (qemuCaps->package) virBufferAsprintf(&buf, "<package>%s</package>\n", @@ -4814,6 +4823,15 @@ virQEMUCapsIsValid(void *data, } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) { + if (STRNEQ_NULLABLE(priv->hostCPUSignature, qemuCaps->hostCPUSignature)) { + VIR_DEBUG("Outdated capabilities for '%s': host CPU changed " + "('%s' vs '%s')", + qemuCaps->binary, + priv->hostCPUSignature, + qemuCaps->hostCPUSignature); + return false; + } + if (priv->microcodeVersion != qemuCaps->microcodeVersion) { VIR_DEBUG("Outdated capabilities for '%s': microcode version " "changed (%u vs %u)", @@ -5286,6 +5304,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, const char *libDir, uid_t runUid, gid_t runGid, + const char *hostCPUSignature, unsigned int microcodeVersion, const char *kernelVersion) { @@ -5324,6 +5343,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, virQEMUCapsInitHostCPUModel(qemuCaps, hostArch, VIR_DOMAIN_VIRT_QEMU); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) { + qemuCaps->hostCPUSignature = g_strdup(hostCPUSignature); qemuCaps->microcodeVersion = microcodeVersion; qemuCaps->kernelVersion = g_strdup(kernelVersion); @@ -5349,6 +5369,7 @@ virQEMUCapsNewData(const char *binary, priv->libDir, priv->runUid, priv->runGid, + priv->hostCPUSignature, virHostCPUGetMicrocodeVersion(), priv->kernelVersion); } @@ -5448,6 +5469,9 @@ virQEMUCapsCacheNew(const char *libDir, priv->hostArch = virArchFromHost(); + if (virHostCPUGetSignature(&priv->hostCPUSignature) < 0) + goto error; + priv->runUid = runUid; priv->runGid = runGid; priv->kvmUsable = VIR_TRISTATE_BOOL_ABSENT; diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 4c053af195..5d2f448e41 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -33,6 +33,7 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, const char *libDir, uid_t runUid, gid_t runGid, + const char *hostCPUSignature, unsigned int microcodeVersion, const char *kernelVersion); diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c index ea88fd2e8a..26ea9ff5ed 100644 --- a/tests/qemucapsprobe.c +++ b/tests/qemucapsprobe.c @@ -77,7 +77,7 @@ main(int argc, char **argv) return EXIT_FAILURE; if (!(caps = virQEMUCapsNewForBinaryInternal(VIR_ARCH_NONE, argv[1], "/tmp", - -1, -1, 0, NULL))) + -1, -1, NULL, 0, NULL))) return EXIT_FAILURE; virObjectUnref(caps); -- 2.26.2
On a Monday in 2020, Jiri Denemark wrote:
The host CPU related info stored in the capabilities cache is no longer valid after the host CPU changes. This is not a frequent situation in real world, but it can easily happen in nested scenarios when a disk image is started with various CPUs.
https://bugzilla.redhat.com/show_bug.cgi?id=1778819
Jiri Denemark (6): util: Define g_autoptr callback for FILE hostcpu: Introduce virHostCPUGetSignature hostcpu: Implement virHostCPUGetSignature for x86 hostcpu: Implement virHostCPUGetSignature for ppc64 hostcpu: Implement virHostCPUGetSignature for s390 qemu: Invalidate capabilities when host CPU changes
src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 24 ++++ src/qemu/qemu_capspriv.h | 1 + src/util/virfile.h | 1 + src/util/virhostcpu.c | 111 ++++++++++++++++++ src/util/virhostcpu.h | 2 + src/util/virhostcpupriv.h | 4 + tests/qemucapsprobe.c | 2 +- .../linux-ppc64-deconf-cpus.signature | 1 + .../linux-ppc64-subcores1.signature | 1 + .../linux-ppc64-subcores2.signature | 1 + .../linux-ppc64-subcores3.signature | 1 + .../linux-s390x-with-frequency.signature | 1 + .../linux-x86_64-test1.signature | 1 + .../linux-x86_64-test2.signature | 1 + .../linux-x86_64-test3.signature | 1 + .../linux-x86_64-test4.signature | 1 + .../linux-x86_64-test5.signature | 1 + .../linux-x86_64-test6.signature | 1 + .../linux-x86_64-test7.signature | 1 + .../linux-x86_64-test8.signature | 1 + .../linux-x86_64-with-die.signature | 1 + tests/virhostcputest.c | 42 ++++++- 23 files changed, 201 insertions(+), 2 deletions(-) create mode 100644 tests/virhostcpudata/linux-ppc64-deconf-cpus.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores1.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores2.signature create mode 100644 tests/virhostcpudata/linux-ppc64-subcores3.signature create mode 100644 tests/virhostcpudata/linux-s390x-with-frequency.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test1.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test2.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test3.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test4.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test5.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test6.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test7.signature create mode 100644 tests/virhostcpudata/linux-x86_64-test8.signature create mode 100644 tests/virhostcpudata/linux-x86_64-with-die.signature
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (4)
-
Boris Fiuczynski -
Christian Borntraeger -
Jiri Denemark -
Ján Tomko