[libvirt] [PATCH 0/7] qemu: Filter CPU features returned by qemuConnectBaselineCPU

The host CPU definitions reported in the capabilities XML may contain CPU features unknown to QEMU, but the result of virConnectBaselineCPU is supposed to be directly usable as a guest CPU definition and thus it should only contain features QEMU knows about. https://bugzilla.redhat.com/show_bug.cgi?id=1450317 Jiri Denemark (7): cpu_conf: Introduce virCPUDefList{Parse,Free} cpu: Use virCPUDefListParse in cpuBaselineXML cpu: Don't log CPU models in cpuBaselineXML cpu: Drop cpuBaselineXML qemu: Pass virArch * to virQEMUCapsCPUFilterFeatures qemu: Publish virQEMUCapsCPUFilterFeatures qemu: Filter CPU features returned by qemuConnectBaselineCPU src/bhyve/bhyve_driver.c | 22 +++++++-- src/conf/cpu_conf.c | 78 ++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 7 +++ src/cpu/cpu.c | 104 ------------------------------------------- src/cpu/cpu.h | 7 --- src/libvirt_private.syms | 3 +- src/libxl/libxl_driver.c | 22 +++++++-- src/qemu/qemu_capabilities.c | 8 ++-- src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_driver.c | 32 +++++++++++-- src/test/test_driver.c | 22 +++++++-- src/vz/vz_driver.c | 22 ++++++++- 12 files changed, 201 insertions(+), 129 deletions(-) -- 2.14.1

For parsing a list of CPU XMLs into a NULL-terminated list of CPU defs. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 7 +++++ src/libvirt_private.syms | 2 ++ 3 files changed, 87 insertions(+) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index c21d11d244..7514842059 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -29,9 +29,12 @@ #include "cpu_conf.h" #include "domain_conf.h" #include "virstring.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_CPU +VIR_LOG_INIT("conf.cpu_conf"); + VIR_ENUM_IMPL(virCPU, VIR_CPU_TYPE_LAST, "host", "guest", "auto") @@ -939,3 +942,78 @@ virCPUDefIsEqual(virCPUDefPtr src, cleanup: return identical; } + + +/* + * Parses a list of CPU XMLs into a NULL-terminated list of CPU defs. + */ +virCPUDefPtr * +virCPUDefListParse(const char **xmlCPUs, + unsigned int ncpus, + virCPUType cpuType) +{ + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + virCPUDefPtr *cpus = NULL; + size_t i; + + VIR_DEBUG("xmlCPUs=%p, ncpus=%u", xmlCPUs, ncpus); + + if (xmlCPUs) { + for (i = 0; i < ncpus; i++) + VIR_DEBUG("xmlCPUs[%zu]=%s", i, NULLSTR(xmlCPUs[i])); + } + + if (!xmlCPUs && ncpus != 0) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("nonzero ncpus doesn't match with NULL xmlCPUs")); + goto error; + } + + if (ncpus < 1) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("no CPUs given")); + goto error; + } + + if (VIR_ALLOC_N(cpus, ncpus + 1)) + goto error; + + for (i = 0; i < ncpus; i++) { + if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], _("(CPU_definition)"), &ctxt))) + goto error; + + if (virCPUDefParseXML(ctxt, NULL, cpuType, &cpus[i]) < 0) + goto error; + + xmlXPathFreeContext(ctxt); + xmlFreeDoc(doc); + ctxt = NULL; + doc = NULL; + } + + return cpus; + + error: + virCPUDefListFree(cpus); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(doc); + return NULL; +} + + +/* + * Frees NULL-terminated list of CPUs created by virCPUDefListParse. + */ +void +virCPUDefListFree(virCPUDefPtr *cpus) +{ + virCPUDefPtr *cpu; + + if (!cpus) + return; + + for (cpu = cpus; *cpu != NULL; cpu++) + virCPUDefFree(*cpu); + + VIR_FREE(cpus); +} diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index b44974f47e..d3e2c84102 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -218,4 +218,11 @@ virCPUDefUpdateFeature(virCPUDefPtr cpu, const char *name, int policy); +virCPUDefPtr * +virCPUDefListParse(const char **xmlCPUs, + unsigned int ncpus, + virCPUType cpuType); +void +virCPUDefListFree(virCPUDefPtr *cpus); + #endif /* __VIR_CPU_CONF_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 247d1175ba..857e417f94 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -82,6 +82,8 @@ virCPUDefFree; virCPUDefFreeFeatures; virCPUDefFreeModel; virCPUDefIsEqual; +virCPUDefListFree; +virCPUDefListParse; virCPUDefParseXML; virCPUDefStealModel; virCPUDefUpdateFeature; -- 2.14.1

