On Wed, Feb 11, 2026 at 12:50:24 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
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 <mprivozn@redhat.com> --- 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.