qemuProcessUpdateGuestCPU: Check cpu if check=full is set

[libvirt PATCH] qemuProcessUpdateGuestCPU: Check cpu if check=full is set

Posted by Tim Wiederhake 3 years, 1 month ago

libvirt performs cpu checking if "check" is set to "partial", but skips
checking the cpu if "check" is set to "full".

See https://bugzilla.redhat.com/show_bug.cgi?id=1840770

Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
 src/qemu/qemu_process.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bfa742577f..5b8c1397ef 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6149,6 +6149,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
     if (virCPUConvertLegacy(hostarch, def->cpu) < 0)
         return -1;
 
+    if (def->cpu->check == VIR_CPU_CHECK_FULL) {
+        virCPUDefPtr host = virQEMUCapsGetHostModel(qemuCaps, def->virtType,
+                                                    VIR_QEMU_CAPS_HOST_CPU_FULL);
+
+        if (virCPUCompare(hostarch, host, def->cpu, true) < 0)
+            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

Re: [libvirt PATCH] qemuProcessUpdateGuestCPU: Check cpu if check=full is set

Posted by Jiri Denemark 3 years, 1 month ago

On Thu, Feb 25, 2021 at 08:17:09 +0100, Tim Wiederhake wrote:
> libvirt performs cpu checking if "check" is set to "partial", but skips
> checking the cpu if "check" is set to "full".

This is intentional because QEMU knows better. I wish we had no CPU
comparison in libvirt at all, but we can't do that for backward
compatibility...

The real problem here is that unlike all other feature policies in our
CPU definition 'forbid' cannot be checked via QEMU.

> See https://bugzilla.redhat.com/show_bug.cgi?id=1840770
> 
> Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> ---
>  src/qemu/qemu_process.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index bfa742577f..5b8c1397ef 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6149,6 +6149,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
>      if (virCPUConvertLegacy(hostarch, def->cpu) < 0)
>          return -1;
>  
> +    if (def->cpu->check == VIR_CPU_CHECK_FULL) {
> +        virCPUDefPtr host = virQEMUCapsGetHostModel(qemuCaps, def->virtType,
> +                                                    VIR_QEMU_CAPS_HOST_CPU_FULL);
> +
> +        if (virCPUCompare(hostarch, host, def->cpu, true) < 0)
> +            return -1;
> +    }
> +

I believe this should be replaced with a more targeted approach to only
check forbidden features. And I guess we can do so for check != none.

Jirka