12:53 p.m.
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 migrtability may differ
depending on a hypervisor or its version. Thus we should preferably use
the data we got from QEMU.
Jiri Denemark (7):
cpu: Introduce virCPUCopyMigratable
qemu: Move common code in virQEMUCapsInitCPUModel one layer up
qemu: Add migratable parameter to virQEMUCapsInitCPUModel
qemu: Move qemuCaps->{kvm,tcg}CPUModel into a struct
qemu: Store migratable host CPU model in qemuCaps
qemu: Pass migratable host model to virCPUUpdate
cpu: Drop feature filtering from virCPUUpdate
src/cpu/cpu.c | 29 +++++++
src/cpu/cpu.h | 8 ++
src/cpu/cpu_x86.c | 34 ++++++--
src/libvirt_private.syms | 1 +
src/qemu/qemu_capabilities.c | 200 ++++++++++++++++++++++++++++++-------------
src/qemu/qemu_capabilities.h | 3 +-
src/qemu/qemu_capspriv.h | 3 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_process.c | 4 +-
tests/cputest.c | 9 +-
10 files changed, 221 insertions(+), 72 deletions(-)
--
2.12.0
12:53 p.m.
New subject: [libvirt] [PATCH 1/7] cpu: Introduce virCPUCopyMigratable
This new internal API makes a copy of virCPUDef while removing all
features which would block migration. It uses cpu_map.xml as a database
of such features, which should only be used as a fallback when we cannot
get the data from a hypervisor. The main goal of this API is to decouple
this filtering from virCPUUpdate so that the hypervisor driver can
filter the features according to the hypervisor.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu.c | 29 +++++++++++++++++++++++++++++
src/cpu/cpu.h | 8 ++++++++
src/cpu/cpu_x86.c | 25 +++++++++++++++++++++++++
src/libvirt_private.syms | 1 +
4 files changed, 63 insertions(+)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 93647a2ed..1cc68f142 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -1130,3 +1130,32 @@ virCPUExpandFeatures(virArch arch,
VIR_DEBUG("nfeatures=%zu", cpu->nfeatures);
return 0;
}
+
+
+/**
+ * virCPUCopyMigratable:
+ *
+ * @arch: CPU architecture
+ * @cpu: CPU definition to be copied
+ *
+ * Makes a copy of @cpu with all features which would block migration removed.
+ *
+ * Returns the copy of the CPU or NULL on error.
+ */
+virCPUDefPtr
+virCPUCopyMigratable(virArch arch,
+ virCPUDefPtr cpu)
+{
+ struct cpuArchDriver *driver;
+
+ VIR_DEBUG("arch=%s, cpu=%p, model=%s",
+ virArchToString(arch), cpu, NULLSTR(cpu->model));
+
+ if (!(driver = cpuGetSubDriver(arch)))
+ return NULL;
+
+ if (driver->copyMigratable)
+ return driver->copyMigratable(cpu);
+ else
+ return virCPUDefCopy(cpu);
+}
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 8c238ad55..352445c40 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -118,6 +118,9 @@ typedef int
typedef int
(*virCPUArchExpandFeatures)(virCPUDefPtr cpu);
+typedef virCPUDefPtr
+(*virCPUArchCopyMigratable)(virCPUDefPtr cpu);
+
struct cpuArchDriver {
const char *name;
const virArch *arch;
@@ -138,6 +141,7 @@ struct cpuArchDriver {
virCPUArchTranslate translate;
virCPUArchConvertLegacy convertLegacy;
virCPUArchExpandFeatures expandFeatures;
+ virCPUArchCopyMigratable copyMigratable;
};
@@ -254,6 +258,10 @@ int
virCPUExpandFeatures(virArch arch,
virCPUDefPtr cpu);
+virCPUDefPtr
+virCPUCopyMigratable(virArch arch,
+ virCPUDefPtr cpu);
+
/* virCPUDataFormat and virCPUDataParse are implemented for unit tests only and
* have no real-life usage
*/
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 48648a7f4..a771b251e 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2903,6 +2903,30 @@ virCPUx86ExpandFeatures(virCPUDefPtr cpu)
}
+static virCPUDefPtr
+virCPUx86CopyMigratable(virCPUDefPtr cpu)
+{
+ virCPUDefPtr copy;
+ virCPUx86MapPtr map;
+
+ if (!(map = virCPUx86GetMap()))
+ return NULL;
+
+ if (!(copy = virCPUDefCopyWithoutModel(cpu)))
+ return NULL;
+
+ if (virCPUDefCopyModelFilter(copy, cpu, false,
+ x86FeatureIsMigratable, map) < 0)
+ goto error;
+
+ return copy;
+
+ error:
+ virCPUDefFree(copy);
+ return NULL;
+}
+
+
int
virCPUx86DataAddCPUID(virCPUDataPtr cpuData,
const virCPUx86CPUID *cpuid)
@@ -2978,4 +3002,5 @@ struct cpuArchDriver cpuDriverX86 = {
.getModels = virCPUx86GetModels,
.translate = virCPUx86Translate,
.expandFeatures = virCPUx86ExpandFeatures,
+ .copyMigratable = virCPUx86CopyMigratable,
};
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b551cb86a..dc6db3b28 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1016,6 +1016,7 @@ virCPUCheckFeature;
virCPUCompare;
virCPUCompareXML;
virCPUConvertLegacy;
+virCPUCopyMigratable;
virCPUDataCheckFeature;
virCPUDataFormat;
virCPUDataFree;
--
2.12.0
11:31 a.m.
New subject: [libvirt] [PATCH 1/7] cpu: Introduce virCPUCopyMigratable
On 03/31/2017 01:53 PM, Jiri Denemark wrote:
This new internal API makes a copy of virCPUDef while removing all
features which would block migration. It uses cpu_map.xml as a database
of such features, which should only be used as a fallback when we cannot
get the data from a hypervisor. The main goal of this API is to decouple
this filtering from virCPUUpdate so that the hypervisor driver can
filter the features according to the hypervisor.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu.c | 29 +++++++++++++++++++++++++++++
src/cpu/cpu.h | 8 ++++++++
src/cpu/cpu_x86.c | 25 +++++++++++++++++++++++++
src/libvirt_private.syms | 1 +
4 files changed, 63 insertions(+)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 93647a2ed..1cc68f142 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -1130,3 +1130,32 @@ virCPUExpandFeatures(virArch arch,
VIR_DEBUG("nfeatures=%zu", cpu->nfeatures);
return 0;
}
+
+
+/**
+ * virCPUCopyMigratable:
+ *
+ * @arch: CPU architecture
+ * @cpu: CPU definition to be copied
+ *
+ * Makes a copy of @cpu with all features which would block migration removed.
+ *
+ * Returns the copy of the CPU or NULL on error.
+ */
+virCPUDefPtr
+virCPUCopyMigratable(virArch arch,
+ virCPUDefPtr cpu)
+{
+ struct cpuArchDriver *driver;
+
+ VIR_DEBUG("arch=%s, cpu=%p, model=%s",
+ virArchToString(arch), cpu, NULLSTR(cpu->model));
+
+ if (!(driver = cpuGetSubDriver(arch)))
+ return NULL;
+
+ if (driver->copyMigratable)
+ return driver->copyMigratable(cpu);
+ else
+ return virCPUDefCopy(cpu);
Not quite sure I read the function comments similarly with respect to
what the code is doing (and peeking ahead to patch 5 where this is first
called).
Essentially this API either makes a 'migratable' version of a virCPUDef
or it just copies the (eventually called) "full" version of a virCPUDef
for architectures that don't support or need to change the "full"
virCPUDef to disable migratability features... (If I've read the tea
leaves right of course ;-))
ACK with that adjustment,
John
12:53 p.m.
New subject: [libvirt] [PATCH 2/7] qemu: Move common code in virQEMUCapsInitCPUModel one layer up
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b1245ad5d..1a15750a3 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3111,17 +3111,11 @@ virQEMUCapsCPUFilterFeatures(const char *name,
*/
static int
virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
- virDomainVirtType type,
+ qemuMonitorCPUModelInfoPtr modelInfo,
virCPUDefPtr cpu)
{
- qemuMonitorCPUModelInfoPtr modelInfo;
size_t i;
- if (type == VIR_DOMAIN_VIRT_KVM)
- modelInfo = qemuCaps->kvmCPUModelInfo;
- else
- modelInfo = qemuCaps->tcgCPUModelInfo;
-
if (!modelInfo) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("missing host CPU model info from QEMU capabilities "
@@ -3163,9 +3157,9 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
static int
virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
virDomainVirtType type,
+ qemuMonitorCPUModelInfoPtr model,
virCPUDefPtr cpu)
{
- qemuMonitorCPUModelInfoPtr model;
virCPUDataPtr data = NULL;
unsigned long long sigFamily = 0;
unsigned long long sigModel = 0;
@@ -3174,11 +3168,6 @@ virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
int ret = -1;
size_t i;
- if (type == VIR_DOMAIN_VIRT_KVM)
- model = qemuCaps->kvmCPUModelInfo;
- else
- model = qemuCaps->tcgCPUModelInfo;
-
if (!model)
return 1;
@@ -3239,12 +3228,18 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
virDomainVirtType type,
virCPUDefPtr cpu)
{
+ qemuMonitorCPUModelInfoPtr model;
int ret = 1;
+ if (type == VIR_DOMAIN_VIRT_KVM)
+ model = qemuCaps->kvmCPUModelInfo;
+ else
+ model = qemuCaps->tcgCPUModelInfo;
+
if (ARCH_IS_S390(qemuCaps->arch))
- ret = virQEMUCapsInitCPUModelS390(qemuCaps, type, cpu);
+ ret = virQEMUCapsInitCPUModelS390(qemuCaps, model, cpu);
else if (ARCH_IS_X86(qemuCaps->arch))
- ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, cpu);
+ ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, model, cpu);
if (ret == 0)
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
--
2.12.0
11:31 a.m.
New subject: [libvirt] [PATCH 2/7] qemu: Move common code in virQEMUCapsInitCPUModel one layer up
On 03/31/2017 01:53 PM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
ACK,
John
12:54 p.m.
New subject: [libvirt] [PATCH 3/7] qemu: Add migratable parameter to virQEMUCapsInitCPUModel
The caller can ask for a migratable CPU model by passing true for the
new parameter.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 36 +++++++++++++++++++++++++-----------
src/qemu/qemu_capspriv.h | 3 ++-
tests/cputest.c | 2 +-
3 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1a15750a3..b8e4e47b6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3112,7 +3112,8 @@ virQEMUCapsCPUFilterFeatures(const char *name,
static int
virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
qemuMonitorCPUModelInfoPtr modelInfo,
- virCPUDefPtr cpu)
+ virCPUDefPtr cpu,
+ bool migratable)
{
size_t i;
@@ -3140,8 +3141,12 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
if (VIR_STRDUP(feature->name, prop->name) < 0)
return -1;
- feature->policy = prop->value.boolean ? VIR_CPU_FEATURE_REQUIRE
- : VIR_CPU_FEATURE_DISABLE;
+
+ if (!prop->value.boolean ||
+ (migratable && prop->migratable == VIR_TRISTATE_BOOL_NO))
+ feature->policy = VIR_CPU_FEATURE_DISABLE;
+ else
+ feature->policy = VIR_CPU_FEATURE_REQUIRE;
cpu->nfeatures++;
}
@@ -3158,7 +3163,8 @@ static int
virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
virDomainVirtType type,
qemuMonitorCPUModelInfoPtr model,
- virCPUDefPtr cpu)
+ virCPUDefPtr cpu,
+ bool migratable)
{
virCPUDataPtr data = NULL;
unsigned long long sigFamily = 0;
@@ -3179,9 +3185,13 @@ virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
switch (prop->type) {
case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN:
- if (prop->value.boolean &&
- virCPUx86DataAddFeature(data, prop->name) < 0)
+ if (!prop->value.boolean ||
+ (migratable && prop->migratable == VIR_TRISTATE_BOOL_NO))
+ continue;
+
+ if (virCPUx86DataAddFeature(data, prop->name) < 0)
goto cleanup;
+
break;
case QEMU_MONITOR_CPU_PROPERTY_STRING:
@@ -3220,13 +3230,14 @@ virQEMUCapsInitCPUModelX86(virQEMUCapsPtr qemuCaps,
/**
* Returns 0 when host CPU model provided by QEMU was filled in qemuCaps,
- * 1 when the caller should fall back to using virCapsPtr->host.cpu,
+ * 1 when the caller should fall back to other methods
* -1 on error.
*/
int
virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
virDomainVirtType type,
- virCPUDefPtr cpu)
+ virCPUDefPtr cpu,
+ bool migratable)
{
qemuMonitorCPUModelInfoPtr model;
int ret = 1;
@@ -3236,10 +3247,13 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
else
model = qemuCaps->tcgCPUModelInfo;
+ if (migratable && model && !model->migratability)
+ return 1;
+
if (ARCH_IS_S390(qemuCaps->arch))
- ret = virQEMUCapsInitCPUModelS390(qemuCaps, model, cpu);
+ ret = virQEMUCapsInitCPUModelS390(qemuCaps, model, cpu, migratable);
else if (ARCH_IS_X86(qemuCaps->arch))
- ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, model, cpu);
+ ret = virQEMUCapsInitCPUModelX86(qemuCaps, type, model, cpu, migratable);
if (ret == 0)
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
@@ -3268,7 +3282,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
cpu->match = VIR_CPU_MATCH_EXACT;
cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
- if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu)) < 0) {
+ if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu, false)) < 0) {
goto error;
} else if (rc == 1) {
VIR_DEBUG("No host CPU model info from QEMU; probing host CPU
directly");
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index 61ccd4517..1baaaf334 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -81,7 +81,8 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
int
virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
virDomainVirtType type,
- virCPUDefPtr cpu);
+ virCPUDefPtr cpu,
+ bool migratable);
void
virQEMUCapsSetCPUModelInfo(virQEMUCapsPtr qemuCaps,
diff --git a/tests/cputest.c b/tests/cputest.c
index 3d3e43f16..8c07cf4f6 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -709,7 +709,7 @@ cpuTestJSONCPUID(const void *arg)
cpu->match = VIR_CPU_MATCH_EXACT;
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
- if (virQEMUCapsInitCPUModel(qemuCaps, VIR_DOMAIN_VIRT_KVM, cpu) != 0)
+ if (virQEMUCapsInitCPUModel(qemuCaps, VIR_DOMAIN_VIRT_KVM, cpu, false) != 0)
goto cleanup;
ret = cpuTestCompareXML(data->arch, cpu, result, false);
--
2.12.0
11:31 a.m.
New subject: [libvirt] [PATCH 3/7] qemu: Add migratable parameter to virQEMUCapsInitCPUModel
On 03/31/2017 01:54 PM, Jiri Denemark wrote:
The caller can ask for a migratable CPU model by passing true for
the
new parameter.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 36 +++++++++++++++++++++++++-----------
src/qemu/qemu_capspriv.h | 3 ++-
tests/cputest.c | 2 +-
3 files changed, 28 insertions(+), 13 deletions(-)
ACK,
John
12:54 p.m.
New subject: [libvirt] [PATCH 4/7] qemu: Move qemuCaps->{kvm, tcg}CPUModel into a struct
We will need to store two more host CPU models and nested structs look
better than separate items with long complicated names.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 65 ++++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b8e4e47b6..c75aa570c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -414,8 +414,10 @@ struct _virQEMUCaps {
* 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;
+ struct {
+ virCPUDefPtr kvm;
+ virCPUDefPtr tcg;
+ } hostCPU;
};
struct virQEMUCapsSearchData {
@@ -2082,6 +2084,31 @@ virQEMUCapsNew(void)
}
+static int
+virQEMUCapsCopyHostCPUData(virQEMUCapsPtr dst,
+ virQEMUCapsPtr src)
+{
+
+ if (src->kvmCPUModelInfo &&
+ !(dst->kvmCPUModelInfo =
qemuMonitorCPUModelInfoCopy(src->kvmCPUModelInfo)))
+ return -1;
+
+ if (src->tcgCPUModelInfo &&
+ !(dst->tcgCPUModelInfo =
qemuMonitorCPUModelInfoCopy(src->tcgCPUModelInfo)))
+ return -1;
+
+ if (src->hostCPU.kvm &&
+ !(dst->hostCPU.kvm = virCPUDefCopy(src->hostCPU.kvm)))
+ return -1;
+
+ if (src->hostCPU.tcg &&
+ !(dst->hostCPU.tcg = virCPUDefCopy(src->hostCPU.tcg)))
+ return -1;
+
+ return 0;
+}
+
+
virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
{
virQEMUCapsPtr ret = virQEMUCapsNew();
@@ -2119,20 +2146,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 (virQEMUCapsCopyHostCPUData(ret, qemuCaps) < 0)
goto error;
if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
@@ -2183,8 +2197,8 @@ void virQEMUCapsDispose(void *obj)
qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
- virCPUDefFree(qemuCaps->kvmCPUModel);
- virCPUDefFree(qemuCaps->tcgCPUModel);
+ virCPUDefFree(qemuCaps->hostCPU.kvm);
+ virCPUDefFree(qemuCaps->hostCPU.tcg);
}
void
@@ -2413,9 +2427,9 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
virDomainVirtType type)
{
if (type == VIR_DOMAIN_VIRT_KVM)
- return qemuCaps->kvmCPUModel;
+ return qemuCaps->hostCPU.kvm;
else
- return qemuCaps->tcgCPUModel;
+ return qemuCaps->hostCPU.tcg;
}
@@ -3296,9 +3310,9 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
}
if (type == VIR_DOMAIN_VIRT_KVM)
- qemuCaps->kvmCPUModel = cpu;
+ qemuCaps->hostCPU.kvm = cpu;
else
- qemuCaps->tcgCPUModel = cpu;
+ qemuCaps->hostCPU.tcg = cpu;
cleanup:
virCPUDefFree(hostCPU);
@@ -4053,10 +4067,9 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
qemuCaps->kvmCPUModelInfo = NULL;
qemuCaps->tcgCPUModelInfo = NULL;
- virCPUDefFree(qemuCaps->kvmCPUModel);
- virCPUDefFree(qemuCaps->tcgCPUModel);
- qemuCaps->kvmCPUModel = NULL;
- qemuCaps->tcgCPUModel = NULL;
+ virCPUDefFree(qemuCaps->hostCPU.kvm);
+ virCPUDefFree(qemuCaps->hostCPU.tcg);
+ memset(&qemuCaps->hostCPU, 0, sizeof(qemuCaps->hostCPU));
}
--
2.12.0
11:34 a.m.
New subject: [libvirt] [PATCH 4/7] qemu: Move qemuCaps->{kvm, tcg}CPUModel into a struct
On 03/31/2017 01:54 PM, Jiri Denemark wrote:
We will need to store two more host CPU models and nested structs
look
better than separate items with long complicated names.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 65 ++++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 26 deletions(-)
There's two patches in here... One to have a separate
virQEMUCapsCopyHostCPUData and the other to create the 'hostCPU'
diff --git a/src/qemu/qemu_capabilities.c
b/src/qemu/qemu_capabilities.c
index b8e4e47b6..c75aa570c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -414,8 +414,10 @@ struct _virQEMUCaps {
* 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;
+ struct {
+ virCPUDefPtr kvm;
+ virCPUDefPtr tcg;
+ } hostCPU;
};
Considering what you do in the next patch... Further pushing down kvm &
tcg into another struct, why not introduce virQEMUCapsCPUModel now and
make it a bit "flatter":
struct virQEMUCapsCPUModel capsCPUModel;
where
virQEMUCapsCPUModel is:
virCPUDefPtr kvmFull;
virCPUDefPtr tcgFull;
and eventually adds
virCPUDefPtr kvmMigrate;
virCPUDefPtr tcgMigrate;
You could also eventually have use the Ptr and pass by & rather than
seeing things like "hostCpu.kvm.full", the helper routines would take a
virQEMUCapsCPUModelPtr and be able to use "capsCPUModel->kvmFull",
"capsCPUModel->tcgFull", etc.
I only say this because when I see "a.b.c" in code, I have two thoughts
- one is this a union and is there anyway to "hide" the
'path.to.the.data.we.want'...
struct virQEMUCapsSearchData {
@@ -2082,6 +2084,31 @@ virQEMUCapsNew(void)
}
+static int
+virQEMUCapsCopyHostCPUData(virQEMUCapsPtr dst,
+ virQEMUCapsPtr src)
+{
+
+ if (src->kvmCPUModelInfo &&
+ !(dst->kvmCPUModelInfo =
qemuMonitorCPUModelInfoCopy(src->kvmCPUModelInfo)))
+ return -1;
+
+ if (src->tcgCPUModelInfo &&
+ !(dst->tcgCPUModelInfo =
qemuMonitorCPUModelInfoCopy(src->tcgCPUModelInfo)))
+ return -1;
+
+ if (src->hostCPU.kvm &&
+ !(dst->hostCPU.kvm = virCPUDefCopy(src->hostCPU.kvm)))
+ return -1;
+
+ if (src->hostCPU.tcg &&
+ !(dst->hostCPU.tcg = virCPUDefCopy(src->hostCPU.tcg)))
+ return -1;
+
+ return 0;
+}
+
+
virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
{
virQEMUCapsPtr ret = virQEMUCapsNew();
@@ -2119,20 +2146,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 (virQEMUCapsCopyHostCPUData(ret, qemuCaps) < 0)
goto error;
The above is "somewhat" it's own hunk with some code motion and some
change to use the hostCpu.{kvm|tcg}
if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
@@ -2183,8 +2197,8 @@ void virQEMUCapsDispose(void *obj)
qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
- virCPUDefFree(qemuCaps->kvmCPUModel);
- virCPUDefFree(qemuCaps->tcgCPUModel);
+ virCPUDefFree(qemuCaps->hostCPU.kvm);
+ virCPUDefFree(qemuCaps->hostCPU.tcg);
Hmmm. perhaps a 3rd possible patch... Considering patch 5 and a single
capsCPUModel, this could become a helper that knows how to virCPUDefFree
each of the structure elements.
}
void
@@ -2413,9 +2427,9 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
virDomainVirtType type)
{
if (type == VIR_DOMAIN_VIRT_KVM)
- return qemuCaps->kvmCPUModel;
+ return qemuCaps->hostCPU.kvm;
else
- return qemuCaps->tcgCPUModel;
+ return qemuCaps->hostCPU.tcg;
}
@@ -3296,9 +3310,9 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
}
if (type == VIR_DOMAIN_VIRT_KVM)
- qemuCaps->kvmCPUModel = cpu;
+ qemuCaps->hostCPU.kvm = cpu;
else
- qemuCaps->tcgCPUModel = cpu;
+ qemuCaps->hostCPU.tcg = cpu;
Seems to me this hunk "could" be it's own helper too - something like
virQEMUCapsSetHostModel... especially once migrate CPU is added...
cleanup:
virCPUDefFree(hostCPU);
@@ -4053,10 +4067,9 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
qemuCaps->kvmCPUModelInfo = NULL;
qemuCaps->tcgCPUModelInfo = NULL;
- virCPUDefFree(qemuCaps->kvmCPUModel);
- virCPUDefFree(qemuCaps->tcgCPUModel);
- qemuCaps->kvmCPUModel = NULL;
- qemuCaps->tcgCPUModel = NULL;
+ virCPUDefFree(qemuCaps->hostCPU.kvm);
+ virCPUDefFree(qemuCaps->hostCPU.tcg);
Similar usage of single helper to free everything.
John
+ memset(&qemuCaps->hostCPU, 0,
sizeof(qemuCaps->hostCPU));
}
6:07 a.m.
New subject: [libvirt] [PATCH 4/7] qemu: Move qemuCaps->{kvm, tcg}CPUModel into a struct
On Tue, Apr 04, 2017 at 12:34:49 -0400, John Ferlan wrote:
On 03/31/2017 01:54 PM, Jiri Denemark wrote:
> We will need to store two more host CPU models and nested structs look
> better than separate items with long complicated names.
>
> Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
> ---
> src/qemu/qemu_capabilities.c | 65 ++++++++++++++++++++++++++------------------
> 1 file changed, 39 insertions(+), 26 deletions(-)
>
There's two patches in here... One to have a separate
virQEMUCapsCopyHostCPUData and the other to create the 'hostCPU'
Oops, I accidentally pushed this series when I wanted to push the
"qemu: Properly reset all migration capabilities" one :-( Are you OK
with me sending the additional changes as follow-up patches rather than
reverting the patches and pushing the updated ones?
Jirka
6:18 a.m.
New subject: [libvirt] [PATCH 4/7] qemu: Move qemuCaps->{kvm, tcg}CPUModel into a struct
On Fri, Apr 07, 2017 at 01:07:52PM +0200, Jiri Denemark wrote:
On Tue, Apr 04, 2017 at 12:34:49 -0400, John Ferlan wrote:
>
>
> On 03/31/2017 01:54 PM, Jiri Denemark wrote:
> > We will need to store two more host CPU models and nested structs look
> > better than separate items with long complicated names.
> >
> > Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
> > ---
> > src/qemu/qemu_capabilities.c | 65 ++++++++++++++++++++++++++------------------
> > 1 file changed, 39 insertions(+), 26 deletions(-)
> >
>
> There's two patches in here... One to have a separate
> virQEMUCapsCopyHostCPUData and the other to create the 'hostCPU'
Oops, I accidentally pushed this series when I wanted to push the
"qemu: Properly reset all migration capabilities" one :-( Are you OK
with me sending the additional changes as follow-up patches rather than
reverting the patches and pushing the updated ones?
Reverting the patches and doing them right would IMO look better,
especially if we ever need to backport them somewhere.
Jan
6:26 a.m.
New subject: [libvirt] [PATCH 4/7] qemu: Move qemuCaps->{kvm, tcg}CPUModel into a struct
On Fri, Apr 07, 2017 at 13:18:53 +0200, Ján Tomko wrote:
On Fri, Apr 07, 2017 at 01:07:52PM +0200, Jiri Denemark wrote:
>On Tue, Apr 04, 2017 at 12:34:49 -0400, John Ferlan wrote:
>>
>>
>> On 03/31/2017 01:54 PM, Jiri Denemark wrote:
>> > We will need to store two more host CPU models and nested structs look
>> > better than separate items with long complicated names.
>> >
>> > Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
>> > ---
>> > src/qemu/qemu_capabilities.c | 65
++++++++++++++++++++++++++------------------
>> > 1 file changed, 39 insertions(+), 26 deletions(-)
>> >
>>
>> There's two patches in here... One to have a separate
>> virQEMUCapsCopyHostCPUData and the other to create the 'hostCPU'
>
>Oops, I accidentally pushed this series when I wanted to push the
>"qemu: Properly reset all migration capabilities" one :-( Are you OK
>with me sending the additional changes as follow-up patches rather than
>reverting the patches and pushing the updated ones?
>
Reverting the patches and doing them right would IMO look better,
especially if we ever need to backport them somewhere.
OK, I guess it's probably also going to be easier to make the changes
and review the resulting patches. I've sent the reverts...
Jirka
12:54 p.m.
New subject: [libvirt] [PATCH 5/7] qemu: Store migratable host CPU model in qemuCaps
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 91 ++++++++++++++++++++++++++++++++++----------
1 file changed, 71 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c75aa570c..b426a5abc 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -373,6 +373,14 @@ struct virQEMUCapsMachineType {
unsigned int maxCpus;
bool hotplugCpus;
};
+
+typedef struct _virQEMUCapsCPUModel virQEMUCapsCPUModel;
+typedef virQEMUCapsCPUModel *virQEMUCapsCPUModelPtr;
+struct _virQEMUCapsCPUModel {
+ virCPUDefPtr full;
+ virCPUDefPtr migratable;
+};
+
/*
* Update the XML parser/formatter when adding more
* information to this struct so that it gets cached
@@ -415,8 +423,8 @@ struct _virQEMUCaps {
* time we probe QEMU or load the results from the cache.
*/
struct {
- virCPUDefPtr kvm;
- virCPUDefPtr tcg;
+ virQEMUCapsCPUModel kvm;
+ virQEMUCapsCPUModel tcg;
} hostCPU;
};
@@ -2097,12 +2105,20 @@ virQEMUCapsCopyHostCPUData(virQEMUCapsPtr dst,
!(dst->tcgCPUModelInfo =
qemuMonitorCPUModelInfoCopy(src->tcgCPUModelInfo)))
return -1;
- if (src->hostCPU.kvm &&
- !(dst->hostCPU.kvm = virCPUDefCopy(src->hostCPU.kvm)))
+ if (src->hostCPU.kvm.full &&
+ !(dst->hostCPU.kvm.full = virCPUDefCopy(src->hostCPU.kvm.full)))
return -1;
- if (src->hostCPU.tcg &&
- !(dst->hostCPU.tcg = virCPUDefCopy(src->hostCPU.tcg)))
+ if (src->hostCPU.kvm.migratable &&
+ !(dst->hostCPU.kvm.migratable =
virCPUDefCopy(src->hostCPU.kvm.migratable)))
+ return -1;
+
+ if (src->hostCPU.tcg.full &&
+ !(dst->hostCPU.tcg.full = virCPUDefCopy(src->hostCPU.tcg.full)))
+ return -1;
+
+ if (src->hostCPU.tcg.migratable &&
+ !(dst->hostCPU.tcg.migratable =
virCPUDefCopy(src->hostCPU.tcg.migratable)))
return -1;
return 0;
@@ -2197,8 +2213,10 @@ void virQEMUCapsDispose(void *obj)
qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
- virCPUDefFree(qemuCaps->hostCPU.kvm);
- virCPUDefFree(qemuCaps->hostCPU.tcg);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.full);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.migratable);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.full);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.migratable);
}
void
@@ -2427,9 +2445,9 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
virDomainVirtType type)
{
if (type == VIR_DOMAIN_VIRT_KVM)
- return qemuCaps->hostCPU.kvm;
+ return qemuCaps->hostCPU.kvm.full;
else
- return qemuCaps->hostCPU.tcg;
+ return qemuCaps->hostCPU.tcg.full;
}
@@ -3276,26 +3294,40 @@ 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;
+ virQEMUCapsCPUModelPtr model;
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) {
@@ -3309,10 +3341,26 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
goto error;
}
+ if (!(migCPU = virQEMUCapsNewHostCPUModel()))
+ goto error;
+
+ if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, migCPU, true)) < 0) {
+ goto error;
+ } else if (rc == 1) {
+ VIR_DEBUG("CPU migratibility not provided by QEMU");
+
+ virCPUDefFree(migCPU);
+ if (!(migCPU = virCPUCopyMigratable(qemuCaps->arch, cpu)))
+ goto error;
+ }
+
if (type == VIR_DOMAIN_VIRT_KVM)
- qemuCaps->hostCPU.kvm = cpu;
+ model = &qemuCaps->hostCPU.kvm;
else
- qemuCaps->hostCPU.tcg = cpu;
+ model = &qemuCaps->hostCPU.tcg;
+
+ model->full = cpu;
+ model->migratable = migCPU;
cleanup:
virCPUDefFree(hostCPU);
@@ -3320,6 +3368,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
error:
virCPUDefFree(cpu);
+ virCPUDefFree(migCPU);
virResetLastError();
goto cleanup;
}
@@ -4067,8 +4116,10 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
qemuCaps->kvmCPUModelInfo = NULL;
qemuCaps->tcgCPUModelInfo = NULL;
- virCPUDefFree(qemuCaps->hostCPU.kvm);
- virCPUDefFree(qemuCaps->hostCPU.tcg);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.full);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.migratable);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.full);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.migratable);
memset(&qemuCaps->hostCPU, 0, sizeof(qemuCaps->hostCPU));
}
--
2.12.0
11:36 a.m.
New subject: [libvirt] [PATCH 5/7] qemu: Store migratable host CPU model in qemuCaps
On 03/31/2017 01:54 PM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 91 ++++++++++++++++++++++++++++++++++----------
1 file changed, 71 insertions(+), 20 deletions(-)
There's three patches in one here... Adding virQEMUCapsCPUModel, adding
virQEMUCapsNewHostCPUModel, and creating the migratable CPUs defs and code.
Once I got to this patch, I went back to patch 4 with my thoughts around
nested structs and a flatter capsCPUModel instead...
diff --git a/src/qemu/qemu_capabilities.c
b/src/qemu/qemu_capabilities.c
index c75aa570c..b426a5abc 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -373,6 +373,14 @@ struct virQEMUCapsMachineType {
unsigned int maxCpus;
bool hotplugCpus;
};
+
+typedef struct _virQEMUCapsCPUModel virQEMUCapsCPUModel;
+typedef virQEMUCapsCPUModel *virQEMUCapsCPUModelPtr;
+struct _virQEMUCapsCPUModel {
+ virCPUDefPtr full;
+ virCPUDefPtr migratable;
+};
+
/*
* Update the XML parser/formatter when adding more
* information to this struct so that it gets cached
@@ -415,8 +423,8 @@ struct _virQEMUCaps {
* time we probe QEMU or load the results from the cache.
*/
struct {
- virCPUDefPtr kvm;
- virCPUDefPtr tcg;
+ virQEMUCapsCPUModel kvm;
+ virQEMUCapsCPUModel tcg;
} hostCPU;
};
@@ -2097,12 +2105,20 @@ virQEMUCapsCopyHostCPUData(virQEMUCapsPtr dst,
!(dst->tcgCPUModelInfo =
qemuMonitorCPUModelInfoCopy(src->tcgCPUModelInfo)))
return -1;
- if (src->hostCPU.kvm &&
- !(dst->hostCPU.kvm = virCPUDefCopy(src->hostCPU.kvm)))
+ if (src->hostCPU.kvm.full &&
+ !(dst->hostCPU.kvm.full = virCPUDefCopy(src->hostCPU.kvm.full)))
return -1;
- if (src->hostCPU.tcg &&
- !(dst->hostCPU.tcg = virCPUDefCopy(src->hostCPU.tcg)))
+ if (src->hostCPU.kvm.migratable &&
+ !(dst->hostCPU.kvm.migratable =
virCPUDefCopy(src->hostCPU.kvm.migratable)))
+ return -1;
+
+ if (src->hostCPU.tcg.full &&
+ !(dst->hostCPU.tcg.full = virCPUDefCopy(src->hostCPU.tcg.full)))
+ return -1;
+
+ if (src->hostCPU.tcg.migratable &&
+ !(dst->hostCPU.tcg.migratable =
virCPUDefCopy(src->hostCPU.tcg.migratable)))
return -1;
Lots of copy'n'paste, but if you went with a flatter capsCPUModel, then
there'd be less churn in name changes and only new settings for
kvmMigrate and tcgMigrate.
return 0;
@@ -2197,8 +2213,10 @@ void virQEMUCapsDispose(void *obj)
qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
- virCPUDefFree(qemuCaps->hostCPU.kvm);
- virCPUDefFree(qemuCaps->hostCPU.tcg);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.full);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.migratable);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.full);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.migratable);
One helper to free...
}
void
@@ -2427,9 +2445,9 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
virDomainVirtType type)
{
if (type == VIR_DOMAIN_VIRT_KVM)
- return qemuCaps->hostCPU.kvm;
+ return qemuCaps->hostCPU.kvm.full;
else
- return qemuCaps->hostCPU.tcg;
+ return qemuCaps->hostCPU.tcg.full;
This wouldn't need a change with a flat capsCPUModel, just leaving the
subsequent patch to handle being able to return the migratable version...
}
@@ -3276,26 +3294,40 @@ 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;
+ virQEMUCapsCPUModelPtr model;
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) {
@@ -3309,10 +3341,26 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
goto error;
}
+ if (!(migCPU = virQEMUCapsNewHostCPUModel()))
+ goto error;
+
+ if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, migCPU, true)) < 0) {
+ goto error;
+ } else if (rc == 1) {
+ VIR_DEBUG("CPU migratibility not provided by QEMU");
migratability (although the dictionary for my email client doesn't like
either word).
+
+ virCPUDefFree(migCPU);
+ if (!(migCPU = virCPUCopyMigratable(qemuCaps->arch, cpu)))
+ goto error;
+ }
+
if (type == VIR_DOMAIN_VIRT_KVM)
- qemuCaps->hostCPU.kvm = cpu;
+ model = &qemuCaps->hostCPU.kvm;
else
- qemuCaps->hostCPU.tcg = cpu;
+ model = &qemuCaps->hostCPU.tcg;
+
+ model->full = cpu;
+ model->migratable = migCPU;
If we had a virQEMUCapsSetHostModel this would be included there too
hiding from this code...
cleanup:
virCPUDefFree(hostCPU);
@@ -3320,6 +3368,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
error:
virCPUDefFree(cpu);
+ virCPUDefFree(migCPU);
virResetLastError();
goto cleanup;
}
@@ -4067,8 +4116,10 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
qemuCaps->kvmCPUModelInfo = NULL;
qemuCaps->tcgCPUModelInfo = NULL;
- virCPUDefFree(qemuCaps->hostCPU.kvm);
- virCPUDefFree(qemuCaps->hostCPU.tcg);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.full);
+ virCPUDefFree(qemuCaps->hostCPU.kvm.migratable);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.full);
+ virCPUDefFree(qemuCaps->hostCPU.tcg.migratable);
This is why I suggested a single API in patch 4...
John
memset(&qemuCaps->hostCPU, 0,
sizeof(qemuCaps->hostCPU));
}
12:54 p.m.
New subject: [libvirt] [PATCH 6/7] qemu: Pass migratable host model to virCPUUpdate
This will allow us to drop feature filtering from virCPUUpdate where it
was just a hack.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 19 ++++++++++++++-----
src/qemu/qemu_capabilities.h | 3 ++-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_process.c | 4 ++--
tests/cputest.c | 7 ++++++-
5 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b426a5abc..d4aace56d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2442,12 +2442,20 @@ virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
virCPUDefPtr
virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
- virDomainVirtType type)
+ virDomainVirtType type,
+ bool migratable)
{
+ virQEMUCapsCPUModelPtr model;
+
if (type == VIR_DOMAIN_VIRT_KVM)
- return qemuCaps->hostCPU.kvm.full;
+ model = &qemuCaps->hostCPU.kvm;
else
- return qemuCaps->hostCPU.tcg.full;
+ model = &qemuCaps->hostCPU.tcg;
+
+ if (migratable)
+ return model->migratable;
+ else
+ return model->full;
}
@@ -2465,7 +2473,7 @@ virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch);
case VIR_CPU_MODE_HOST_MODEL:
- return !!virQEMUCapsGetHostModel(qemuCaps, type);
+ return !!virQEMUCapsGetHostModel(qemuCaps, type, false);
case VIR_CPU_MODE_CUSTOM:
if (type == VIR_DOMAIN_VIRT_KVM)
@@ -5500,7 +5508,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,
+ false);
domCaps->cpu.hostModel = virCPUDefCopy(cpu);
}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d44682f2a..410fa28b2 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -450,7 +450,8 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
char ***names,
size_t *count);
virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
- virDomainVirtType type);
+ virDomainVirtType type,
+ bool migratable);
bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
virCapsPtr caps,
virDomainVirtType type,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 64d2d7105..c1a8ba065 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6886,7 +6886,7 @@ 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, false);
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 a20beb13c..07d0883c3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5300,12 +5300,12 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
if (def->cpu->check == VIR_CPU_CHECK_PARTIAL &&
virCPUCompare(caps->host.arch,
- virQEMUCapsGetHostModel(qemuCaps, def->virtType),
+ virQEMUCapsGetHostModel(qemuCaps, def->virtType, false),
def->cpu, true) < 0)
return -1;
if (virCPUUpdate(def->os.arch, def->cpu,
- virQEMUCapsGetHostModel(qemuCaps, def->virtType)) < 0)
+ virQEMUCapsGetHostModel(qemuCaps, def->virtType, true)) < 0)
goto cleanup;
if (virQEMUCapsGetCPUDefinitions(qemuCaps, def->virtType,
diff --git a/tests/cputest.c b/tests/cputest.c
index 8c07cf4f6..efa891dc1 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.0
11:37 a.m.
New subject: [libvirt] [PATCH 6/7] qemu: Pass migratable host model to virCPUUpdate
On 03/31/2017 01:54 PM, Jiri Denemark wrote:
This will allow us to drop feature filtering from virCPUUpdate where
it
was just a hack.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 19 ++++++++++++++-----
src/qemu/qemu_capabilities.h | 3 ++-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_process.c | 4 ++--
tests/cputest.c | 7 ++++++-
5 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b426a5abc..d4aace56d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2442,12 +2442,20 @@ virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
virCPUDefPtr
virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
- virDomainVirtType type)
+ virDomainVirtType type,
+ bool migratable)
{
+ virQEMUCapsCPUModelPtr model;
+
if (type == VIR_DOMAIN_VIRT_KVM)
- return qemuCaps->hostCPU.kvm.full;
+ model = &qemuCaps->hostCPU.kvm;
else
- return qemuCaps->hostCPU.tcg.full;
+ model = &qemuCaps->hostCPU.tcg;
+
+ if (migratable)
+ return model->migratable;
+ else
+ return model->full;
}
This would have obvious impacts with a flatter model.
ACK in general for the algorithm adjustments
John
@@ -2465,7 +2473,7 @@ virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch);
case VIR_CPU_MODE_HOST_MODEL:
- return !!virQEMUCapsGetHostModel(qemuCaps, type);
+ return !!virQEMUCapsGetHostModel(qemuCaps, type, false);
case VIR_CPU_MODE_CUSTOM:
if (type == VIR_DOMAIN_VIRT_KVM)
@@ -5500,7 +5508,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,
+ false);
domCaps->cpu.hostModel = virCPUDefCopy(cpu);
}
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d44682f2a..410fa28b2 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -450,7 +450,8 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
char ***names,
size_t *count);
virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
- virDomainVirtType type);
+ virDomainVirtType type,
+ bool migratable);
bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
virCapsPtr caps,
virDomainVirtType type,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 64d2d7105..c1a8ba065 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6886,7 +6886,7 @@ 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, false);
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 a20beb13c..07d0883c3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5300,12 +5300,12 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
if (def->cpu->check == VIR_CPU_CHECK_PARTIAL &&
virCPUCompare(caps->host.arch,
- virQEMUCapsGetHostModel(qemuCaps, def->virtType),
+ virQEMUCapsGetHostModel(qemuCaps, def->virtType, false),
def->cpu, true) < 0)
return -1;
if (virCPUUpdate(def->os.arch, def->cpu,
- virQEMUCapsGetHostModel(qemuCaps, def->virtType)) < 0)
+ virQEMUCapsGetHostModel(qemuCaps, def->virtType, true)) < 0)
goto cleanup;
if (virQEMUCapsGetCPUDefinitions(qemuCaps, def->virtType,
diff --git a/tests/cputest.c b/tests/cputest.c
index 8c07cf4f6..efa891dc1 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;
}
12:54 p.m.
New subject: [libvirt] [PATCH 7/7] cpu: Drop feature filtering from virCPUUpdate
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(a)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.0
11:39 a.m.
New subject: [libvirt] [PATCH 7/7] cpu: Drop feature filtering from virCPUUpdate
On 03/31/2017 01:54 PM, 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(a)redhat.com>
---
src/cpu/cpu_x86.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
ACK
John
2820
days inactive
2827
days old
17 comments
3 participants
participants (3)
-
Jiri Denemark -
John Ferlan -
Ján Tomko