[libvirt] [PATCH RESEND 2/4] qemu: Validate notify VM exit feature is available only on x86

Lin Ma posted 4 patches 2 years, 7 months ago
[libvirt] [PATCH RESEND 2/4] qemu: Validate notify VM exit feature is available only on x86
Posted by Lin Ma 2 years, 7 months ago
Signed-off-by: Lin Ma <lma@suse.de>
---
 src/qemu/qemu_validate.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index a53729d349..6ec5af0028 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -219,8 +219,18 @@ qemuValidateDomainDefFeatures(const virDomainDef *def,
             }
             break;
 
-        case VIR_DOMAIN_FEATURE_SMM:
         case VIR_DOMAIN_FEATURE_KVM:
+            if (def->kvm_features) {
+                if (def->kvm_features->features[VIR_DOMAIN_KVM_NOTIFY_VMEXIT] != VIR_TRISTATE_SWITCH_ABSENT &&
+                    !ARCH_IS_X86(def->os.arch)) {
+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                                   _("Notification VM exit is only supported on x86 architecture"));
+                    return -1;
+                }
+            }
+            break;
+
+        case VIR_DOMAIN_FEATURE_SMM:
         case VIR_DOMAIN_FEATURE_XEN:
         case VIR_DOMAIN_FEATURE_ACPI:
         case VIR_DOMAIN_FEATURE_PAE:
-- 
2.41.0
Re: [libvirt] [PATCH RESEND 2/4] qemu: Validate notify VM exit feature is available only on x86
Posted by Jonathon Jongsma 2 years, 7 months ago
On 7/3/23 1:30 AM, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@suse.de>
> ---
>   src/qemu/qemu_validate.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index a53729d349..6ec5af0028 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -219,8 +219,18 @@ qemuValidateDomainDefFeatures(const virDomainDef *def,
>               }
>               break;
>   
> -        case VIR_DOMAIN_FEATURE_SMM:
>           case VIR_DOMAIN_FEATURE_KVM:
> +            if (def->kvm_features) {
> +                if (def->kvm_features->features[VIR_DOMAIN_KVM_NOTIFY_VMEXIT] != VIR_TRISTATE_SWITCH_ABSENT &&
> +                    !ARCH_IS_X86(def->os.arch)) {
> +                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                   _("Notification VM exit is only supported on x86 architecture"));
> +                    return -1;
> +                }
> +            }
> +            break;
> +
> +        case VIR_DOMAIN_FEATURE_SMM:
>           case VIR_DOMAIN_FEATURE_XEN:
>           case VIR_DOMAIN_FEATURE_ACPI:
>           case VIR_DOMAIN_FEATURE_PAE:


This doesn't seem sufficient. If this was only added in qemu 7.2, an 
architecture check doesn't seem sufficient to decide whether to enable 
this feature. For example, on Fedora 37 (qemu 7.0.0), if I configure a 
domain to use this option, it doesn't give me a configuration error, but 
(predictably) gives an error when trying to start the domain:

error: internal error: process exited while connecting to monitor: 
2023-07-05T15:58:43.783724Z qemu-system-x86_64: -accel 
kvm,notify-vmexit=disable: Property 'kvm-accel.notify-vmexit' not found

Ideally we would check that the specific qemu binary supports this 
option. We generally use capabilities for this.

Jonathon