Re: [libvirt PATCH 1/2] qemu: remove use of (+|-)name syntax for -cpu featres
On Wed, Oct 06, 2021 at 08:23:58AM +0200, Peter Krempa wrote:
> On Tue, Oct 05, 2021 at 18:07:03 +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
> > 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
> > 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é
> > ---
>
> [...]
>
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index eaa1e0deb9..a1dba1cb7e 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);
>
> This function, which is no longer called ...
>
>
> > -
> > -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,
> > @@ -6332,12 +6317,12 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver,
> > 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", cpu->features[i].name);
> > break;
> >
> > case VIR_CPU_FEATURE_DISABLE:
> > case VIR_CPU_FEATURE_FORBID:
> > -qemuBuildCpuFeature(qemuCaps, buf, cpu->features[i].name,
> > false);
> > +virBufferAsprintf(buf, ",%s=off", cpu->features[i].name);
>
> ... here ...
>
> > break;
> >
> > case VIR_CPU_FEATURE_OPTIONAL:
>
> [...]
>
> > diff --git a/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> > b/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> > index 3ce594ba13..7f29bcd10d 100644
>
> ... is causing a change even on modern versions ...
>
> > --- a/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> > +++ b/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> > @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> > -S \
> > -object
> > secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes
> > \
> > -machine pc-q35-5.0,accel=kvm,usb=off,dump-guest-core=off \
> > --cpu
> > Skylake-Client-IBRS,ss=on,vmx=on,hypervisor=on,tsc-adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,skip-l1dfl-vmentry=on,pschange-mc-no=on
> > \
> > +-cpu
> > Skylake-Client-IBRS,ss=on,vmx=on,hypervisor=on,tsc_adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,skip-l1dfl-vmentry=on,pschange-mc-no=on
> > \
>
> ... which should not have been impacted. Specifically the changed thing
> it the above commandline is
> (it's rather hard to spot):
>
> tsc-adjust -> tsc_adjust
Doh, yes, very subtle.
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 :|
Re: [libvirt PATCH 1/2] qemu: remove use of (+|-)name syntax for -cpu featres
On Tue, Oct 05, 2021 at 18:07:03 +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
> 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
> 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é
> ---
[...]
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index eaa1e0deb9..a1dba1cb7e 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);
This function, which is no longer called ...
> -
> -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,
> @@ -6332,12 +6317,12 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver,
> 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", cpu->features[i].name);
> break;
>
> case VIR_CPU_FEATURE_DISABLE:
> case VIR_CPU_FEATURE_FORBID:
> -qemuBuildCpuFeature(qemuCaps, buf, cpu->features[i].name, false);
> +virBufferAsprintf(buf, ",%s=off", cpu->features[i].name);
... here ...
> break;
>
> case VIR_CPU_FEATURE_OPTIONAL:
[...]
> diff --git a/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> b/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> index 3ce594ba13..7f29bcd10d 100644
... is causing a change even on modern versions ...
> --- a/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> +++ b/tests/qemuxml2argvdata/cpu-host-model.x86_64-5.0.0.args
> @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> -S \
> -object
> secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes
> \
> -machine pc-q35-5.0,accel=kvm,usb=off,dump-guest-core=off \
> --cpu
> Skylake-Client-IBRS,ss=on,vmx=on,hypervisor=on,tsc-adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,skip-l1dfl-vmentry=on,pschange-mc-no=on
> \
> +-cpu
> Skylake-Client-IBRS,ss=on,vmx=on,hypervisor=on,tsc_adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,skip-l1dfl-vmentry=on,pschange-mc-no=on
> \
... which should not have been impacted. Specifically the changed thing
it the above commandline is
(it's rather hard to spot):
tsc-adjust -> tsc_adjust
[libvirt PATCH 1/2] qemu: remove use of (+|-)name syntax for -cpu featres
The -cpu arg gained support for feature=on|off syntax for the x86
emulator in 2.4.0
commit 38e5c119c2925812bd441450ab9e5e00fc79e662
Author: Eduardo Habkost
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
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é
---
src/qemu/qemu_command.c | 32 ++-
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-model.x86_64-4.1.0.args | 2 +-
.../cpu-host-model.x86_64-4.2.0.args | 2 +-
.../cpu-host-model.x86_64-5.0.0.args | 2 +-
.../cpu-host-model.x86_64-5.1.0.args | 2 +-
.../cpu-host-model.x86_64-5.2.0.args | 2 +-
.../cpu-host-model.x86_64-6.0.0.args | 2 +-
.../cpu-host-model.x86_64-6.1.0.args | 2 +-
.../cpu-host-model.x86_64-latest.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 +-
39 files changed, 47 insertions(+), 61 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index eaa1e0deb9..a1dba1cb7e 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,
@@ -6332,12 +6317,12 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver,
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", cpu->features[i].name);
break;
case VIR_CPU_FEATURE_DISABLE:
case VIR_CPU_FEATURE_FORBID:
-qemuBuildCpuFeature(qemuCaps, buf, cpu->features[i].name, false);
+virBufferAsprintf(buf, ",%s=off", cpu->features[i].name);
break;
case VIR_CPU_FEATURE_OPTIONAL:
@@ -6394,8 +6379,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;
