7:20 a.m.
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
7:20 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Separate fetching CPU definitions from filling qemuCaps
virQEMUCapsProbeQMPCPUDefinitions is now a small wrapper which fills in
qemuCaps with CPU models fetched by virQEMUCapsFetchCPUDefinitions.
Signed-off-by: Jiri Denemark <jdenemar(a)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
6:59 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Separate fetching CPU definitions from filling qemuCaps
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(a)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. */
7:33 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Separate fetching CPU definitions from filling qemuCaps
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(a)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
7:20 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Make sure host-model uses CPU model supported by QEMU
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(a)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
8:30 a.m.
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
2573
days inactive
2577
days old
5 comments
3 participants
participants (3)
-
Jiri Denemark -
John Ferlan -
Ján Tomko