[libvirt] [PATCH v2 43/45] cpu: Rework cpuCompare* APIs

Both cpuCompare* APIs are renamed to virCPUCompare*. And they should now work for any guest CPU definition, i.e., even for host-passthrough (trivial) and host-model CPUs. The implementation in x86 driver is enhanced to provide a hint about -noTSX Broadwell and Haswell models when appropriate. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 2: - adapted to changes in previous patches - added check for xml == NULL in virCPUCompareXML src/cpu/cpu.c | 42 +++++++++++++++--------- src/cpu/cpu.h | 21 ++++++------ src/cpu/cpu_arm.c | 8 ++--- src/cpu/cpu_ppc64.c | 15 +++++++-- src/cpu/cpu_x86.c | 84 ++++++++++++++++++++++++++++++++++++------------ src/libvirt_private.syms | 4 +-- src/libxl/libxl_driver.c | 14 ++------ src/qemu/qemu_driver.c | 14 ++------ tests/cputest.c | 4 +-- 9 files changed, 126 insertions(+), 80 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index ba5bf1a..995d925 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -91,8 +91,9 @@ cpuGetSubDriverByName(const char *name) /** - * cpuCompareXML: + * virCPUCompareXML: * + * @arch: CPU architecture * @host: host CPU definition * @xml: XML description of either guest or host CPU to be compared with @host * @failIncompatible: return an error instead of VIR_CPU_COMPARE_INCOMPATIBLE @@ -107,25 +108,31 @@ cpuGetSubDriverByName(const char *name) * two CPUs are incompatible. */ virCPUCompareResult -cpuCompareXML(virCPUDefPtr host, - const char *xml, - bool failIncompatible) +virCPUCompareXML(virArch arch, + virCPUDefPtr host, + const char *xml, + bool failIncompatible) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; virCPUDefPtr cpu = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; - VIR_DEBUG("host=%p, xml=%s", host, NULLSTR(xml)); + VIR_DEBUG("arch=%s, host=%p, xml=%s", + virArchToString(arch), host, NULLSTR(xml)); + + if (!xml) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("missing CPU definition")); + goto cleanup; + } if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt))) goto cleanup; - cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO); - if (cpu == NULL) + if (!(cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO))) goto cleanup; - ret = cpuCompare(host, cpu, failIncompatible); + ret = virCPUCompare(arch, host, cpu, failIncompatible); cleanup: virCPUDefFree(cpu); @@ -137,8 +144,9 @@ cpuCompareXML(virCPUDefPtr host, /** - * cpuCompare: + * virCPUCompare: * + * @arch: CPU architecture * @host: host CPU definition * @cpu: either guest or host CPU to be compared with @host * @failIncompatible: return an error instead of VIR_CPU_COMPARE_INCOMPATIBLE @@ -153,21 +161,23 @@ cpuCompareXML(virCPUDefPtr host, * two CPUs are incompatible. */ virCPUCompareResult -cpuCompare(virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) +virCPUCompare(virArch arch, + virCPUDefPtr host, + virCPUDefPtr cpu, + bool failIncompatible) { struct cpuArchDriver *driver; - VIR_DEBUG("host=%p, cpu=%p", host, cpu); + VIR_DEBUG("arch=%s, host=%p, cpu=%p", + virArchToString(arch), host, cpu); - if ((driver = cpuGetSubDriver(host->arch)) == NULL) + if (!(driver = cpuGetSubDriver(arch))) return VIR_CPU_COMPARE_ERROR; - if (driver->compare == NULL) { + if (!driver->compare) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot compare CPUs of %s architecture"), - virArchToString(host->arch)); + virArchToString(arch)); return VIR_CPU_COMPARE_ERROR; } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index f14c2c8..51bacb7 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -45,7 +45,7 @@ struct _virCPUData { typedef virCPUCompareResult -(*cpuArchCompare) (virCPUDefPtr host, +(*virCPUArchCompare)(virCPUDefPtr host, virCPUDefPtr cpu, bool failIncompatible); @@ -116,7 +116,7 @@ struct cpuArchDriver { const char *name; const virArch *arch; unsigned int narch; - cpuArchCompare compare; + virCPUArchCompare compare; cpuArchDecode decode; cpuArchEncode encode; cpuArchDataFree free; @@ -134,16 +134,17 @@ struct cpuArchDriver { virCPUCompareResult -cpuCompareXML(virCPUDefPtr host, - const char *xml, - bool failIncompatible) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +virCPUCompareXML(virArch arch, + virCPUDefPtr host, + const char *xml, + bool failIncompatible); virCPUCompareResult -cpuCompare (virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +virCPUCompare(virArch arch, + virCPUDefPtr host, + virCPUDefPtr cpu, + bool failIncompatible) + ATTRIBUTE_NONNULL(3); int cpuDecode (virCPUDefPtr cpu, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 37bb71a..db603a6 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -112,9 +112,9 @@ armBaseline(virCPUDefPtr *cpus, } static virCPUCompareResult -armCompare(virCPUDefPtr host ATTRIBUTE_UNUSED, - virCPUDefPtr cpu ATTRIBUTE_UNUSED, - bool failMessages ATTRIBUTE_UNUSED) +virCPUarmCompare(virCPUDefPtr host ATTRIBUTE_UNUSED, + virCPUDefPtr cpu ATTRIBUTE_UNUSED, + bool failMessages ATTRIBUTE_UNUSED) { return VIR_CPU_COMPARE_IDENTICAL; } @@ -123,7 +123,7 @@ struct cpuArchDriver cpuDriverArm = { .name = "arm", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = armCompare, + .compare = virCPUarmCompare, .decode = NULL, .encode = NULL, .free = armDataFree, diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 6f005e5..21ea0e1 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -634,13 +634,24 @@ ppc64Compute(virCPUDefPtr host, } static virCPUCompareResult -ppc64DriverCompare(virCPUDefPtr host, +virCPUppc64Compare(virCPUDefPtr host, virCPUDefPtr cpu, bool failIncompatible) { virCPUCompareResult ret; char *message = NULL; + if (!host || !host->model) { + if (failIncompatible) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + _("unknown host CPU")); + } else { + VIR_WARN("unknown host CPU"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + } + return -1; + } + ret = ppc64Compute(host, cpu, NULL, &message); if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { @@ -902,7 +913,7 @@ struct cpuArchDriver cpuDriverPPC64 = { .name = "ppc64", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = ppc64DriverCompare, + .compare = virCPUppc64Compare, .decode = ppc64DriverDecode, .encode = NULL, .free = ppc64DriverFree, diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 7224d76..6397dcb 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1057,7 +1057,8 @@ x86ModelFromCPU(const virCPUDef *cpu, policy != -1) return x86ModelNew(); - if (policy == VIR_CPU_FEATURE_REQUIRE || policy == -1) { + if (cpu->model && + (policy == VIR_CPU_FEATURE_REQUIRE || policy == -1)) { if (!(model = x86ModelFind(map, cpu->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model); @@ -1519,12 +1520,6 @@ x86Compute(virCPUDefPtr host, virArch arch; size_t i; - if (!cpu->model) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("no guest CPU model specified")); - return VIR_CPU_COMPARE_ERROR; - } - if (cpu->arch != VIR_ARCH_NONE) { bool found = false; @@ -1565,7 +1560,7 @@ x86Compute(virCPUDefPtr host, } if (!(map = virCPUx86GetMap()) || - !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE)) || + !(host_model = x86ModelFromCPU(host, map, -1)) || !(cpu_force = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORCE)) || !(cpu_require = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_REQUIRE)) || !(cpu_optional = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_OPTIONAL)) || @@ -1664,25 +1659,68 @@ x86Compute(virCPUDefPtr host, static virCPUCompareResult -x86Compare(virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) +virCPUx86Compare(virCPUDefPtr host, + virCPUDefPtr cpu, + bool failIncompatible) { - virCPUCompareResult ret; + virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; + virCPUx86MapPtr map; + virCPUx86ModelPtr model = NULL; char *message = NULL; + if (!host || !host->model) { + if (failIncompatible) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + _("unknown host CPU")); + } else { + VIR_WARN("unknown host CPU"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + } + goto cleanup; + } + ret = x86Compute(host, cpu, NULL, &message); - if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { - ret = VIR_CPU_COMPARE_ERROR; - if (message) { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); - } else { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + if (ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + bool noTSX = false; + + if (STREQ_NULLABLE(cpu->model, "Haswell") || + STREQ_NULLABLE(cpu->model, "Broadwell")) { + if (!(map = virCPUx86GetMap())) + goto cleanup; + + if (!(model = x86ModelFromCPU(cpu, map, -1))) + goto cleanup; + + noTSX = !x86FeatureInData("hle", &model->data, map) || + !x86FeatureInData("rtm", &model->data, map); + } + + if (failIncompatible) { + ret = VIR_CPU_COMPARE_ERROR; + if (message) { + if (noTSX) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("%s; try using '%s-noTSX' CPU model"), + message, cpu->model); + } else { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); + } + } else { + if (noTSX) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("try using '%s-noTSX' CPU model"), + cpu->model); + } else { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } + } } } - VIR_FREE(message); + cleanup: + VIR_FREE(message); + x86ModelFree(model); return ret; } @@ -1693,6 +1731,12 @@ x86GuestData(virCPUDefPtr host, virCPUDataPtr *data, char **message) { + if (!guest->model) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("no guest CPU model specified")); + return VIR_CPU_COMPARE_ERROR; + } + return x86Compute(host, guest, data, message); } @@ -2719,7 +2763,7 @@ struct cpuArchDriver cpuDriverX86 = { .name = "x86", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = x86Compare, + .compare = virCPUx86Compare, .decode = x86DecodeCPUData, .encode = x86Encode, .free = x86FreeCPUData, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c4fd7ae..0834d50 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -964,8 +964,6 @@ virSecretObjSetValueSize; # cpu/cpu.h cpuBaseline; cpuBaselineXML; -cpuCompare; -cpuCompareXML; cpuDataFormat; cpuDataFree; cpuDataParse; @@ -975,6 +973,8 @@ cpuGetModels; cpuGuestData; cpuNodeData; virCPUCheckFeature; +virCPUCompare; +virCPUCompareXML; virCPUDataCheckFeature; virCPUTranslate; virCPUUpdate; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 04ffaa6..42fa129 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6369,18 +6369,8 @@ libxlConnectCompareCPU(virConnectPtr conn, cfg = libxlDriverConfigGet(driver); - if (!cfg->caps->host.cpu || - !cfg->caps->host.cpu->model) { - if (failIncompatible) { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", - _("cannot get host CPU capabilities")); - } else { - VIR_WARN("cannot get host CPU capabilities"); - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - } - } else { - ret = cpuCompareXML(cfg->caps->host.cpu, xmlDesc, failIncompatible); - } + ret = virCPUCompareXML(cfg->caps->host.arch, cfg->caps->host.cpu, + xmlDesc, failIncompatible); virObjectUnref(cfg); return ret; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ca47b4e..9995265 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12809,18 +12809,8 @@ qemuConnectCompareCPU(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (!caps->host.cpu || - !caps->host.cpu->model) { - if (failIncompatible) { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", - _("cannot get host CPU capabilities")); - } else { - VIR_WARN("cannot get host CPU capabilities"); - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - } - } else { - ret = cpuCompareXML(caps->host.cpu, xmlDesc, failIncompatible); - } + ret = virCPUCompareXML(caps->host.arch, caps->host.cpu, + xmlDesc, failIncompatible); cleanup: virObjectUnref(caps); diff --git a/tests/cputest.c b/tests/cputest.c index 72f00b6..6842d27 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -215,7 +215,7 @@ cpuTestCompare(const void *arg) !(cpu = cpuTestLoadXML(data->arch, data->name))) goto cleanup; - result = cpuCompare(host, cpu, false); + result = virCPUCompare(host->arch, host, cpu, false); if (data->result == VIR_CPU_COMPARE_ERROR) virResetLastError(); @@ -360,7 +360,7 @@ cpuTestBaseline(const void *arg) for (i = 0; i < ncpus; i++) { virCPUCompareResult cmp; - cmp = cpuCompare(cpus[i], baseline, false); + cmp = virCPUCompare(cpus[i]->arch, cpus[i], baseline, false); if (cmp != VIR_CPU_COMPARE_SUPERSET && cmp != VIR_CPU_COMPARE_IDENTICAL) { VIR_TEST_VERBOSE("\nbaseline CPU is incompatible with CPU %zu\n", -- 2.10.0