On 1/6/26 15:39, Daniel P. Berrangé wrote:
> On Tue, Jan 06, 2026 at 03:25:17PM +0100, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <[email protected]>
>>
>> Turns out, that synic hyperv enlightenment not always requires
>> vpindex. Some (older) machine types (e.g. pc-i440fx-3.0,
>> pc-i440fx-rhel7.6.0) can run with synic enabled and vpindex
>> disabled. This is because they did enable 'x-hv-synic-kvm-only'
>> CPU property, but starting from QEMU commit v3.1.0-rc0~44^2~9 the
>> property is disabled by default.
>>
>> To avoid parsing machine type version, let's just skip this
>> dependency validation for all i440fx machine types with a note to
>> remove the limitation once affected machine types go out of
>> support.
>
> pc-q35-3.0 has the same behaviour as pc-i440fx-3.0, so
> it isn't accurate to say the problem is specific to i440fx.
> That is only the case in RHEL downstream since it strips
> out upstream machine types and none of RHEL q35 machine
> types in RHEL-9 are old enough to be affected.
Aaagrh. You're right.
>
>>
>> Now, q35 has this dependency, although it might be not visible at
>> the first glance. Inside of target/i386/kvm/kvm.c there's
>> kvm_hyperv_properties[] array declared and in there
>> HYPERV_FEAT_SYNIC doesn't have any .dependencies set. But looking
>> couple of pages down at kvm_hyperv_expand_features() function,
>> there's the following check:
>>
>> /* Additional dependencies not covered by kvm_hyperv_properties[] */
>> if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) &&
>> !cpu->hyperv_synic_kvm_only &&
>> !hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)) {
>> error_setg(errp, "Hyper-V %s requires Hyper-V %s",
>> kvm_hyperv_properties[HYPERV_FEAT_SYNIC].desc,
>> kvm_hyperv_properties[HYPERV_FEAT_VPINDEX].desc);
>> return false;
>> }
>>
>> And as noted above, 'hyperv_synic_kvm_only' is false by default.
>>
>> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/837
>> Resolves: https://issues.redhat.com/browse/RHEL-138689
>> Fixes: 1822d030c32d9857020ee8385b0a8808a29a472f
>> Signed-off-by: Michal Privoznik <[email protected]>
>> ---
>> src/qemu/qemu_validate.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index da08fd17cd..a8b1029d0c 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -112,7 +112,13 @@ qemuValidateDomainDefHypervFeatures(const virDomainDef
>> *def)
>> return -1;
>> }
>>
>> - CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
>> + if (!qemuDomainIsI440FX(def)) {
>> + /* Some i440fx machine types don't have this dependency and unless
>> we
>> + * want to parse machine type version (we do not), just skip this
>> + * check. The worst that'll happen is QEMU will error out later.
>> + * TODO: Remove once pc-i440fx-3.0 is no longer supported. */
>> + CHECK_HV_FEAT(VIR_DOMAIN_HYPERV_SYNIC, VIR_DOMAIN_HYPERV_VPINDEX);
>> + }
>
> I think we need to remove this check entirely until we can depend on a QEMU
> upstream version that drops the -3.0 machine types. This happened in the
> QEMU 10.1.0 release which started automatically removing versioned machine
> types that were >= 18 versions / 6 years old.
Yeah. But bumping min QEMU version in libvirt to 10.1.0 is in far
future. So removing this check makes only sense. Will post v2 shortly.
Michal
