[libvirt PATCH v2 0/1] qemuProcessUpdateGuestCPU: Check host cpu for forbidden features

V1: https://listman.redhat.com/archives/libvir-list/2021-February/msg01275.ht= ml Changes since V1: * Only check for forbidden features * Check if `check` !=3D none * Renamed patch Tim Wiederhake (1): qemuProcessUpdateGuestCPU: Check host cpu for forbidden features src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) --=20 2.26.2

See https://bugzilla.redhat.com/show_bug.cgi?id=1840770 Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bfa742577f..cecf606312 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6149,6 +6149,33 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (virCPUConvertLegacy(hostarch, def->cpu) < 0) return -1; + if (def->cpu->check != VIR_CPU_CHECK_NONE) { + virCPUDefPtr host; + size_t i; + + host = virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_FULL); + + for (i = 0; i < def->cpu->nfeatures; ++i) { + virCPUFeatureDefPtr feature; + + if (def->cpu->features[i].policy != VIR_CPU_FEATURE_FORBID) + continue; + + feature = virCPUDefFindFeature(host, def->cpu->features[i].name); + if (!feature) + continue; + + if (feature->policy == VIR_CPU_FEATURE_DISABLE) + continue; + + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("Host CPU provides forbidden feature '%s'"), + def->cpu->features[i].name); + return -1; + } + } + /* nothing to update for host-passthrough / maximum */ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && def->cpu->mode != VIR_CPU_MODE_MAXIMUM) { -- 2.26.2

On 2/25/21 10:23 AM, Tim Wiederhake wrote:
See https://bugzilla.redhat.com/show_bug.cgi?id=1840770
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bfa742577f..cecf606312 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6149,6 +6149,33 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (virCPUConvertLegacy(hostarch, def->cpu) < 0) return -1;
+ if (def->cpu->check != VIR_CPU_CHECK_NONE) { + virCPUDefPtr host; + size_t i; + + host = virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_FULL); + + for (i = 0; i < def->cpu->nfeatures; ++i) { + virCPUFeatureDefPtr feature; + + if (def->cpu->features[i].policy != VIR_CPU_FEATURE_FORBID) + continue; + + feature = virCPUDefFindFeature(host, def->cpu->features[i].name); + if (!feature) + continue; + + if (feature->policy == VIR_CPU_FEATURE_DISABLE) + continue; + + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("Host CPU provides forbidden feature '%s'"), + def->cpu->features[i].name); + return -1; + } + } + /* nothing to update for host-passthrough / maximum */ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && def->cpu->mode != VIR_CPU_MODE_MAXIMUM) {

On Thu, Feb 25, 2021 at 14:23:06 +0100, Tim Wiederhake wrote:
See https://bugzilla.redhat.com/show_bug.cgi?id=1840770
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_process.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bfa742577f..cecf606312 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6149,6 +6149,33 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (virCPUConvertLegacy(hostarch, def->cpu) < 0) return -1;
+ if (def->cpu->check != VIR_CPU_CHECK_NONE) { + virCPUDefPtr host; + size_t i; + + host = virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_FULL); + + for (i = 0; i < def->cpu->nfeatures; ++i) { + virCPUFeatureDefPtr feature; + + if (def->cpu->features[i].policy != VIR_CPU_FEATURE_FORBID) + continue; + + feature = virCPUDefFindFeature(host, def->cpu->features[i].name);
This would crash in case virQEMUCapsGetHostModel returned NULL, which may happen quite easily, especially on non-x86 architectures.
+ if (!feature) + continue; + + if (feature->policy == VIR_CPU_FEATURE_DISABLE) + continue; + + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("Host CPU provides forbidden feature '%s'"), + def->cpu->features[i].name); + return -1; + } + } +
This new code is a good candidate for separation into a new function. I believe we don't need architecture specific implementation for this function, but something like virCPUCheckForbiddenFeatures in src/cpu/cpu.c seems like a logical choice.
/* nothing to update for host-passthrough / maximum */ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && def->cpu->mode != VIR_CPU_MODE_MAXIMUM) {
Jirka
participants (3)
-
Daniel Henrique Barboza
-
Jiri Denemark
-
Tim Wiederhake