On Thu, Sep 14, 2017 at 12:57:14PM +0200, Jiri Denemark wrote:
For parsing a list of CPU XMLs into a NULL-terminated list of CPU defs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 7 +++++ src/libvirt_private.syms | 2 ++ 3 files changed, 87 insertions(+)
[...]
+/* + * Parses a list of CPU XMLs into a NULL-terminated list of CPU defs. + */ +virCPUDefPtr * +virCPUDefListParse(const char **xmlCPUs, + unsigned int ncpus, + virCPUType cpuType) +{ +
[...]
+ if (ncpus < 1) {
Interesting way of spelling '== 0'
+ virReportError(VIR_ERR_INVALID_ARG, "%s", _("no CPUs given")); + goto error; + } +
Jan

On Fri, Sep 15, 2017 at 14:04:42 +0200, Ján Tomko wrote:
On Thu, Sep 14, 2017 at 12:57:14PM +0200, Jiri Denemark wrote:
For parsing a list of CPU XMLs into a NULL-terminated list of CPU defs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/cpu_conf.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 7 +++++ src/libvirt_private.syms | 2 ++ 3 files changed, 87 insertions(+)
[...]
+/* + * Parses a list of CPU XMLs into a NULL-terminated list of CPU defs. + */ +virCPUDefPtr * +virCPUDefListParse(const char **xmlCPUs, + unsigned int ncpus, + virCPUType cpuType) +{ +
[...]
+ if (ncpus < 1) {
Interesting way of spelling '== 0'
Yeah, caused by copy&paste engineering... see the next patch where the same condition gets removed from cpuBaselineXML. Fixed. Jirka

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu.c | 52 ++++++---------------------------------------------- 1 file changed, 6 insertions(+), 46 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 96160901e1..bf3c6d53dd 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -523,11 +523,9 @@ cpuBaselineXML(const char **xmlCPUs, unsigned int nmodels, unsigned int flags) { - xmlDocPtr doc = NULL; - xmlXPathContextPtr ctxt = NULL; virCPUDefPtr *cpus = NULL; virCPUDefPtr cpu = NULL; - char *cpustr; + char *cpustr = NULL; size_t i; VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels); @@ -535,67 +533,29 @@ cpuBaselineXML(const char **xmlCPUs, virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); - if (xmlCPUs) { - for (i = 0; i < ncpus; i++) - VIR_DEBUG("xmlCPUs[%zu]=%s", i, NULLSTR(xmlCPUs[i])); - } if (models) { for (i = 0; i < nmodels; i++) VIR_DEBUG("models[%zu]=%s", i, NULLSTR(models[i])); } - if (xmlCPUs == NULL && ncpus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("nonzero ncpus doesn't match with NULL xmlCPUs")); - return NULL; - } - - if (ncpus < 1) { - virReportError(VIR_ERR_INVALID_ARG, "%s", _("No CPUs given")); - return NULL; - } - - if (VIR_ALLOC_N(cpus, ncpus)) - goto error; - - for (i = 0; i < ncpus; i++) { - if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], _("(CPU_definition)"), &ctxt))) - goto error; - - if (virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_HOST, &cpus[i]) < 0) - goto error; - - xmlXPathFreeContext(ctxt); - xmlFreeDoc(doc); - ctxt = NULL; - doc = NULL; - } + if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) + goto cleanup; if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)))) - goto error; + goto cleanup; if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && virCPUExpandFeatures(cpus[0]->arch, cpu) < 0) - goto error; + goto cleanup; cpustr = virCPUDefFormat(cpu, NULL, false); cleanup: - if (cpus) { - for (i = 0; i < ncpus; i++) - virCPUDefFree(cpus[i]); - VIR_FREE(cpus); - } + virCPUDefListFree(cpus); virCPUDefFree(cpu); - xmlXPathFreeContext(ctxt); - xmlFreeDoc(doc); return cpustr; - - error: - cpustr = NULL; - goto cleanup; } -- 2.14.1

