[PATCH 3/3] qemu_validate: Drop VIR_DOMAIN_HYPERV_STIMER dependency on VIR_DOMAIN_HYPERV_VPINDEX

Michal Privoznik via Devel posted 3 patches 3 weeks, 5 days ago
There is a newer version of this series
[PATCH 3/3] qemu_validate: Drop VIR_DOMAIN_HYPERV_STIMER dependency on VIR_DOMAIN_HYPERV_VPINDEX
Posted by Michal Privoznik via Devel 3 weeks, 5 days ago
From: Michal Privoznik <mprivozn@redhat.com>

The original commit (v11.9.0-rc1~84) added a dependency checking
of VIR_DOMAIN_HYPERV_STIMER on VIR_DOMAIN_HYPERV_VPINDEX
(meaning, if stimer is on then vpindex must also be on). It
justified this by citing QEMU documentation:

Per QEMU documentation (docs/system/i386/hyperv.rst):

``hv-stimer``
  Enables Hyper-V synthetic timers. <snip/>

  Requires: ``hv-vpindex``, ``hv-synic``, ``hv-time``

While the documentation is almost correct (see previous commit
when it's incorrect), the code express no dependency on vpindex
(kvm_hyperv_properties[] array from target/i386/kvm/kvm.c):

    [HYPERV_FEAT_STIMER] = {
        .desc = "synthetic timers (hv-stimer)",
        .flags = {
            {.func = HV_CPUID_FEATURES, .reg = R_EAX,
             .bits = HV_SYNTIMERS_AVAILABLE}
        },
        .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
    },

If transitivity is taken into account then the documentation is
of course correct (minus that one aforementioned special case).
Well, there's no need for us to implement transitional checks.
VIR_DOMAIN_HYPERV_STIMER requires VIR_DOMAIN_HYPERV_SYNIC and
whether that requires VIR_DOMAIN_HYPERV_VPINDEX is another
question.

Just drop the transitive check.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
Resolves: https://issues.redhat.com/browse/RHEL-138689
Fixes: da261327ea94300d1aa2d3b76ba9dcd4de6160f6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_validate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index a8b1029d0c..ed746ba8dc 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -130,7 +130,6 @@ qemuValidateDomainDefHypervFeatures(const virDomainDef *def)
         }
     }
 
-    CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_STIMER, VIR_DOMAIN_HYPERV_VPINDEX);
     CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_STIMER, VIR_DOMAIN_HYPERV_SYNIC);
 
     CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_TLBFLUSH, VIR_DOMAIN_HYPERV_VPINDEX);
-- 
2.52.0
Re: [PATCH 3/3] qemu_validate: Drop VIR_DOMAIN_HYPERV_STIMER dependency on VIR_DOMAIN_HYPERV_VPINDEX
Posted by Daniel P. Berrangé via Devel 3 weeks, 5 days ago
On Tue, Jan 06, 2026 at 03:25:18PM +0100, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <mprivozn@redhat.com>
> 
> The original commit (v11.9.0-rc1~84) added a dependency checking
> of VIR_DOMAIN_HYPERV_STIMER on VIR_DOMAIN_HYPERV_VPINDEX
> (meaning, if stimer is on then vpindex must also be on). It
> justified this by citing QEMU documentation:
> 
> Per QEMU documentation (docs/system/i386/hyperv.rst):
> 
> ``hv-stimer``
>   Enables Hyper-V synthetic timers. <snip/>
> 
>   Requires: ``hv-vpindex``, ``hv-synic``, ``hv-time``
> 
> While the documentation is almost correct (see previous commit
> when it's incorrect), the code express no dependency on vpindex
> (kvm_hyperv_properties[] array from target/i386/kvm/kvm.c):
> 
>     [HYPERV_FEAT_STIMER] = {
>         .desc = "synthetic timers (hv-stimer)",
>         .flags = {
>             {.func = HV_CPUID_FEATURES, .reg = R_EAX,
>              .bits = HV_SYNTIMERS_AVAILABLE}
>         },
>         .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_TIME)
>     },
> 
> If transitivity is taken into account then the documentation is
> of course correct (minus that one aforementioned special case).
> Well, there's no need for us to implement transitional checks.
> VIR_DOMAIN_HYPERV_STIMER requires VIR_DOMAIN_HYPERV_SYNIC and
> whether that requires VIR_DOMAIN_HYPERV_VPINDEX is another
> question.
> 
> Just drop the transitive check.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
> Resolves: https://issues.redhat.com/browse/RHEL-138689
> Fixes: da261327ea94300d1aa2d3b76ba9dcd4de6160f6
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_validate.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|