[libvirt] [PATCH 0/2] qemu: Fix upgrade with running domains with host-model CPUs
When upgrading old libvirt on a host with EPYC host CPU (or any other CPU which was added to cpu_map.xml in newer libvirt releases), we would replace host-model CPU definition for running domains to EPYC even though a different CPU model was used to start the domain. And sync old libvirt doesn't know about EPYC, migrating the domain to another host running the same old libvirt would be impossible. https://bugzilla.redhat.com/show_bug.cgi?id=1521202 Jiri Denemark (2): qemu: Separate fetching CPU definitions from filling qemuCaps qemu: Make sure host-model uses CPU model supported by QEMU src/qemu/qemu_capabilities.c | 59 +++++++++++++++++++++++++++----------------- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_process.c | 30 ++++++++++++++++++++++ 3 files changed, 68 insertions(+), 22 deletions(-) -- 2.15.1
virQEMUCapsProbeQMPCPUDefinitions is now a small wrapper which fills in qemuCaps with CPU models fetched by virQEMUCapsFetchCPUDefinitions. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 59 +++++++++++++++++++++++++++----------------- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 29714855b0..8c65de956e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2956,30 +2956,19 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, } -int -virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, - qemuMonitorPtr mon, - bool tcg) +virDomainCapsCPUModelsPtr +virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon) { - virDomainCapsCPUModelsPtr models; - qemuMonitorCPUDefInfoPtr *cpus; - int ncpus; - int ret = -1; + virDomainCapsCPUModelsPtr models = NULL; + qemuMonitorCPUDefInfoPtr *cpus = NULL; + int ncpus = 0; size_t i; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_DEFINITIONS)) - return 0; - if ((ncpus = qemuMonitorGetCPUDefinitions(mon, &cpus)) < 0) - return -1; + goto error; if (!(models = virDomainCapsCPUModelsNew(ncpus))) - goto cleanup; - - if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) - qemuCaps->tcgCPUModels = models; - else - qemuCaps->kvmCPUModels = models; + goto error; for (i = 0; i < ncpus; i++) { virDomainCapsCPUUsable usable = VIR_DOMCAPS_CPU_USABLE_UNKNOWN; @@ -2991,18 +2980,44 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, if (virDomainCapsCPUModelsAddSteal(models, &cpus[i]->name, usable, &cpus[i]->blockers) < 0) - goto cleanup; + goto error; } - ret = 0; - cleanup: for (i = 0; i < ncpus; i++) qemuMonitorCPUDefInfoFree(cpus[i]); VIR_FREE(cpus); - return ret; + return models; + + error: + virObjectUnref(models); + models = NULL; + goto cleanup; } + +int +virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon, + bool tcg) +{ + virDomainCapsCPUModelsPtr models = NULL; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_DEFINITIONS)) + return 0; + + if (!(models = virQEMUCapsFetchCPUDefinitions(mon))) + return -1; + + if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) + qemuCaps->tcgCPUModels = models; + else + qemuCaps->kvmCPUModels = models; + + return 0; +} + + static int virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9f239a0ecf..e73dbaa557 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -485,6 +485,7 @@ int virQEMUCapsAddCPUDefinitions(virQEMUCapsPtr qemuCaps, virDomainCapsCPUUsable usable); virDomainCapsCPUModelsPtr virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, virDomainVirtType type); +virDomainCapsCPUModelsPtr virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon); typedef enum { /* Host CPU definition reported in domain capabilities. */ -- 2.15.1
On 12/07/2017 08:20 AM, Jiri Denemark wrote:
virQEMUCapsProbeQMPCPUDefinitions is now a small wrapper which fills in qemuCaps with CPU models fetched by virQEMUCapsFetchCPUDefinitions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 59 +++++++++++++++++++++++++++----------------- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 29714855b0..8c65de956e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2956,30 +2956,19 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, }
-int -virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, - qemuMonitorPtr mon, - bool tcg) +virDomainCapsCPUModelsPtr +virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon) { - virDomainCapsCPUModelsPtr models; - qemuMonitorCPUDefInfoPtr *cpus; - int ncpus; - int ret = -1; + virDomainCapsCPUModelsPtr models = NULL; + qemuMonitorCPUDefInfoPtr *cpus = NULL; + int ncpus = 0; size_t i;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_DEFINITIONS)) - return 0; - if ((ncpus = qemuMonitorGetCPUDefinitions(mon, &cpus)) < 0) - return -1; + goto error;
This adjustment causes a Coverity found problem for the "for (i = 0; i < ncpus; i++)" in cleanup: on failure ncpus == -1. John
if (!(models = virDomainCapsCPUModelsNew(ncpus))) - goto cleanup; - - if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) - qemuCaps->tcgCPUModels = models; - else - qemuCaps->kvmCPUModels = models; + goto error;
for (i = 0; i < ncpus; i++) { virDomainCapsCPUUsable usable = VIR_DOMCAPS_CPU_USABLE_UNKNOWN; @@ -2991,18 +2980,44 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
if (virDomainCapsCPUModelsAddSteal(models, &cpus[i]->name, usable, &cpus[i]->blockers) < 0) - goto cleanup; + goto error; }
- ret = 0; - cleanup: for (i = 0; i < ncpus; i++) qemuMonitorCPUDefInfoFree(cpus[i]); VIR_FREE(cpus); - return ret; + return models; + + error: + virObjectUnref(models); + models = NULL; + goto cleanup; }
+ +int +virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, + qemuMonitorPtr mon, + bool tcg) +{ + virDomainCapsCPUModelsPtr models = NULL; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_DEFINITIONS)) + return 0; + + if (!(models = virQEMUCapsFetchCPUDefinitions(mon))) + return -1; + + if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) + qemuCaps->tcgCPUModels = models; + else + qemuCaps->kvmCPUModels = models; + + return 0; +} + + static int virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9f239a0ecf..e73dbaa557 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -485,6 +485,7 @@ int virQEMUCapsAddCPUDefinitions(virQEMUCapsPtr qemuCaps, virDomainCapsCPUUsable usable); virDomainCapsCPUModelsPtr virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, virDomainVirtType type); +virDomainCapsCPUModelsPtr virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon);
typedef enum { /* Host CPU definition reported in domain capabilities. */
On Mon, Dec 11, 2017 at 07:59:44 -0500, John Ferlan wrote:
On 12/07/2017 08:20 AM, Jiri Denemark wrote:
virQEMUCapsProbeQMPCPUDefinitions is now a small wrapper which fills in qemuCaps with CPU models fetched by virQEMUCapsFetchCPUDefinitions.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_capabilities.c | 59 +++++++++++++++++++++++++++----------------- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 29714855b0..8c65de956e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2956,30 +2956,19 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, }
-int -virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, - qemuMonitorPtr mon, - bool tcg) +virDomainCapsCPUModelsPtr +virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon) { - virDomainCapsCPUModelsPtr models; - qemuMonitorCPUDefInfoPtr *cpus; - int ncpus; - int ret = -1; + virDomainCapsCPUModelsPtr models = NULL; + qemuMonitorCPUDefInfoPtr *cpus = NULL; + int ncpus = 0; size_t i;
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_DEFINITIONS)) - return 0; - if ((ncpus = qemuMonitorGetCPUDefinitions(mon, &cpus)) < 0) - return -1; + goto error;
This adjustment causes a Coverity found problem for the "for (i = 0; i < ncpus; i++)" in cleanup: on failure ncpus == -1.
Oh right, a patch is coming... Jirka
When reconnecting to a running domain started by old libvirt, which did not change host-model into a custom CPU definition, we replace the CPU definition with a specific CPU model from host capabilities. However, that CPU model may not be supported by the running qemu process. We need to translate the CPU model to one of the models which libvirt could have used when starting the domain. https://bugzilla.redhat.com/show_bug.cgi?id=1521202 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 93a24cde15..a0f430f89f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3853,6 +3853,30 @@ qemuProcessUpdateAndVerifyCPU(virQEMUDriverPtr driver, } +static virDomainCapsCPUModelsPtr +qemuProcessFetchCPUDefinitions(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainCapsCPUModelsPtr models = NULL; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto error; + + models = virQEMUCapsFetchCPUDefinitions(priv->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto error; + + return models; + + error: + virObjectUnref(models); + return NULL; +} + + static int qemuProcessUpdateCPU(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3860,6 +3884,7 @@ qemuProcessUpdateCPU(virQEMUDriverPtr driver, { virCPUDataPtr cpu = NULL; virCPUDataPtr disabled = NULL; + virDomainCapsCPUModelsPtr models = NULL; int ret = -1; if (qemuProcessFetchGuestCPU(driver, vm, asyncJob, &cpu, &disabled) < 0) @@ -3868,11 +3893,16 @@ qemuProcessUpdateCPU(virQEMUDriverPtr driver, if (qemuProcessUpdateLiveGuestCPU(vm, cpu, disabled) < 0) goto cleanup; + if (!(models = qemuProcessFetchCPUDefinitions(driver, vm, asyncJob)) || + virCPUTranslate(vm->def->os.arch, vm->def->cpu, models) < 0) + goto cleanup; + ret = 0; cleanup: virCPUDataFree(cpu); virCPUDataFree(disabled); + virObjectUnref(models); return ret; } -- 2.15.1
On Thu, Dec 07, 2017 at 02:20:05PM +0100, Jiri Denemark wrote:
When upgrading old libvirt on a host with EPYC host CPU (or any other CPU which was added to cpu_map.xml in newer libvirt releases), we would replace host-model CPU definition for running domains to EPYC even though a different CPU model was used to start the domain. And sync old libvirt doesn't know about EPYC, migrating the domain to another host running the same old libvirt would be impossible.
https://bugzilla.redhat.com/show_bug.cgi?id=1521202
Jiri Denemark (2): qemu: Separate fetching CPU definitions from filling qemuCaps qemu: Make sure host-model uses CPU model supported by QEMU
src/qemu/qemu_capabilities.c | 59 +++++++++++++++++++++++++++----------------- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_process.c | 30 ++++++++++++++++++++++ 3 files changed, 68 insertions(+), 22 deletions(-)
ACK series Jan
participants (3)
-
Jiri Denemark -
John Ferlan -
Ján Tomko