With the previous refactorings, there's no real benefit from the
qemuBuildCpuFeature helper method. Only one of the callers really
needs the CPU feature name re-writing logic, the others can just
use the right name directly.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/qemu/qemu_command.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 92125dbc85..f24c8842aa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6292,18 +6292,6 @@ qemuBuildGlobalControllerCommandLine(virCommand *cmd,
 }
 
 
-static void
-qemuBuildCpuFeature(virQEMUCaps *qemuCaps,
-                    virBuffer *buf,
-                    const char *name,
-                    bool state)
-{
-    name = virQEMUCapsCPUFeatureToQEMU(qemuCaps, name);
-
-    virBufferAsprintf(buf, ",%s=%s", name, state ? "on" : "off");
-}
-
-
 static int
 qemuBuildCpuModelArgStr(virQEMUDriver *driver,
                         const virDomainDef *def,
@@ -6376,15 +6364,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:
@@ -6441,8 +6431,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:
@@ -6480,13 +6470,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, ",kvm-pv-unhalt=%s",
+                          def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] ==
+                          VIR_TRISTATE_SWITCH_ON ? "on" : "off");
     }
 
     if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
-- 
2.31.1

On Fri, Oct 08, 2021 at 10:01:43 +0100, Daniel P. Berrangé wrote:
> With the previous refactorings, there's no real benefit from the
> qemuBuildCpuFeature helper method. Only one of the callers really
> needs the CPU feature name re-writing logic, the others can just
> use the right name directly.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/qemu/qemu_command.c | 31 +++++++++++--------------------
>  1 file changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 92125dbc85..f24c8842aa 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6292,18 +6292,6 @@ qemuBuildGlobalControllerCommandLine(virCommand *cmd,
>  }
>  
>  
> -static void
> -qemuBuildCpuFeature(virQEMUCaps *qemuCaps,
> -                    virBuffer *buf,
> -                    const char *name,
> -                    bool state)
> -{
> -    name = virQEMUCapsCPUFeatureToQEMU(qemuCaps, name);
> -
> -    virBufferAsprintf(buf, ",%s=%s", name, state ? "on" : "off");
> -}
> -
> -
>  static int
>  qemuBuildCpuModelArgStr(virQEMUDriver *driver,
>                          const virDomainDef *def,
> @@ -6376,15 +6364,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);

That's not really the kind of formatting we use in libvirt. This looks
more like go :-)

Either
           const char *featname = virQEMUCapsCPUFeatureToQEMU(qemuCaps,
                                                              cpu->features[i].name);

or just put the all on a single line.

Anyway, nicely separated patch which makes it obvious removing the
function did not have unexpected side effects.

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>