[libvirt] [PATCH 0/9] qemu: More CPU enhancements and fixes
This is technically a v2 of the not yet pushed part of "qemu: Use migratable host CPU model from QEMU" series, but the patches were significantly rewritten based on the review comments and on the need for additional regression fix. Current libvirt makes a migratable host CPU model by removing features marked in cpu_map.xml with migratable='no'. This is not ideal because cpu_map.xml is a static database while feature migratability may differ depending on a hypervisor or its version. Thus we should preferably use the data we got from QEMU. And to make things even more complicated, we need to use both CPUID data we probed and data from QEMU when checking whether a guest CPU can run on the host to maintain backward compatibility with older libvirt. Thus we need to store three different views of the host CPU model with each QEMU binary. Jiri Denemark (9): qemu: Introduce virQEMUCapsSetHostModel qemu: Move qemuCaps CPU data copying into a separate function qemu: Introduce virQEMUCapsHostCPUDataClear qemu: Move qemuCaps host CPU data in a struct qemu: Prepare qemuCaps for multiple host CPU defs qemu: Pass migratable host CPU model to virCPUUpdate cpu: Drop feature filtering from virCPUUpdate cpu: Introduce virCPUGetHostIsSupported qemu: Use more data for comparing CPUs src/cpu/cpu.c | 20 +++ src/cpu/cpu.h | 3 + src/cpu/cpu_x86.c | 9 +- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 288 +++++++++++++++++++++++++++++-------------- src/qemu/qemu_capabilities.h | 16 ++- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_process.c | 6 +- tests/cputest.c | 7 +- 9 files changed, 247 insertions(+), 106 deletions(-) -- 2.12.2
A simple helper as a complement to virQEMUCapsGetHostModel. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3ab99f450..d2b6b7604 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2455,6 +2455,18 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, } +static void +virQEMUCapsSetHostModel(virQEMUCapsPtr qemuCaps, + virDomainVirtType type, + virCPUDefPtr cpu) +{ + if (type == VIR_DOMAIN_VIRT_KVM) + qemuCaps->kvmCPUModel = cpu; + else + qemuCaps->tcgCPUModel = cpu; +} + + bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, virCapsPtr caps, @@ -3336,10 +3348,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, goto error; } - if (type == VIR_DOMAIN_VIRT_KVM) - qemuCaps->kvmCPUModel = cpu; - else - qemuCaps->tcgCPUModel = cpu; + virQEMUCapsSetHostModel(qemuCaps, type, cpu); cleanup: virCPUDefFree(hostCPU); -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:50PM +0200, Jiri Denemark wrote:
A simple helper as a complement to virQEMUCapsGetHostModel.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
ACK Pavel
This introduces virQEMUCapsHostCPUDataCopy which will later be refactored a bit and called twice from virQEMUCapsNewCopy. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d2b6b7604..218fd50b2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2118,6 +2118,30 @@ virQEMUCapsNew(void) } +static int +virQEMUCapsHostCPUDataCopy(virQEMUCapsPtr dst, + virQEMUCapsPtr src) +{ + if (src->kvmCPUModel && + !(dst->kvmCPUModel = virCPUDefCopy(src->kvmCPUModel))) + return -1; + + if (src->tcgCPUModel && + !(dst->tcgCPUModel = virCPUDefCopy(src->tcgCPUModel))) + return -1; + + if (src->kvmCPUModelInfo && + !(dst->kvmCPUModelInfo = qemuMonitorCPUModelInfoCopy(src->kvmCPUModelInfo))) + return -1; + + if (src->tcgCPUModelInfo && + !(dst->tcgCPUModelInfo = qemuMonitorCPUModelInfoCopy(src->tcgCPUModelInfo))) + return -1; + + return 0; +} + + virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) { virQEMUCapsPtr ret = virQEMUCapsNew(); @@ -2155,20 +2179,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) goto error; } - if (qemuCaps->kvmCPUModel && - !(ret->kvmCPUModel = virCPUDefCopy(qemuCaps->kvmCPUModel))) - goto error; - - if (qemuCaps->tcgCPUModel && - !(ret->tcgCPUModel = virCPUDefCopy(qemuCaps->tcgCPUModel))) - goto error; - - if (qemuCaps->kvmCPUModelInfo && - !(ret->kvmCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->kvmCPUModelInfo))) - goto error; - - if (qemuCaps->tcgCPUModelInfo && - !(ret->tcgCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->tcgCPUModelInfo))) + if (virQEMUCapsHostCPUDataCopy(ret, qemuCaps) < 0) goto error; if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0) -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:51PM +0200, Jiri Denemark wrote:
This introduces virQEMUCapsHostCPUDataCopy which will later be refactored a bit and called twice from virQEMUCapsNewCopy.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
ACK Pavel
To keep freeing of host CPU data in one place. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 218fd50b2..6f0d6287e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2142,6 +2142,21 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsPtr dst, } +static void +virQEMUCapsHostCPUDataClear(virQEMUCapsPtr qemuCaps) +{ + qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo); + qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo); + qemuCaps->kvmCPUModelInfo = NULL; + qemuCaps->tcgCPUModelInfo = NULL; + + virCPUDefFree(qemuCaps->kvmCPUModel); + virCPUDefFree(qemuCaps->tcgCPUModel); + qemuCaps->kvmCPUModel = NULL; + qemuCaps->tcgCPUModel = NULL; +} + + virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) { virQEMUCapsPtr ret = virQEMUCapsNew(); @@ -2228,10 +2243,7 @@ void virQEMUCapsDispose(void *obj) VIR_FREE(qemuCaps->gicCapabilities); - qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo); - qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo); - virCPUDefFree(qemuCaps->kvmCPUModel); - virCPUDefFree(qemuCaps->tcgCPUModel); + virQEMUCapsHostCPUDataClear(qemuCaps); } void @@ -4109,15 +4121,7 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps) VIR_FREE(qemuCaps->gicCapabilities); qemuCaps->ngicCapabilities = 0; - qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo); - qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo); - qemuCaps->kvmCPUModelInfo = NULL; - qemuCaps->tcgCPUModelInfo = NULL; - - virCPUDefFree(qemuCaps->kvmCPUModel); - virCPUDefFree(qemuCaps->tcgCPUModel); - qemuCaps->kvmCPUModel = NULL; - qemuCaps->tcgCPUModel = NULL; + virQEMUCapsHostCPUDataClear(qemuCaps); } -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:52PM +0200, Jiri Denemark wrote:
To keep freeing of host CPU data in one place.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)
ACK Pavel
We need to store several CPU related data structure for both KVM and TCG. So instead of keeping two different copies of everything let's make a virQEMUCapsHostCPUData struct and use it twice. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 166 +++++++++++++++++++++---------------------- 1 file changed, 81 insertions(+), 85 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6f0d6287e..f914154be 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -373,6 +373,19 @@ struct virQEMUCapsMachineType { unsigned int maxCpus; bool hotplugCpus; }; + +typedef struct _virQEMUCapsHostCPUData virQEMUCapsHostCPUData; +typedef virQEMUCapsHostCPUData *virQEMUCapsHostCPUDataPtr; +struct _virQEMUCapsHostCPUData { + /* Only the "info" part is stored in the capabilities cache, the rest is + * re-computed from other fields and external data sources everytime we + * probe QEMU or load the cache. + */ + qemuMonitorCPUModelInfoPtr info; + /* Host CPU definition reported in domain capabilities. */ + virCPUDefPtr reported; +}; + /* * Update the XML parser/formatter when adding more * information to this struct so that it gets cached @@ -407,15 +420,8 @@ struct _virQEMUCaps { size_t ngicCapabilities; virGICCapability *gicCapabilities; - qemuMonitorCPUModelInfoPtr kvmCPUModelInfo; - qemuMonitorCPUModelInfoPtr tcgCPUModelInfo; - - /* Anything below is not stored in the cache since the values are - * re-computed from the other fields or external data sources every - * time we probe QEMU or load the results from the cache. - */ - virCPUDefPtr kvmCPUModel; - virCPUDefPtr tcgCPUModel; + virQEMUCapsHostCPUData kvmCPU; + virQEMUCapsHostCPUData tcgCPU; }; struct virQEMUCapsSearchData { @@ -2119,23 +2125,15 @@ virQEMUCapsNew(void) static int -virQEMUCapsHostCPUDataCopy(virQEMUCapsPtr dst, - virQEMUCapsPtr src) +virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUDataPtr dst, + virQEMUCapsHostCPUDataPtr src) { - if (src->kvmCPUModel && - !(dst->kvmCPUModel = virCPUDefCopy(src->kvmCPUModel))) + if (src->info && + !(dst->info = qemuMonitorCPUModelInfoCopy(src->info))) return -1; - if (src->tcgCPUModel && - !(dst->tcgCPUModel = virCPUDefCopy(src->tcgCPUModel))) - return -1; - - if (src->kvmCPUModelInfo && - !(dst->kvmCPUModelInfo = qemuMonitorCPUModelInfoCopy(src->kvmCPUModelInfo))) - return -1; - - if (src->tcgCPUModelInfo && - !(dst->tcgCPUModelInfo = qemuMonitorCPUModelInfoCopy(src->tcgCPUModelInfo))) + if (src->reported && + !(dst->reported = virCPUDefCopy(src->reported))) return -1; return 0; @@ -2143,17 +2141,12 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsPtr dst, static void -virQEMUCapsHostCPUDataClear(virQEMUCapsPtr qemuCaps) +virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData) { - qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo); - qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo); - qemuCaps->kvmCPUModelInfo = NULL; - qemuCaps->tcgCPUModelInfo = NULL; + qemuMonitorCPUModelInfoFree(cpuData->info); + virCPUDefFree(cpuData->reported); - virCPUDefFree(qemuCaps->kvmCPUModel); - virCPUDefFree(qemuCaps->tcgCPUModel); - qemuCaps->kvmCPUModel = NULL; - qemuCaps->tcgCPUModel = NULL; + memset(cpuData, '\0', sizeof(*cpuData)); } @@ -2194,7 +2187,8 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) goto error; } - if (virQEMUCapsHostCPUDataCopy(ret, qemuCaps) < 0) + if (virQEMUCapsHostCPUDataCopy(&ret->kvmCPU, &qemuCaps->kvmCPU) < 0 || + virQEMUCapsHostCPUDataCopy(&ret->tcgCPU, &qemuCaps->tcgCPU) < 0) goto error; if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0) @@ -2243,7 +2237,8 @@ void virQEMUCapsDispose(void *obj) VIR_FREE(qemuCaps->gicCapabilities); - virQEMUCapsHostCPUDataClear(qemuCaps); + virQEMUCapsHostCPUDataClear(&qemuCaps->kvmCPU); + virQEMUCapsHostCPUDataClear(&qemuCaps->tcgCPU); } void @@ -2467,14 +2462,24 @@ virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, } +static virQEMUCapsHostCPUDataPtr +virQEMUCapsGetHostCPUData(virQEMUCapsPtr qemuCaps, + virDomainVirtType type) +{ + if (type == VIR_DOMAIN_VIRT_KVM) + return &qemuCaps->kvmCPU; + else + return &qemuCaps->tcgCPU; +} + + virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, virDomainVirtType type) { - if (type == VIR_DOMAIN_VIRT_KVM) - return qemuCaps->kvmCPUModel; - else - return qemuCaps->tcgCPUModel; + virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); + + return cpuData->reported; } @@ -2483,10 +2488,9 @@ virQEMUCapsSetHostModel(virQEMUCapsPtr qemuCaps, virDomainVirtType type, virCPUDefPtr cpu) { - if (type == VIR_DOMAIN_VIRT_KVM) - qemuCaps->kvmCPUModel = cpu; - else - qemuCaps->tcgCPUModel = cpu; + virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); + + cpuData->reported = cpu; } @@ -2882,24 +2886,28 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon, bool tcg) { - qemuMonitorCPUModelInfoPtr *modelInfo; + qemuMonitorCPUModelInfoPtr modelInfo = NULL; qemuMonitorCPUModelInfoPtr nonMigratable = NULL; virHashTablePtr hash = NULL; const char *model; qemuMonitorCPUModelExpansionType type; + virDomainVirtType virtType; + virQEMUCapsHostCPUDataPtr cpuData; int ret = -1; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) return 0; if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) { - modelInfo = &qemuCaps->tcgCPUModelInfo; + virtType = VIR_DOMAIN_VIRT_QEMU; model = "max"; } else { - modelInfo = &qemuCaps->kvmCPUModelInfo; + virtType = VIR_DOMAIN_VIRT_KVM; model = "host"; } + cpuData = virQEMUCapsGetHostCPUData(qemuCaps, virtType); + /* Some x86_64 features defined in cpu_map.xml use spelling which differ * from the one preferred by QEMU. Static expansion would give us only the * preferred spelling, thus we need to do a full expansion on the result of @@ -2910,14 +2918,14 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, else type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC; - if (qemuMonitorGetCPUModelExpansion(mon, type, model, true, modelInfo) < 0) - return -1; + if (qemuMonitorGetCPUModelExpansion(mon, type, model, true, &modelInfo) < 0) + goto cleanup; /* Try to check migratability of each feature. */ - if (*modelInfo && + if (modelInfo && qemuMonitorGetCPUModelExpansion(mon, type, model, false, &nonMigratable) < 0) - goto error; + goto cleanup; if (nonMigratable) { qemuMonitorCPUPropertyPtr prop; @@ -2925,12 +2933,12 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, size_t i; if (!(hash = virHashCreate(0, NULL))) - goto error; + goto cleanup; - for (i = 0; i < (*modelInfo)->nprops; i++) { - prop = (*modelInfo)->props + i; + for (i = 0; i < modelInfo->nprops; i++) { + prop = modelInfo->props + i; if (virHashAddEntry(hash, prop->name, prop) < 0) - goto error; + goto cleanup; } for (i = 0; i < nonMigratable->nprops; i++) { @@ -2948,21 +2956,18 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, } } - (*modelInfo)->migratability = true; + modelInfo->migratability = true; } + VIR_STEAL_PTR(cpuData->info, modelInfo); ret = 0; cleanup: virHashFree(hash); qemuMonitorCPUModelInfoFree(nonMigratable); + qemuMonitorCPUModelInfoFree(modelInfo); return ret; - - error: - qemuMonitorCPUModelInfoFree(*modelInfo); - *modelInfo = NULL; - goto cleanup; } struct tpmTypeToCaps { @@ -3315,21 +3320,19 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps, virCPUDefPtr cpu, bool migratable) { - qemuMonitorCPUModelInfoPtr model; + virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); int ret = 1; - if (type == VIR_DOMAIN_VIRT_KVM) - model = qemuCaps->kvmCPUModelInfo; - else - model = qemuCaps->tcgCPUModelInfo; - - if (migratable && model && !model->migratability) + if (migratable && cpuData->info && !cpuData->info->migratability) return 1; - if (ARCH_IS_S390(qemuCaps->arch)) - ret = virQEMUCapsInitCPUModelS390(qemuCaps, model, cpu, migratable); - else if (ARCH_IS_X86(qemuCaps->arch)) - ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, model, cpu, migratable); + if (ARCH_IS_S390(qemuCaps->arch)) { + ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpuData->info, + cpu, migratable); + } else if (ARCH_IS_X86(qemuCaps->arch)) { + ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, cpuData->info, + cpu, migratable); + } if (ret == 0) cpu->fallback = VIR_CPU_FALLBACK_FORBID; @@ -3389,10 +3392,9 @@ virQEMUCapsSetCPUModelInfo(virQEMUCapsPtr qemuCaps, virDomainVirtType type, qemuMonitorCPUModelInfoPtr modelInfo) { - if (type == VIR_DOMAIN_VIRT_KVM) - qemuCaps->kvmCPUModelInfo = modelInfo; - else - qemuCaps->tcgCPUModelInfo = modelInfo; + virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); + + cpuData->info = modelInfo; } @@ -3851,18 +3853,11 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps, virBufferPtr buf, virDomainVirtType type) { - qemuMonitorCPUModelInfoPtr model; - const char *typeStr; + virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); + qemuMonitorCPUModelInfoPtr model = cpuData->info; + const char *typeStr = type == VIR_DOMAIN_VIRT_KVM ? "kvm" : "tcg"; size_t i; - if (type == VIR_DOMAIN_VIRT_KVM) { - typeStr = "kvm"; - model = qemuCaps->kvmCPUModelInfo; - } else { - typeStr = "tcg"; - model = qemuCaps->tcgCPUModelInfo; - } - if (!model) return; @@ -4121,7 +4116,8 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps) VIR_FREE(qemuCaps->gicCapabilities); qemuCaps->ngicCapabilities = 0; - virQEMUCapsHostCPUDataClear(qemuCaps); + virQEMUCapsHostCPUDataClear(&qemuCaps->kvmCPU); + virQEMUCapsHostCPUDataClear(&qemuCaps->tcgCPU); } -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:53PM +0200, Jiri Denemark wrote:
We need to store several CPU related data structure for both KVM and TCG. So instead of keeping two different copies of everything let's make a virQEMUCapsHostCPUData struct and use it twice.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 166 +++++++++++++++++++++---------------------- 1 file changed, 81 insertions(+), 85 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6f0d6287e..f914154be 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c
[...]
@@ -2143,17 +2141,12 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsPtr dst,
static void -virQEMUCapsHostCPUDataClear(virQEMUCapsPtr qemuCaps) +virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData) { - qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo); - qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo); - qemuCaps->kvmCPUModelInfo = NULL; - qemuCaps->tcgCPUModelInfo = NULL; + qemuMonitorCPUModelInfoFree(cpuData->info); + virCPUDefFree(cpuData->reported);
- virCPUDefFree(qemuCaps->kvmCPUModel); - virCPUDefFree(qemuCaps->tcgCPUModel); - qemuCaps->kvmCPUModel = NULL; - qemuCaps->tcgCPUModel = NULL; + memset(cpuData, '\0', sizeof(*cpuData));
We tend to use memset(ptr, 0, size); :)
}
ACK Pavel
Soon we will need to store multiple host CPU definitions in virQEMUCapsHostCPUData and qemuCaps users will want to request the one they need. This patch introduces virQEMUCapsHostCPUType enum which will be used for specifying the requested CPU definition. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 16 ++++++++++++---- src/qemu/qemu_capabilities.h | 10 +++++++++- src/qemu/qemu_command.c | 3 ++- src/qemu/qemu_process.c | 6 ++++-- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f914154be..3bfc79c09 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2475,11 +2475,17 @@ virQEMUCapsGetHostCPUData(virQEMUCapsPtr qemuCaps, virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, - virDomainVirtType type) + virDomainVirtType type, + virQEMUCapsHostCPUType cpuType) { virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); - return cpuData->reported; + switch (cpuType) { + case VIR_QEMU_CAPS_HOST_CPU_REPORTED: + return cpuData->reported; + } + + return NULL; } @@ -2508,7 +2514,8 @@ virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch); case VIR_CPU_MODE_HOST_MODEL: - return !!virQEMUCapsGetHostModel(qemuCaps, type); + return !!virQEMUCapsGetHostModel(qemuCaps, type, + VIR_QEMU_CAPS_HOST_CPU_REPORTED); case VIR_CPU_MODE_CUSTOM: if (type == VIR_DOMAIN_VIRT_KVM) @@ -5521,7 +5528,8 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps, if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype, VIR_CPU_MODE_HOST_MODEL)) { - virCPUDefPtr cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype); + virCPUDefPtr cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype, + VIR_QEMU_CAPS_HOST_CPU_REPORTED); domCaps->cpu.hostModel = virCPUDefCopy(cpu); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index cca9a12b5..bd147c009 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -449,8 +449,16 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, virDomainVirtType type, char ***names, size_t *count); + +typedef enum { + /* Host CPU definition reported in domain capabilities. */ + VIR_QEMU_CAPS_HOST_CPU_REPORTED, +} virQEMUCapsHostCPUType; + virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, - virDomainVirtType type); + virDomainVirtType type, + virQEMUCapsHostCPUType cpuType); + bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, virCapsPtr caps, virDomainVirtType type, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 57654246c..7e5cb3209 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6899,7 +6899,8 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, if (def->cpu->mode == VIR_CPU_MODE_CUSTOM) cpuDef = def->cpu; else if (def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) - cpuDef = virQEMUCapsGetHostModel(qemuCaps, def->virtType); + cpuDef = virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_REPORTED); if (cpuDef) { int svm = virCPUCheckFeature(def->os.arch, cpuDef, "svm"); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5e119a237..c05cd9e7a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5304,12 +5304,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (def->cpu->check == VIR_CPU_CHECK_PARTIAL && virCPUCompare(caps->host.arch, - virQEMUCapsGetHostModel(qemuCaps, def->virtType), + virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_REPORTED), def->cpu, true) < 0) return -1; if (virCPUUpdate(def->os.arch, def->cpu, - virQEMUCapsGetHostModel(qemuCaps, def->virtType)) < 0) + virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_REPORTED)) < 0) goto cleanup; if (virQEMUCapsGetCPUDefinitions(qemuCaps, def->virtType, -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:54PM +0200, Jiri Denemark wrote:
Soon we will need to store multiple host CPU definitions in virQEMUCapsHostCPUData and qemuCaps users will want to request the one they need. This patch introduces virQEMUCapsHostCPUType enum which will be used for specifying the requested CPU definition.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 16 ++++++++++++---- src/qemu/qemu_capabilities.h | 10 +++++++++- src/qemu/qemu_command.c | 3 ++- src/qemu/qemu_process.c | 6 ++++-- 4 files changed, 27 insertions(+), 8 deletions(-)
ACK Pavel
We already know from QEMU which CPU features will block migration. Let's use this information to make a migratable copy of the host CPU model and use it for updating guest CPU specification. This will allow us to drop feature filtering from virCPUUpdate where it was just a hack. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 57 +++++++++++++++++++++++++++++++++++++------- src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_process.c | 2 +- tests/cputest.c | 7 +++++- 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3bfc79c09..1d95e67b3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -384,6 +384,8 @@ struct _virQEMUCapsHostCPUData { qemuMonitorCPUModelInfoPtr info; /* Host CPU definition reported in domain capabilities. */ virCPUDefPtr reported; + /* Migratable host CPU definition used for updating guest CPU. */ + virCPUDefPtr migratable; }; /* @@ -2136,6 +2138,10 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUDataPtr dst, !(dst->reported = virCPUDefCopy(src->reported))) return -1; + if (src->migratable && + !(dst->migratable = virCPUDefCopy(src->migratable))) + return -1; + return 0; } @@ -2145,6 +2151,7 @@ virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData) { qemuMonitorCPUModelInfoFree(cpuData->info); virCPUDefFree(cpuData->reported); + virCPUDefFree(cpuData->migratable); memset(cpuData, '\0', sizeof(*cpuData)); } @@ -2483,6 +2490,9 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, switch (cpuType) { case VIR_QEMU_CAPS_HOST_CPU_REPORTED: return cpuData->reported; + + case VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE: + return cpuData->migratable; } return NULL; @@ -2492,11 +2502,13 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, static void virQEMUCapsSetHostModel(virQEMUCapsPtr qemuCaps, virDomainVirtType type, - virCPUDefPtr cpu) + virCPUDefPtr reported, + virCPUDefPtr migratable) { virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); - cpuData->reported = cpu; + cpuData->reported = reported; + cpuData->migratable = migratable; } @@ -3348,26 +3360,39 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps, } +static virCPUDefPtr +virQEMUCapsNewHostCPUModel(void) +{ + virCPUDefPtr cpu; + + if (VIR_ALLOC(cpu) < 0) + return NULL; + + cpu->type = VIR_CPU_TYPE_GUEST; + cpu->mode = VIR_CPU_MODE_CUSTOM; + cpu->match = VIR_CPU_MATCH_EXACT; + cpu->fallback = VIR_CPU_FALLBACK_ALLOW; + + return cpu; +} + + void virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virCapsPtr caps, virDomainVirtType type) { virCPUDefPtr cpu = NULL; + virCPUDefPtr migCPU = NULL; virCPUDefPtr hostCPU = NULL; int rc; if (!caps || !virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch)) return; - if (VIR_ALLOC(cpu) < 0) + if (!(cpu = virQEMUCapsNewHostCPUModel())) goto error; - cpu->type = VIR_CPU_TYPE_GUEST; - cpu->mode = VIR_CPU_MODE_CUSTOM; - cpu->match = VIR_CPU_MATCH_EXACT; - cpu->fallback = VIR_CPU_FALLBACK_ALLOW; - if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu, false)) < 0) { goto error; } else if (rc == 1) { @@ -3381,7 +3406,20 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, goto error; } - virQEMUCapsSetHostModel(qemuCaps, type, cpu); + if (!(migCPU = virQEMUCapsNewHostCPUModel())) + goto error; + + if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, migCPU, true)) < 0) { + goto error; + } else if (rc == 1) { + VIR_DEBUG("CPU migratability not provided by QEMU"); + + virCPUDefFree(migCPU); + if (!(migCPU = virCPUCopyMigratable(qemuCaps->arch, cpu))) + goto error; + } + + virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU); cleanup: virCPUDefFree(hostCPU); @@ -3389,6 +3427,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, error: virCPUDefFree(cpu); + virCPUDefFree(migCPU); virResetLastError(); goto cleanup; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index bd147c009..f04f74060 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -453,6 +453,8 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, typedef enum { /* Host CPU definition reported in domain capabilities. */ VIR_QEMU_CAPS_HOST_CPU_REPORTED, + /* Migratable host CPU definition used for updating guest CPU. */ + VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE, } virQEMUCapsHostCPUType; virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c05cd9e7a..6b77a3969 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5311,7 +5311,7 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (virCPUUpdate(def->os.arch, def->cpu, virQEMUCapsGetHostModel(qemuCaps, def->virtType, - VIR_QEMU_CAPS_HOST_CPU_REPORTED)) < 0) + VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE)) < 0) goto cleanup; if (virQEMUCapsGetCPUDefinitions(qemuCaps, def->virtType, diff --git a/tests/cputest.c b/tests/cputest.c index 528030754..d5e023c40 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -393,6 +393,7 @@ cpuTestUpdate(const void *arg) const struct data *data = arg; int ret = -1; virCPUDefPtr host = NULL; + virCPUDefPtr migHost = NULL; virCPUDefPtr cpu = NULL; char *result = NULL; @@ -400,7 +401,10 @@ cpuTestUpdate(const void *arg) !(cpu = cpuTestLoadXML(data->arch, data->name))) goto cleanup; - if (virCPUUpdate(host->arch, cpu, host) < 0) + if (!(migHost = virCPUCopyMigratable(data->arch, host))) + goto cleanup; + + if (virCPUUpdate(host->arch, cpu, migHost) < 0) goto cleanup; if (virAsprintf(&result, "%s+%s", data->host, data->name) < 0) @@ -411,6 +415,7 @@ cpuTestUpdate(const void *arg) cleanup: virCPUDefFree(host); virCPUDefFree(cpu); + virCPUDefFree(migHost); VIR_FREE(result); return ret; } -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:55PM +0200, Jiri Denemark wrote:
We already know from QEMU which CPU features will block migration. Let's use this information to make a migratable copy of the host CPU model and use it for updating guest CPU specification. This will allow us to drop feature filtering from virCPUUpdate where it was just a hack.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 57 +++++++++++++++++++++++++++++++++++++------- src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_process.c | 2 +- tests/cputest.c | 7 +++++- 4 files changed, 57 insertions(+), 11 deletions(-)
ACK Pavel
Because of the changes done in the previous commit, @host is already a migratable CPU and there's no need to do any additional filtering. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index a771b251e..53359ff9b 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2549,8 +2549,7 @@ x86Baseline(virCPUDefPtr *cpus, static int x86UpdateHostModel(virCPUDefPtr guest, - const virCPUDef *host, - virCPUx86MapPtr map) + const virCPUDef *host) { virCPUDefPtr updated = NULL; size_t i; @@ -2559,11 +2558,9 @@ x86UpdateHostModel(virCPUDefPtr guest, if (!(updated = virCPUDefCopyWithoutModel(host))) goto cleanup; - /* Remove non-migratable features by default */ updated->type = VIR_CPU_TYPE_GUEST; updated->mode = VIR_CPU_MODE_CUSTOM; - if (virCPUDefCopyModelFilter(updated, host, true, - x86FeatureIsMigratable, map) < 0) + if (virCPUDefCopyModel(updated, host, true) < 0) goto cleanup; if (guest->vendor_id) { @@ -2627,7 +2624,7 @@ virCPUx86Update(virCPUDefPtr guest, if (guest->mode == VIR_CPU_MODE_HOST_MODEL || guest->match == VIR_CPU_MATCH_MINIMUM) - ret = x86UpdateHostModel(guest, host, map); + ret = x86UpdateHostModel(guest, host); else ret = 0; -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:56PM +0200, Jiri Denemark wrote:
Because of the changes done in the previous commit, @host is already a migratable CPU and there's no need to do any additional filtering.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
ACK Pavel
Sometimes we want to call virCPUGetHost only when it is implemented for a given architecture to avoid logging expected and possibly misleading errors. The new virCPUGetHostIsSupported API may be used to guard such calls to virCPUGetHost. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu.c | 20 ++++++++++++++++++++ src/cpu/cpu.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 24 insertions(+) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 8a407ac18..702b14dbb 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -358,6 +358,26 @@ virCPUDataFree(virCPUDataPtr data) /** + * virCPUGetHostIsSupported: + * + * @arch: CPU architecture + * + * Check whether virCPUGetHost is supported for @arch. + * + * Returns true if virCPUGetHost is supported, false otherwise. + */ +bool +virCPUGetHostIsSupported(virArch arch) +{ + struct cpuArchDriver *driver; + + VIR_DEBUG("arch=%s", virArchToString(arch)); + + return (driver = cpuGetSubDriver(arch)) && driver->getHost; +} + + +/** * virCPUGetHost: * * @arch: CPU architecture diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 352445c40..c6ca111e9 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -183,6 +183,9 @@ virCPUDataNew(virArch arch); void virCPUDataFree(virCPUDataPtr data); +bool +virCPUGetHostIsSupported(virArch arch); + virCPUDefPtr virCPUGetHost(virArch arch, virCPUType type, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8509f639e..d2b7b5cd1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1033,6 +1033,7 @@ virCPUDataNew; virCPUDataParse; virCPUExpandFeatures; virCPUGetHost; +virCPUGetHostIsSupported; virCPUGetModels; virCPUProbeHost; virCPUTranslate; -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:57PM +0200, Jiri Denemark wrote:
Sometimes we want to call virCPUGetHost only when it is implemented for a given architecture to avoid logging expected and possibly misleading errors. The new virCPUGetHostIsSupported API may be used to guard such calls to virCPUGetHost.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu.c | 20 ++++++++++++++++++++ src/cpu/cpu.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 24 insertions(+)
ACK Pavel
With QEMU older than 2.9.0 libvirt uses CPUID instruction to determine what CPU features are supported on the host. This was later used when checking compatibility of guest CPUs. Since QEMU 2.9.0 we ask QEMU for the host CPU data. But the two methods we use usually provide disjoint sets of CPU features because QEMU/KVM does not support all features provided by the host CPU and on the other hand it can enable some feature even if the host CPU does not support them. So if there is a domain which requires a CPU features disabled by QEMU/KVM, libvirt will refuse to start it with QEMU > 2.9.0 as its guest CPU is incompatible with the host CPU data we got from QEMU. But such domain would happily start on older QEMU (of course, the features would be missing the guest CPU). To fix this regression, we need to combine both CPU feature sets when checking guest CPU compatibility. https://bugzilla.redhat.com/show_bug.cgi?id=1439933 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 35 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_process.c | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1d95e67b3..531677865 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -386,6 +386,10 @@ struct _virQEMUCapsHostCPUData { virCPUDefPtr reported; /* Migratable host CPU definition used for updating guest CPU. */ virCPUDefPtr migratable; + /* CPU definition with features detected by libvirt using virCPUGetHost + * combined with features reported by QEMU. This is used for backward + * compatible comparison between a guest CPU and a host CPU. */ + virCPUDefPtr full; }; /* @@ -2142,6 +2146,10 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUDataPtr dst, !(dst->migratable = virCPUDefCopy(src->migratable))) return -1; + if (src->full && + !(dst->full = virCPUDefCopy(src->full))) + return -1; + return 0; } @@ -2152,6 +2160,7 @@ virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUDataPtr cpuData) qemuMonitorCPUModelInfoFree(cpuData->info); virCPUDefFree(cpuData->reported); virCPUDefFree(cpuData->migratable); + virCPUDefFree(cpuData->full); memset(cpuData, '\0', sizeof(*cpuData)); } @@ -2493,6 +2502,11 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, case VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE: return cpuData->migratable; + + case VIR_QEMU_CAPS_HOST_CPU_FULL: + /* 'full' is non-NULL only if we have data from both QEMU and + * virCPUGetHost */ + return cpuData->full ? cpuData->full : cpuData->reported; } return NULL; @@ -2503,12 +2517,14 @@ static void virQEMUCapsSetHostModel(virQEMUCapsPtr qemuCaps, virDomainVirtType type, virCPUDefPtr reported, - virCPUDefPtr migratable) + virCPUDefPtr migratable, + virCPUDefPtr full) { virQEMUCapsHostCPUDataPtr cpuData = virQEMUCapsGetHostCPUData(qemuCaps, type); cpuData->reported = reported; cpuData->migratable = migratable; + cpuData->full = full; } @@ -3385,6 +3401,8 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virCPUDefPtr cpu = NULL; virCPUDefPtr migCPU = NULL; virCPUDefPtr hostCPU = NULL; + virCPUDefPtr fullCPU = NULL; + size_t i; int rc; if (!caps || !virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch)) @@ -3404,6 +3422,18 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virQEMUCapsCPUFilterFeatures, qemuCaps) < 0) goto error; + } else if (type == VIR_DOMAIN_VIRT_KVM && + virCPUGetHostIsSupported(qemuCaps->arch)) { + if (!(fullCPU = virCPUGetHost(qemuCaps->arch, VIR_CPU_TYPE_GUEST, + NULL, NULL, 0))) + goto error; + + for (i = 0; i < cpu->nfeatures; i++) { + if (cpu->features[i].policy == VIR_CPU_FEATURE_REQUIRE && + virCPUDefUpdateFeature(fullCPU, cpu->features[i].name, + VIR_CPU_FEATURE_REQUIRE) < 0) + goto error; + } } if (!(migCPU = virQEMUCapsNewHostCPUModel())) @@ -3419,7 +3449,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, goto error; } - virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU); + virQEMUCapsSetHostModel(qemuCaps, type, cpu, migCPU, fullCPU); cleanup: virCPUDefFree(hostCPU); @@ -3428,6 +3458,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, error: virCPUDefFree(cpu); virCPUDefFree(migCPU); + virCPUDefFree(fullCPU); virResetLastError(); goto cleanup; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f04f74060..a7eec52a9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -455,6 +455,10 @@ typedef enum { VIR_QEMU_CAPS_HOST_CPU_REPORTED, /* Migratable host CPU definition used for updating guest CPU. */ VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE, + /* CPU definition with features detected by libvirt using virCPUGetHost + * combined with features reported by QEMU. This is used for backward + * compatible comparison between a guest CPU and a host CPU. */ + VIR_QEMU_CAPS_HOST_CPU_FULL, } virQEMUCapsHostCPUType; virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6b77a3969..53170d732 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5305,7 +5305,7 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (def->cpu->check == VIR_CPU_CHECK_PARTIAL && virCPUCompare(caps->host.arch, virQEMUCapsGetHostModel(qemuCaps, def->virtType, - VIR_QEMU_CAPS_HOST_CPU_REPORTED), + VIR_QEMU_CAPS_HOST_CPU_FULL), def->cpu, true) < 0) return -1; -- 2.12.2
On Thu, Apr 13, 2017 at 03:32:58PM +0200, Jiri Denemark wrote:
With QEMU older than 2.9.0 libvirt uses CPUID instruction to determine what CPU features are supported on the host. This was later used when checking compatibility of guest CPUs. Since QEMU 2.9.0 we ask QEMU for the host CPU data. But the two methods we use usually provide disjoint sets of CPU features because QEMU/KVM does not support all features provided by the host CPU and on the other hand it can enable some feature even if the host CPU does not support them.
So if there is a domain which requires a CPU features disabled by QEMU/KVM, libvirt will refuse to start it with QEMU > 2.9.0 as its guest CPU is incompatible with the host CPU data we got from QEMU. But such domain would happily start on older QEMU (of course, the features would be missing the guest CPU). To fix this regression, we need to combine both CPU feature sets when checking guest CPU compatibility.
https://bugzilla.redhat.com/show_bug.cgi?id=1439933
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 35 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_process.c | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-)
ACK Pavel
On Wed, Apr 19, 2017 at 16:02:48 +0200, Pavel Hrdina wrote:
On Thu, Apr 13, 2017 at 03:32:58PM +0200, Jiri Denemark wrote:
With QEMU older than 2.9.0 libvirt uses CPUID instruction to determine what CPU features are supported on the host. This was later used when checking compatibility of guest CPUs. Since QEMU 2.9.0 we ask QEMU for the host CPU data. But the two methods we use usually provide disjoint sets of CPU features because QEMU/KVM does not support all features provided by the host CPU and on the other hand it can enable some feature even if the host CPU does not support them.
So if there is a domain which requires a CPU features disabled by QEMU/KVM, libvirt will refuse to start it with QEMU > 2.9.0 as its guest CPU is incompatible with the host CPU data we got from QEMU. But such domain would happily start on older QEMU (of course, the features would be missing the guest CPU). To fix this regression, we need to combine both CPU feature sets when checking guest CPU compatibility.
https://bugzilla.redhat.com/show_bug.cgi?id=1439933
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 35 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_capabilities.h | 4 ++++ src/qemu/qemu_process.c | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-)
ACK
Thanks, I pushed this series. Jirka
participants (2)
-
Jiri Denemark -
Pavel Hrdina