On Wed, Feb 11, 2026 at 12:50:24 +0100, Michal Privoznik via Devel wrote:
> From: Michal Privoznik <[email protected]>
>
> Since some hyperv features might be already enabled/disabled when
> entering qemuProcessEnableDomainFeatures() only those which are
> not set in domain XML (i.e. are VIR_TRISTATE_SWITCH_ABSENT)
> should be modified. Furthermore, some features are not a simple
> on/off switch, but a number or a string even. Well, that doesn't
> matter really as the logic for setting them is the same: only set
> their value iff they are not already set.
>
> Resolves: https://issues.redhat.com/browse/RHEL-148219
> Signed-off-by: Michal Privoznik <[email protected]>
> ---
> src/qemu/qemu_process.c | 24 ++++++++++++-------
> .../hyperv-host-model.x86_64-latest.args | 2 +-
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index c5b2a5fda8..27aa1896d4 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6960,30 +6960,38 @@ qemuProcessEnableDomainFeatures(virDomainObj *vm)
> }
>
> for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
> + virTristateSwitch origState = vm->def->hyperv.features[i];
> +
> if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(hv->features, i))
> continue;
>
> - vm->def->hyperv.features[i] = VIR_TRISTATE_SWITCH_ON;
> + if (vm->def->hyperv.features[i] == VIR_TRISTATE_SWITCH_ABSENT)
> + vm->def->hyperv.features[i] = VIR_TRISTATE_SWITCH_ON;
[...]
>
> if (i == VIR_DOMAIN_HYPERV_SPINLOCKS) {
> if (hv->spinlocks != 0) {
> - vm->def->hyperv.spinlocks = hv->spinlocks;
> + if (vm->def->hyperv.spinlocks == 0)
> + vm->def->hyperv.spinlocks = hv->spinlocks;
This feature is parsed as:
case VIR_DOMAIN_HYPERV_SPINLOCKS:
if (value != VIR_TRISTATE_SWITCH_ON)
break;
if (virXMLPropUIntDefault(node, "retries", 0, VIR_XML_PROP_NONE,
&def->hyperv.spinlocks, UINT_MAX) < 0) {
return -1;
}
if (def->hyperv.spinlocks < 0xFFF) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("HyperV spinlock retry count must be at least
4095"));
return -1;
}
Thus the value will never be 0 if it was enabled in the XML. Practically
though I don't think it matters because the user in such case set what
they wanted.
To make it more obvious that this would happen I'd probably skip the
attempt to set anything if origState of this feature is not _ABSENT.
In fact IMO none of the settings should be auto-filled if the user
provided config of the feature.
> } else {
> - vm->def->hyperv.features[i] = VIR_TRISTATE_SWITCH_ABSENT;
> + vm->def->hyperv.features[i] = origState;
> }
> } else if (i == VIR_DOMAIN_HYPERV_STIMER) {
> - vm->def->hyperv.stimer_direct = hv->stimer_direct;
> + if (vm->def->hyperv.stimer_direct == VIR_TRISTATE_SWITCH_ABSENT)
> + vm->def->hyperv.stimer_direct = hv->stimer_direct;
> /* Both hv-stimer and hv-stimer-direct require hv-time which is
> * expose as a timer. Enable it. */
> qemuProcessMaybeAddHypervTimer(vm->def);
> } else if (i == VIR_DOMAIN_HYPERV_TLBFLUSH) {
> - vm->def->hyperv.tlbflush_direct = hv->tlbflush_direct;
> - vm->def->hyperv.tlbflush_extended = hv->tlbflush_extended;
> + if (vm->def->hyperv.tlbflush_direct ==
> VIR_TRISTATE_SWITCH_ABSENT)
> + vm->def->hyperv.tlbflush_direct = hv->tlbflush_direct;
> + if (vm->def->hyperv.tlbflush_extended ==
> VIR_TRISTATE_SWITCH_ABSENT)
> + vm->def->hyperv.tlbflush_extended = hv->tlbflush_extended;
> } else if (i == VIR_DOMAIN_HYPERV_VENDOR_ID) {
> if (hv->vendor_id) {
> - vm->def->hyperv.vendor_id = g_strdup(hv->vendor_id);
> + if (!vm->def->hyperv.vendor_id)
> + vm->def->hyperv.vendor_id = g_strdup(hv->vendor_id);
e.g. this feature can't exist without having the 'value' field populated
by the user.
So [1] could then be:
if (vm->def->hyperv.features[i] == VIR_TRISTATE_SWITCH_ABSENT) {
vm->def->hyperv.features[i] = VIR_TRISTATE_SWITCH_ON;
} else {
/* if the user provided already config for this we skip the
* auto-population code */
continue;
}
and the rest would then require no change at all.
