[libvirt PATCH v2 0/2] qemu: changes to use feature=on|off syntax for -cpu unconditionally

Given our min QEMU, it doesn't seem like we need to support either the old "-feature" / "+feature" syntax, nor the bare "feature" syntax. v2: - Now with cpu name compatibility retained Daniel P. Berrangé (2): qemu: remove use of (+|-)name syntax for -cpu featres qemu: remove use of implicit boolean syntax for -cpu features src/qemu/qemu_command.c | 40 +++++++------------ .../clock-timer-hyperv-rtc.args | 2 +- 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 +- .../hyperv-stimer-direct.x86_64-latest.args | 2 +- .../qemuxml2argvdata/hyperv.x86_64-4.0.0.args | 2 +- .../hyperv.x86_64-latest.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 +- 35 files changed, 48 insertions(+), 60 deletions(-) -- 2.31.1

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); -} - - static int qemuBuildCpuModelArgStr(virQEMUDriver *driver, const virDomainDef *def, @@ -6329,15 +6314,17 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver, virBufferAsprintf(buf, ",vendor=%s", cpu->vendor_id); for (i = 0; i < cpu->nfeatures; i++) { + const char *featname = virQEMUCapsCPUFeatureToQEMU( + qemuCaps, cpu->features[i].name); switch ((virCPUFeaturePolicy) cpu->features[i].policy) { case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: - qemuBuildCpuFeature(qemuCaps, buf, cpu->features[i].name, true); + virBufferAsprintf(buf, ",%s=on", featname); break; case VIR_CPU_FEATURE_DISABLE: case VIR_CPU_FEATURE_FORBID: - qemuBuildCpuFeature(qemuCaps, buf, cpu->features[i].name, false); + virBufferAsprintf(buf, ",%s=off", featname); break; case VIR_CPU_FEATURE_OPTIONAL: @@ -6394,8 +6381,8 @@ qemuBuildCpuCommandLine(virCommand *cmd, switch ((virDomainTimerNameType)timer->name) { case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: if (timer->present != -1) { - qemuBuildCpuFeature(qemuCaps, &buf, "kvmclock", - !!timer->present); + virBufferAsprintf(&buf, ",kvmclock=%s", + timer->present ? "on" : "off"); } break; case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: @@ -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"); } if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK]) { - qemuBuildCpuFeature(qemuCaps, &buf, VIR_CPU_x86_KVM_PV_UNHALT, - def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] == VIR_TRISTATE_SWITCH_ON); + virBufferAsprintf(&buf, ",%s=%s", VIR_CPU_x86_KVM_PV_UNHALT, + def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] == + VIR_TRISTATE_SWITCH_ON ? "on" : "off"); } if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) { diff --git a/tests/qemuxml2argvdata/cpu-Haswell2.args b/tests/qemuxml2argvdata/cpu-Haswell2.args index ed08de0d72..2d23840418 100644 --- a/tests/qemuxml2argvdata/cpu-Haswell2.args +++ b/tests/qemuxml2argvdata/cpu-Haswell2.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu Haswell,-hle,-rtm \ +-cpu Haswell,hle=off,rtm=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-Haswell3.args b/tests/qemuxml2argvdata/cpu-Haswell3.args index 110017cf0e..4bc2e0bc00 100644 --- a/tests/qemuxml2argvdata/cpu-Haswell3.args +++ b/tests/qemuxml2argvdata/cpu-Haswell3.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu Haswell,+rtm,+hle \ +-cpu Haswell,rtm=on,hle=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-cache-disable3.args b/tests/qemuxml2argvdata/cpu-cache-disable3.args index 57e5da66ad..5f4e31d298 100644 --- a/tests/qemuxml2argvdata/cpu-cache-disable3.args +++ b/tests/qemuxml2argvdata/cpu-cache-disable3.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+acpi,+ss,+ht,+tm,+pbe,+ds_cpl,+vmx,+est,+tm2,+cx16,+xtpr,+lahf_lm,l3-cache=off \ +-cpu core2duo,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,ds_cpl=on,vmx=on,est=on,tm2=on,cx16=on,xtpr=on,lahf_lm=on,l3-cache=off \ -m 214 \ -realtime mlock=off \ -smp 1,sockets=1,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-check-default-partial.args b/tests/qemuxml2argvdata/cpu-check-default-partial.args index d1010b1e5d..fbc5b84e0a 100644 --- a/tests/qemuxml2argvdata/cpu-check-default-partial.args +++ b/tests/qemuxml2argvdata/cpu-check-default-partial.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+acpi,+ss,+ht,+tm,+pbe,+ds_cpl,+vmx,+est,+tm2,+cx16,+xtpr,+lahf_lm \ +-cpu core2duo,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,ds_cpl=on,vmx=on,est=on,tm2=on,cx16=on,xtpr=on,lahf_lm=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-eoi-disabled.args b/tests/qemuxml2argvdata/cpu-eoi-disabled.args index ce4b2e358f..e7cdcd2fd4 100644 --- a/tests/qemuxml2argvdata/cpu-eoi-disabled.args +++ b/tests/qemuxml2argvdata/cpu-eoi-disabled.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu qemu32,-kvm_pv_eoi \ +-cpu qemu32,kvm_pv_eoi=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-eoi-enabled.args b/tests/qemuxml2argvdata/cpu-eoi-enabled.args index 6e60e11792..e22fc0410e 100644 --- a/tests/qemuxml2argvdata/cpu-eoi-enabled.args +++ b/tests/qemuxml2argvdata/cpu-eoi-enabled.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu qemu32,+kvm_pv_eoi \ +-cpu qemu32,kvm_pv_eoi=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-exact1.args b/tests/qemuxml2argvdata/cpu-exact1.args index f0498090ed..b764431173 100644 --- a/tests/qemuxml2argvdata/cpu-exact1.args +++ b/tests/qemuxml2argvdata/cpu-exact1.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu qemu64,-mca,-pse36,-clflush,-syscall,-nx,-lm,-svm \ +-cpu qemu64,mca=off,pse36=off,clflush=off,syscall=off,nx=off,lm=off,svm=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-exact2-nofallback.args b/tests/qemuxml2argvdata/cpu-exact2-nofallback.args index 0bb2f4c088..64609add1b 100644 --- a/tests/qemuxml2argvdata/cpu-exact2-nofallback.args +++ b/tests/qemuxml2argvdata/cpu-exact2-nofallback.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+ht,+tm,+ds_cpl,+xtpr,+3dnowext,+lahf_lm,-nx,-cx16,-tm2,-pbe,-ss,-sse4a,-wdt \ +-cpu core2duo,ds=on,ht=on,tm=on,ds_cpl=on,xtpr=on,3dnowext=on,lahf_lm=on,nx=off,cx16=off,tm2=off,pbe=off,ss=off,sse4a=off,wdt=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-exact2.args b/tests/qemuxml2argvdata/cpu-exact2.args index 0bb2f4c088..64609add1b 100644 --- a/tests/qemuxml2argvdata/cpu-exact2.args +++ b/tests/qemuxml2argvdata/cpu-exact2.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+ht,+tm,+ds_cpl,+xtpr,+3dnowext,+lahf_lm,-nx,-cx16,-tm2,-pbe,-ss,-sse4a,-wdt \ +-cpu core2duo,ds=on,ht=on,tm=on,ds_cpl=on,xtpr=on,3dnowext=on,lahf_lm=on,nx=off,cx16=off,tm2=off,pbe=off,ss=off,sse4a=off,wdt=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-fallback.args b/tests/qemuxml2argvdata/cpu-fallback.args index 0dbe3bf366..5a51295c74 100644 --- a/tests/qemuxml2argvdata/cpu-fallback.args +++ b/tests/qemuxml2argvdata/cpu-fallback.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu Penryn,-sse4.1,-sse4.2,-popcnt,-aes \ +-cpu Penryn,sse4.1=off,sse4.2=off,popcnt=off,aes=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-host-kvmclock.args b/tests/qemuxml2argvdata/cpu-host-kvmclock.args index 63bd54c1b2..0a62f64747 100644 --- a/tests/qemuxml2argvdata/cpu-host-kvmclock.args +++ b/tests/qemuxml2argvdata/cpu-host-kvmclock.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu host,-kvmclock \ +-cpu host,kvmclock=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-host-model-cmt.args b/tests/qemuxml2argvdata/cpu-host-model-cmt.args index ffa9236a47..59ec868330 100644 --- a/tests/qemuxml2argvdata/cpu-host-model-cmt.args +++ b/tests/qemuxml2argvdata/cpu-host-model-cmt.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,+smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm \ +-cpu Haswell,vme=on,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,dtes64=on,monitor=on,ds_cpl=on,vmx=on,smx=on,est=on,tm2=on,xtpr=on,pdcm=on,f16c=on,rdrand=on,pdpe1gb=on,abm=on,lahf_lm=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-host-model-fallback.args b/tests/qemuxml2argvdata/cpu-host-model-fallback.args index e224f30d84..ed9424896f 100644 --- a/tests/qemuxml2argvdata/cpu-host-model-fallback.args +++ b/tests/qemuxml2argvdata/cpu-host-model-fallback.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu Penryn,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+monitor,+ds_cpl,+vmx,+est,+tm2,+xtpr,-sse4.1,+cx16,+lahf_lm \ +-cpu Penryn,vme=on,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,monitor=on,ds_cpl=on,vmx=on,est=on,tm2=on,xtpr=on,sse4.1=off,cx16=on,lahf_lm=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-host-model-vendor.args b/tests/qemuxml2argvdata/cpu-host-model-vendor.args index 967d74c7c2..a0705a835d 100644 --- a/tests/qemuxml2argvdata/cpu-host-model-vendor.args +++ b/tests/qemuxml2argvdata/cpu-host-model-vendor.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu 'core2duo,vendor=Libvirt QEMU,+ds,+acpi,+ss,+ht,+tm,+pbe,+ds_cpl,+vmx,+est,+tm2,+cx16,+xtpr,+lahf_lm' \ +-cpu 'core2duo,vendor=Libvirt QEMU,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,ds_cpl=on,vmx=on,est=on,tm2=on,cx16=on,xtpr=on,lahf_lm=on' \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-host-model.args b/tests/qemuxml2argvdata/cpu-host-model.args index 96fe2f9edb..bce7e37d50 100644 --- a/tests/qemuxml2argvdata/cpu-host-model.args +++ b/tests/qemuxml2argvdata/cpu-host-model.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine q35,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+acpi,+ss,+ht,+tm,+pbe,+ds_cpl,+vmx,+est,+tm2,+cx16,+xtpr,+lahf_lm \ +-cpu core2duo,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,ds_cpl=on,vmx=on,est=on,tm2=on,cx16=on,xtpr=on,lahf_lm=on \ -m 214 \ -realtime mlock=off \ -smp 1,sockets=1,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-host-passthrough-features.args b/tests/qemuxml2argvdata/cpu-host-passthrough-features.args index 8c03bd1171..468088f0b2 100644 --- a/tests/qemuxml2argvdata/cpu-host-passthrough-features.args +++ b/tests/qemuxml2argvdata/cpu-host-passthrough-features.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu host,+abm,+ds,-invtsc \ +-cpu host,abm=on,ds=on,invtsc=off \ -m 214 \ -realtime mlock=off \ -smp 1,sockets=1,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-kvmclock.args b/tests/qemuxml2argvdata/cpu-kvmclock.args index e12fbbb503..9bd3748f04 100644 --- a/tests/qemuxml2argvdata/cpu-kvmclock.args +++ b/tests/qemuxml2argvdata/cpu-kvmclock.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,-kvmclock \ +-cpu core2duo,kvmclock=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-minimum1.args b/tests/qemuxml2argvdata/cpu-minimum1.args index ff3365b7ff..bc86256b00 100644 --- a/tests/qemuxml2argvdata/cpu-minimum1.args +++ b/tests/qemuxml2argvdata/cpu-minimum1.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+acpi,+ss,+ht,+tm,+pbe,+ds_cpl,+vmx,+est,+tm2,+cx16,+xtpr,+lahf_lm \ +-cpu core2duo,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,ds_cpl=on,vmx=on,est=on,tm2=on,cx16=on,xtpr=on,lahf_lm=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-minimum2.args b/tests/qemuxml2argvdata/cpu-minimum2.args index 7a07bea1b4..0839c8100c 100644 --- a/tests/qemuxml2argvdata/cpu-minimum2.args +++ b/tests/qemuxml2argvdata/cpu-minimum2.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+acpi,+ss,+ht,+tm,+pbe,+ds_cpl,+vmx,+est,+tm2,+cx16,+xtpr,+lahf_lm,-syscall,-nx,-lm,-svm \ +-cpu core2duo,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,ds_cpl=on,vmx=on,est=on,tm2=on,cx16=on,xtpr=on,lahf_lm=on,syscall=off,nx=off,lm=off,svm=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-strict1.args b/tests/qemuxml2argvdata/cpu-strict1.args index fcd159e5fd..d5ffdabcc8 100644 --- a/tests/qemuxml2argvdata/cpu-strict1.args +++ b/tests/qemuxml2argvdata/cpu-strict1.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+acpi,+ht,+tm,+ds_cpl,+vmx,+est,+xtpr,+3dnowext,+lahf_lm,-nx,-cx16,-tm2,-pbe,-ss,-sse4a,-wdt \ +-cpu core2duo,ds=on,acpi=on,ht=on,tm=on,ds_cpl=on,vmx=on,est=on,xtpr=on,3dnowext=on,lahf_lm=on,nx=off,cx16=off,tm2=off,pbe=off,ss=off,sse4a=off,wdt=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args b/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args index bb6b5c9986..10aa65150f 100644 --- a/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args +++ b/tests/qemuxml2argvdata/cpu-translation.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu Haswell,pclmulqdq=on,ds-cpl=on,tsc-adjust=on,fxsr-opt=on,lahf-lm=on,cmp-legacy=on,nodeid-msr=on,perfctr-core=on,perfctr-nb=on,kvm-pv-eoi=on,kvm-pv-unhalt=on \ +-cpu Haswell,pclmulqdq=on,ds-cpl=on,tsc-adjust=on,fxsr-opt=on,lahf-lm=on,cmp-legacy=on,nodeid-msr=on,perfctr-core=on,perfctr-nb=on,kvm_pv_eoi=on,kvm_pv_unhalt=on \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ diff --git a/tests/qemuxml2argvdata/cpu-tsc-frequency.args b/tests/qemuxml2argvdata/cpu-tsc-frequency.args index 3a7d69d6b7..0f9bc25ab7 100644 --- a/tests/qemuxml2argvdata/cpu-tsc-frequency.args +++ b/tests/qemuxml2argvdata/cpu-tsc-frequency.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,+smx,+est,+tm2,+xtpr,+pdcm,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,+invtsc,tsc-frequency=4567890000 \ +-cpu Haswell,vme=on,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,dtes64=on,monitor=on,ds_cpl=on,vmx=on,smx=on,est=on,tm2=on,xtpr=on,pdcm=on,f16c=on,rdrand=on,pdpe1gb=on,abm=on,lahf_lm=on,invtsc=on,tsc-frequency=4567890000 \ -m 214 \ -realtime mlock=off \ -smp 1,sockets=1,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args b/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args index 6f67fb3bc6..32ae60cd90 100644 --- a/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args +++ b/tests/qemuxml2argvdata/eoi-disabled.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,kvm-pv-eoi=off \ +-cpu qemu64,kvm_pv_eoi=off \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ diff --git a/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args b/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args index 8ba1627a53..ae6f735459 100644 --- a/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args +++ b/tests/qemuxml2argvdata/eoi-enabled.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,kvm-pv-eoi=on \ +-cpu qemu64,kvm_pv_eoi=on \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ diff --git a/tests/qemuxml2argvdata/graphics-spice-timeout.args b/tests/qemuxml2argvdata/graphics-spice-timeout.args index 61d986b2e6..9a14cfc3b2 100644 --- a/tests/qemuxml2argvdata/graphics-spice-timeout.args +++ b/tests/qemuxml2argvdata/graphics-spice-timeout.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=spice \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-f14/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu core2duo,+ds,+acpi,+ss,+ht,+tm,+pbe,+ds_cpl,+vmx,+est,+tm2,+cx16,+xtpr,+lahf_lm \ +-cpu core2duo,ds=on,acpi=on,ss=on,ht=on,tm=on,pbe=on,ds_cpl=on,vmx=on,est=on,tm2=on,cx16=on,xtpr=on,lahf_lm=on \ -m 1024 \ -realtime mlock=off \ -smp 2,sockets=1,cores=2,threads=1 \ diff --git a/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args b/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args index 57bdb536ab..0e67351e38 100644 --- a/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args +++ b/tests/qemuxml2argvdata/kvmclock+eoi-disabled.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,kvmclock=off,kvm-pv-eoi=off \ +-cpu qemu64,kvmclock=off,kvm_pv_eoi=off \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ diff --git a/tests/qemuxml2argvdata/kvmclock.args b/tests/qemuxml2argvdata/kvmclock.args index 174cc7ad07..cce20dbfed 100644 --- a/tests/qemuxml2argvdata/kvmclock.args +++ b/tests/qemuxml2argvdata/kvmclock.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu qemu32,-kvmclock \ +-cpu qemu32,kvmclock=off \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/pci-bridge-many-disks.args b/tests/qemuxml2argvdata/pci-bridge-many-disks.args index eec754d2ae..500b4d0e47 100644 --- a/tests/qemuxml2argvdata/pci-bridge-many-disks.args +++ b/tests/qemuxml2argvdata/pci-bridge-many-disks.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-lots-of-disks/master-key.aes \ -machine pc-i440fx-1.4,accel=tcg,usb=off,dump-guest-core=off \ --cpu qemu64,-kvmclock \ +-cpu qemu64,kvmclock=off \ -bios /usr/share/seabios/bios.bin \ -m 3907 \ -realtime mlock=off \ diff --git a/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args b/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args index a7a107f4b8..2188ff477d 100644 --- a/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args +++ b/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,kvm-pv-unhalt=off \ +-cpu qemu64,kvm_pv_unhalt=off \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ diff --git a/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args b/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args index b217034eb0..a4f1eaedbc 100644 --- a/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args +++ b/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,kvm-pv-unhalt=on \ +-cpu qemu64,kvm_pv_unhalt=on \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ -- 2.31.1

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(). ...
diff --git a/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args b/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args index a7a107f4b8..2188ff477d 100644 --- a/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args +++ b/tests/qemuxml2argvdata/pv-spinlock-disabled.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,kvm-pv-unhalt=off \ +-cpu qemu64,kvm_pv_unhalt=off \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ diff --git a/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args b/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args index b217034eb0..a4f1eaedbc 100644 --- a/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args +++ b/tests/qemuxml2argvdata/pv-spinlock-enabled.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,kvm-pv-unhalt=on \ +-cpu qemu64,kvm_pv_unhalt=on \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \
Jirka

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 :|

On Thu, Oct 07, 2021 at 11:09:19 +0100, Daniel P. Berrangé wrote:
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.
Well, only when QEMU_CAPS_CANONICAL_CPU_FEATURES is set. If it is always set for any QEMU we deal with, we can just drop it too, though. BTW the same applies to VIR_CPU_x86_KVM_PV_UNHALT just a few lines below and I can't guarantee I spotted all cases :-) As I said, keeping qemuBuildCpuFeature would be easier for reviewers as chances for making a mistake would be significantly lower... Jirka

On Thu, Oct 07, 2021 at 12:36:14PM +0200, Jiri Denemark wrote:
On Thu, Oct 07, 2021 at 11:09:19 +0100, Daniel P. Berrangé wrote:
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.
Well, only when QEMU_CAPS_CANONICAL_CPU_FEATURES is set. If it is always set for any QEMU we deal with, we can just drop it too, though.
IIUC, this CAPS is set based on whether QEMU_CAPS_CPU_UNAVAILABLE_FEATURES is set. QAPI docs for @unavailable-features says @unavailable-features: List of properties that prevent the CPU model from running in the current host. (since 2.8) so feels like we can drop these caps, since we require QEMU >= 2.11 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 :|

Some CPU features are still added using implicit syntax "feature" which is a deprecated shorthand for "feature=on". Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_command.c | 6 +++--- tests/qemuxml2argvdata/clock-timer-hyperv-rtc.args | 2 +- .../hyperv-stimer-direct.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/hyperv.x86_64-4.0.0.args | 2 +- tests/qemuxml2argvdata/hyperv.x86_64-latest.args | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0f1cdd9372..83d278fd02 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6387,7 +6387,7 @@ qemuBuildCpuCommandLine(virCommand *cmd, break; case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: if (timer->present == 1) - virBufferAddLit(&buf, ",hv-time"); + virBufferAddLit(&buf, ",hv-time=on"); break; case VIR_DOMAIN_TIMER_NAME_TSC: if (timer->frequency > 0) @@ -6451,12 +6451,12 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_HYPERV_IPI: case VIR_DOMAIN_HYPERV_EVMCS: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) - virBufferAsprintf(&buf, ",%s%s", + virBufferAsprintf(&buf, ",%s%s=on", hvPrefix, virDomainHypervTypeToString(i)); if ((i == VIR_DOMAIN_HYPERV_STIMER) && (def->hyperv_stimer_direct == VIR_TRISTATE_SWITCH_ON)) - virBufferAsprintf(&buf, ",%s", VIR_CPU_x86_HV_STIMER_DIRECT); + virBufferAsprintf(&buf, ",%s=on", VIR_CPU_x86_HV_STIMER_DIRECT); break; case VIR_DOMAIN_HYPERV_SPINLOCKS: diff --git a/tests/qemuxml2argvdata/clock-timer-hyperv-rtc.args b/tests/qemuxml2argvdata/clock-timer-hyperv-rtc.args index 0f1707fd7b..11ba1eb1e8 100644 --- a/tests/qemuxml2argvdata/clock-timer-hyperv-rtc.args +++ b/tests/qemuxml2argvdata/clock-timer-hyperv-rtc.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu qemu32,hv-time \ +-cpu qemu32,hv-time=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/hyperv-stimer-direct.x86_64-latest.args b/tests/qemuxml2argvdata/hyperv-stimer-direct.x86_64-latest.args index a05c009389..5f8041c0fe 100644 --- a/tests/qemuxml2argvdata/hyperv-stimer-direct.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hyperv-stimer-direct.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu qemu64,hv-vpindex,hv-synic,hv-stimer,hv-stimer-direct \ +-cpu qemu64,hv-vpindex=on,hv-synic=on,hv-stimer=on,hv-stimer-direct=on \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ diff --git a/tests/qemuxml2argvdata/hyperv.x86_64-4.0.0.args b/tests/qemuxml2argvdata/hyperv.x86_64-4.0.0.args index 100223b53a..82a2aa3d5b 100644 --- a/tests/qemuxml2argvdata/hyperv.x86_64-4.0.0.args +++ b/tests/qemuxml2argvdata/hyperv.x86_64-4.0.0.args @@ -12,7 +12,7 @@ QEMU_AUDIO_DRV=none \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc-i440fx-4.0,accel=tcg,usb=off,dump-guest-core=off \ --cpu 'qemu32,hv_relaxed,hv_vapic,hv-spinlocks=0x2fff,hv_vpindex,hv_runtime,hv_synic,hv_stimer,hv_reset,hv-vendor-id=KVM Hv,hv_frequencies,hv_reenlightenment,hv_tlbflush,hv_ipi,hv_evmcs' \ +-cpu 'qemu32,hv_relaxed=on,hv_vapic=on,hv-spinlocks=0x2fff,hv_vpindex=on,hv_runtime=on,hv_synic=on,hv_stimer=on,hv_reset=on,hv-vendor-id=KVM Hv,hv_frequencies=on,hv_reenlightenment=on,hv_tlbflush=on,hv_ipi=on,hv_evmcs=on' \ -m 214 \ -overcommit mem-lock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/hyperv.x86_64-latest.args b/tests/qemuxml2argvdata/hyperv.x86_64-latest.args index 32ee369ff3..9b70980f3a 100644 --- a/tests/qemuxml2argvdata/hyperv.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hyperv.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-backend=pc.ram \ --cpu 'qemu64,hv-relaxed,hv-vapic,hv-spinlocks=0x2fff,hv-vpindex,hv-runtime,hv-synic,hv-stimer,hv-reset,hv-vendor-id=KVM Hv,hv-frequencies,hv-reenlightenment,hv-tlbflush,hv-ipi,hv-evmcs' \ +-cpu 'qemu64,hv-relaxed=on,hv-vapic=on,hv-spinlocks=0x2fff,hv-vpindex=on,hv-runtime=on,hv-synic=on,hv-stimer=on,hv-reset=on,hv-vendor-id=KVM Hv,hv-frequencies=on,hv-reenlightenment=on,hv-tlbflush=on,hv-ipi=on,hv-evmcs=on' \ -m 214 \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ -overcommit mem-lock=off \ -- 2.31.1
participants (2)
-
Daniel P. Berrangé
-
Jiri Denemark