On 08/12/2016 09:33 AM, Jiri Denemark wrote:
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@redhat.com> --- src/cpu/cpu.c | 37 ++++++++++++--------- 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/qemu/qemu_driver.c | 14 ++------ tests/cputest.c | 4 +-- 8 files changed, 119 insertions(+), 68 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index d6b0372..4ea192c 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
Existing, @failIncompatible description is missing
* @@ -104,25 +105,26 @@ cpuGetSubDriverByName(const char *name) * the @host CPU. */ 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));
The prototype changed such that 'host' and 'xml' could be passed as NULL without a compiler complaint (ok a static analysis complaint). Was that by design?
if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt)))
Having xml as NULL probably doesn't work well here.
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);
Allowing host to be NULL will cause failure in PPC and X86 which perhaps is expected.
cleanup: virCPUDefFree(cpu); @@ -134,8 +136,9 @@ cpuCompareXML(virCPUDefPtr host,
/** - * cpuCompare: + * virCPUCompare: * + * @arch: CPU architecture * @host: host CPU definition * @cpu: either guest or host CPU to be compared with @host * @@ -147,21 +150,23 @@ cpuCompareXML(virCPUDefPtr host, * the @host CPU. */ 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 77ccb38..0e81e91 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);
Was removing NONNULL by design? I think it needs to be replaced for xml at least.
+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 3b68d83..2cef5bc 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 782c917..e0987c4 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)) {
Shall I assume this related to the compare patch? or a fix as a result? I assume it's related to changes in x86Compute and x86GuestData, but it wasn't really clear from the commit message.
if (!(model = x86ModelFind(map, cpu->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model);
[...]