They are logged in cpuBaseline anyway. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index bf3c6d53dd..e75f406040 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -526,18 +526,12 @@ cpuBaselineXML(const char **xmlCPUs, virCPUDefPtr *cpus = NULL; virCPUDefPtr cpu = NULL; char *cpustr = NULL; - size_t i; VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels); virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); - if (models) { - for (i = 0; i < nmodels; i++) - VIR_DEBUG("models[%zu]=%s", i, NULLSTR(models[i])); - } - if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) goto cleanup; -- 2.14.1

The implementation of virConnectBaselineCPU may be different for each hypervisor. Thus it shouldn't really be implmented in the cpu code. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/bhyve/bhyve_driver.c | 22 +++++++++++++++--- src/cpu/cpu.c | 58 ------------------------------------------------ src/cpu/cpu.h | 7 ------ src/libvirt_private.syms | 1 - src/libxl/libxl_driver.c | 22 +++++++++++++++--- src/qemu/qemu_driver.c | 22 +++++++++++++++--- src/test/test_driver.c | 22 +++++++++++++++--- src/vz/vz_driver.c | 22 +++++++++++++++++- 8 files changed, 97 insertions(+), 79 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 3bcff88975..e8241f39ff 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1420,7 +1420,9 @@ bhyveConnectBaselineCPU(virConnectPtr conn, unsigned int ncpus, unsigned int flags) { - char *cpu = NULL; + virCPUDefPtr *cpus = NULL; + virCPUDefPtr cpu = NULL; + char *cpustr = NULL; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -1428,10 +1430,24 @@ bhyveConnectBaselineCPU(virConnectPtr conn, if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; - cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); + if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) + goto cleanup; + + if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0, + !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)))) + goto cleanup; + + if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && + virCPUExpandFeatures(cpus[0]->arch, cpu) < 0) + goto cleanup; + + cpustr = virCPUDefFormat(cpu, NULL, false); cleanup: - return cpu; + virCPUDefListFree(cpus); + virCPUDefFree(cpu); + + return cpustr; } static int diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index e75f406040..a7c7c381b9 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -495,64 +495,6 @@ virCPUProbeHost(virArch arch) } -/** - * cpuBaselineXML: - * - * @xmlCPUs: list of host CPU XML descriptions - * @ncpus: number of CPUs in @xmlCPUs - * @models: list of CPU models that can be considered for the baseline CPU - * @nmodels: number of CPU models in @models - * @flags: bitwise-OR of virConnectBaselineCPUFlags - * - * Computes the most feature-rich CPU which is compatible with all given - * host CPUs. If @models array is NULL, all models supported by libvirt will - * be considered when computing the baseline CPU model, otherwise the baseline - * CPU model will be one of the provided CPU @models. - * - * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt - * will explicitly list all CPU features that are part of the host CPU, - * without this flag features that are part of the CPU model will not be - * listed. - * - * Returns XML description of the baseline CPU or NULL on error. - */ -char * -cpuBaselineXML(const char **xmlCPUs, - unsigned int ncpus, - const char **models, - unsigned int nmodels, - unsigned int flags) -{ - virCPUDefPtr *cpus = NULL; - virCPUDefPtr cpu = NULL; - char *cpustr = NULL; - - VIR_DEBUG("ncpus=%u, nmodels=%u", ncpus, nmodels); - - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | - VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); - - if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) - goto cleanup; - - if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, - !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)))) - goto cleanup; - - if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && - virCPUExpandFeatures(cpus[0]->arch, cpu) < 0) - goto cleanup; - - cpustr = virCPUDefFormat(cpu, NULL, false); - - cleanup: - virCPUDefListFree(cpus); - virCPUDefFree(cpu); - - return cpustr; -} - - /** * cpuBaseline: * diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index c6ca111e97..5dda46ee70 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -196,13 +196,6 @@ virCPUGetHost(virArch arch, virCPUDefPtr virCPUProbeHost(virArch arch); -char * -cpuBaselineXML(const char **xmlCPUs, - unsigned int ncpus, - const char **models, - unsigned int nmodels, - unsigned int flags); - virCPUDefPtr cpuBaseline (virCPUDefPtr *cpus, unsigned int ncpus, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 857e417f94..888e4e329b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1077,7 +1077,6 @@ virStoragePoolObjVolumeListExport; # cpu/cpu.h cpuBaseline; -cpuBaselineXML; cpuDecode; cpuEncode; virCPUCheckFeature; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 8fefce6631..4861e5db21 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -6450,7 +6450,9 @@ libxlConnectBaselineCPU(virConnectPtr conn, unsigned int ncpus, unsigned int flags) { - char *cpu = NULL; + virCPUDefPtr *cpus = NULL; + virCPUDefPtr cpu = NULL; + char *cpustr = NULL; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -6458,10 +6460,24 @@ libxlConnectBaselineCPU(virConnectPtr conn, if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; - cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); + if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) + goto cleanup; + + if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0, + !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)))) + goto cleanup; + + if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && + virCPUExpandFeatures(cpus[0]->arch, cpu) < 0) + goto cleanup; + + cpustr = virCPUDefFormat(cpu, NULL, false); cleanup: - return cpu; + virCPUDefListFree(cpus); + virCPUDefFree(cpu); + + return cpustr; } static virHypervisorDriver libxlHypervisorDriver = { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b334cf20be..e92c114f3e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12989,7 +12989,9 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int ncpus, unsigned int flags) { - char *cpu = NULL; + virCPUDefPtr *cpus = NULL; + virCPUDefPtr cpu = NULL; + char *cpustr = NULL; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -12997,10 +12999,24 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, if (virConnectBaselineCPUEnsureACL(conn) < 0) goto cleanup; - cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); + if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) + goto cleanup; + + if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0, + !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)))) + goto cleanup; + + if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && + virCPUExpandFeatures(cpus[0]->arch, cpu) < 0) + goto cleanup; + + cpustr = virCPUDefFormat(cpu, NULL, false); cleanup: - return cpu; + virCPUDefListFree(cpus); + virCPUDefFree(cpu); + + return cpustr; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index aa38f54dd9..6e8a4b5782 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1535,13 +1535,29 @@ testConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int ncpus, unsigned int flags) { - char *cpu; + virCPUDefPtr *cpus = NULL; + virCPUDefPtr cpu = NULL; + char *cpustr = NULL; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); - cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); + if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) + goto cleanup; - return cpu; + if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0, false))) + goto cleanup; + + if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && + virCPUExpandFeatures(cpus[0]->arch, cpu) < 0) + goto cleanup; + + cpustr = virCPUDefFormat(cpu, NULL, false); + + cleanup: + virCPUDefListFree(cpus); + virCPUDefFree(cpu); + + return cpustr; } static int testNodeGetInfo(virConnectPtr conn, diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 6f4aee3652..daeed5f114 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -945,12 +945,32 @@ vzConnectBaselineCPU(virConnectPtr conn, unsigned int ncpus, unsigned int flags) { + virCPUDefPtr *cpus = NULL; + virCPUDefPtr cpu = NULL; + char *cpustr = NULL; + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); if (virConnectBaselineCPUEnsureACL(conn) < 0) return NULL; - return cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); + if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) + goto cleanup; + + if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0, false))) + goto cleanup; + + if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && + virCPUExpandFeatures(cpus[0]->arch, cpu) < 0) + goto cleanup; + + cpustr = virCPUDefFormat(cpu, NULL, false); + + cleanup: + virCPUDefListFree(cpus); + virCPUDefFree(cpu); + + return cpustr; } -- 2.14.1

