On Thu, Oct 07, 2021 at 11:53:00AM +0200, Jiri Denemark wrote:
On Thu, Oct 07, 2021 at 10:05:12 +0100, Daniel P. Berrangé wrote:
The -cpu arg gained support for feature=on|off syntax for the x86 emulator in 2.4.0
commit 38e5c119c2925812bd441450ab9e5e00fc79e662 Author: Eduardo Habkost <ehabkost@redhat.com> Date: Mon Mar 23 17:29:32 2015 -0300
target-i386: Register QOM properties for feature flags
Most other targets gained this syntax even earlier in 1.4.1
commit 1590bbcb02921dfe8e3cf66e3a3aafd31193babf Author: Andreas Färber <afaerber@suse.de> Date: Mon Mar 3 23:33:51 2014 +0100
cpu: Implement CPUClass::parse_features() for the rest of CPUs
CPUs who do not provide their own implementation of feature parsing will treat each option as a QOM property and set it to the supplied value.
There appears no reason to keep supporting "+|-feature" syntax, given the current minimum QEMU version.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_command.c | 34 ++++++------------- tests/qemuxml2argvdata/cpu-Haswell2.args | 2 +- tests/qemuxml2argvdata/cpu-Haswell3.args | 2 +- .../qemuxml2argvdata/cpu-cache-disable3.args | 2 +- .../cpu-check-default-partial.args | 2 +- tests/qemuxml2argvdata/cpu-eoi-disabled.args | 2 +- tests/qemuxml2argvdata/cpu-eoi-enabled.args | 2 +- tests/qemuxml2argvdata/cpu-exact1.args | 2 +- .../cpu-exact2-nofallback.args | 2 +- tests/qemuxml2argvdata/cpu-exact2.args | 2 +- tests/qemuxml2argvdata/cpu-fallback.args | 2 +- tests/qemuxml2argvdata/cpu-host-kvmclock.args | 2 +- .../qemuxml2argvdata/cpu-host-model-cmt.args | 2 +- .../cpu-host-model-fallback.args | 2 +- .../cpu-host-model-vendor.args | 2 +- tests/qemuxml2argvdata/cpu-host-model.args | 2 +- .../cpu-host-passthrough-features.args | 2 +- tests/qemuxml2argvdata/cpu-kvmclock.args | 2 +- tests/qemuxml2argvdata/cpu-minimum1.args | 2 +- tests/qemuxml2argvdata/cpu-minimum2.args | 2 +- tests/qemuxml2argvdata/cpu-strict1.args | 2 +- .../cpu-translation.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/cpu-tsc-frequency.args | 2 +- .../eoi-disabled.x86_64-latest.args | 2 +- .../eoi-enabled.x86_64-latest.args | 2 +- .../graphics-spice-timeout.args | 2 +- .../kvmclock+eoi-disabled.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/kvmclock.args | 2 +- .../pci-bridge-many-disks.args | 2 +- .../pv-spinlock-disabled.x86_64-latest.args | 2 +- .../pv-spinlock-enabled.x86_64-latest.args | 2 +- 31 files changed, 41 insertions(+), 53 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index eaa1e0deb9..0f1cdd9372 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6242,21 +6242,6 @@ qemuBuildGlobalControllerCommandLine(virCommand *cmd, }
-static void -qemuBuildCpuFeature(virQEMUCaps *qemuCaps, - virBuffer *buf, - const char *name, - bool state) -{ - name = virQEMUCapsCPUFeatureToQEMU(qemuCaps, name); - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) - virBufferAsprintf(buf, ",%s=%s", name, state ? "on" : "off"); - else - virBufferAsprintf(buf, ",%c%s", state ? '+' : '-', name); -}
I guess it would have been easier and perhaps clearer to just remove the else branch and the test itself, because...
@@ -6433,13 +6420,14 @@ qemuBuildCpuCommandLine(virCommand *cmd, }
if (def->apic_eoi) { - qemuBuildCpuFeature(qemuCaps, &buf, "kvm_pv_eoi", - def->apic_eoi == VIR_TRISTATE_SWITCH_ON); + virBufferAsprintf(&buf, ",kvm_pv_eoi=%s", def->apic_eoi == + VIR_TRISTATE_SWITCH_ON ? "on" : "off");
This is affected by the same issue spotted by Peter in v1. In other words, when replacing qemuBuildCpuFeature you need to make sure the feature name goes through virQEMUCapsCPUFeatureToQEMU().
virQEMUCapsCPUFeatureToQEMU() is a constant mapping. It makes sense to use it in the earlier loop, because we're getting feature names dynamically from the XML config. In this case though, we've just got a constant string. Seems better to just pass the right constant string in directly rather than send it through this remapping. 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 :|