[libvirt PATCH 1/2] qemu: remove use of (+|-)name syntax for -cpu featres

[Daniel P. Berrangé]

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 <ehabkost at 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 at 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 at redhat.com>
> > ---
> 
> [...]
> 
> > 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 :|