The filter only needs to know the CPU architecture. Passing virQEMUCapsPtr as opaque is a bit overkill. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c690cb3498..52d63f44ec 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3320,9 +3320,9 @@ static bool virQEMUCapsCPUFilterFeatures(const char *name, void *opaque) { - virQEMUCapsPtr qemuCaps = opaque; + virArch *arch = opaque; - if (!ARCH_IS_X86(qemuCaps->arch)) + if (!ARCH_IS_X86(*arch)) return true; if (STREQ(name, "cmt") || @@ -3534,7 +3534,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, if (!hostCPU || virCPUDefCopyModelFilter(cpu, hostCPU, true, virQEMUCapsCPUFilterFeatures, - qemuCaps) < 0) + &qemuCaps->arch) < 0) goto error; } else if (type == VIR_DOMAIN_VIRT_KVM && virCPUGetHostIsSupported(qemuCaps->arch)) { -- 2.14.1

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 52d63f44ec..7bc88be0f5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3316,7 +3316,7 @@ virQEMUCapsProbeQMPGICCapabilities(virQEMUCapsPtr qemuCaps, } -static bool +bool virQEMUCapsCPUFilterFeatures(const char *name, void *opaque) { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 85c390abf5..1400e885d9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -547,4 +547,7 @@ int virQEMUCapsFillDomainCaps(virCapsPtr caps, bool virQEMUCapsGuestIsNative(virArch host, virArch guest); +bool virQEMUCapsCPUFilterFeatures(const char *name, + void *opaque); + #endif /* __QEMU_CAPABILITIES_H__*/ -- 2.14.1

The host CPU definitions reported in the capabilities XML may contain CPU features unknown to QEMU, but the result of virConnectBaselineCPU is supposed to be directly usable as a guest CPU definition and thus it should only contain features QEMU knows about. https://bugzilla.redhat.com/show_bug.cgi?id=1450317 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e92c114f3e..e1a0dd553e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12990,6 +12990,7 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int flags) { virCPUDefPtr *cpus = NULL; + virCPUDefPtr baseline = NULL; virCPUDefPtr cpu = NULL; char *cpustr = NULL; @@ -13002,8 +13003,16 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, if (!(cpus = virCPUDefListParse(xmlCPUs, ncpus, VIR_CPU_TYPE_HOST))) goto cleanup; - if (!(cpu = cpuBaseline(cpus, ncpus, NULL, 0, - !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)))) + if (!(baseline = cpuBaseline(cpus, ncpus, NULL, 0, + !!(flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE)))) + goto cleanup; + + if (!(cpu = virCPUDefCopyWithoutModel(baseline))) + goto cleanup; + + if (virCPUDefCopyModelFilter(cpu, baseline, false, + virQEMUCapsCPUFilterFeatures, + &cpus[0]->arch) < 0) goto cleanup; if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) && @@ -13014,6 +13023,7 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, cleanup: virCPUDefListFree(cpus); + virCPUDefFree(baseline); virCPUDefFree(cpu); return cpustr; -- 2.14.1

On Thu, Sep 14, 2017 at 12:57:13PM +0200, Jiri Denemark wrote:
The host CPU definitions reported in the capabilities XML may contain CPU features unknown to QEMU, but the result of virConnectBaselineCPU is supposed to be directly usable as a guest CPU definition and thus it should only contain features QEMU knows about.
https://bugzilla.redhat.com/show_bug.cgi?id=1450317
Jiri Denemark (7): cpu_conf: Introduce virCPUDefList{Parse,Free} cpu: Use virCPUDefListParse in cpuBaselineXML cpu: Don't log CPU models in cpuBaselineXML cpu: Drop cpuBaselineXML qemu: Pass virArch * to virQEMUCapsCPUFilterFeatures qemu: Publish virQEMUCapsCPUFilterFeatures qemu: Filter CPU features returned by qemuConnectBaselineCPU
src/bhyve/bhyve_driver.c | 22 +++++++-- src/conf/cpu_conf.c | 78 ++++++++++++++++++++++++++++++++ src/conf/cpu_conf.h | 7 +++ src/cpu/cpu.c | 104 ------------------------------------------- src/cpu/cpu.h | 7 --- src/libvirt_private.syms | 3 +- src/libxl/libxl_driver.c | 22 +++++++-- src/qemu/qemu_capabilities.c | 8 ++-- src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_driver.c | 32 +++++++++++-- src/test/test_driver.c | 22 +++++++-- src/vz/vz_driver.c | 22 ++++++++- 12 files changed, 201 insertions(+), 129 deletions(-)
ACK series Jan
participants (2)
-
Jiri Denemark
-
Ján Tomko