[2/3] qemu: command: rework adding of default cpu model

Message ID b6c210ac5c8fd8e70410cab15320d4d98c1c81cb.1500075702.git.crobinso@redhat.com
State New
Headers show
Series
  • qemu: command: clean up default cpu formatting
Related show

Commit Message

Cole Robinson July 14, 2017, 11:43 p.m.
Certain XML features that aren't in the <cpu> block map to -cpu
flags on the qemu cli. If one of these is specified but the user
didn't explicitly pass an XML <cpu> model, we need to format a
default model on the command line.

The current code handles this by sprinkling this default cpu handling
among all the different flag string formatting. Instead, switch it
to do this just once.

This alters some test output slightly: the previous code would
write the default -cpu in some cases when no flags were actually
added, so the output was redundant.

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 src/qemu/qemu_command.c                            | 72 +++++++---------------
 .../qemuxml2argvdata/qemuxml2argv-hyperv-off.args  |  1 -
 .../qemuxml2argv-kvm-features-off.args             |  1 -
 3 files changed, 22 insertions(+), 52 deletions(-)

-- 
2.13.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Ján Tomko July 21, 2017, 2:25 p.m. | #1
On Fri, Jul 14, 2017 at 07:43:05PM -0400, Cole Robinson wrote:
>Certain XML features that aren't in the <cpu> block map to -cpu

>flags on the qemu cli. If one of these is specified but the user

>didn't explicitly pass an XML <cpu> model, we need to format a

>default model on the command line.

>

>The current code handles this by sprinkling this default cpu handling

>among all the different flag string formatting. Instead, switch it

>to do this just once.

>

>This alters some test output slightly: the previous code would

>write the default -cpu in some cases when no flags were actually

>added, so the output was redundant.

>

>Signed-off-by: Cole Robinson <crobinso@redhat.com>

>---

> src/qemu/qemu_command.c                            | 72 +++++++---------------

> .../qemuxml2argvdata/qemuxml2argv-hyperv-off.args  |  1 -

> .../qemuxml2argv-kvm-features-off.args             |  1 -

> 3 files changed, 22 insertions(+), 52 deletions(-)

>


ACK

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch hide | download patch | download mbox

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b83261246..aa12479f7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6714,11 +6714,11 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
                         virQEMUCapsPtr qemuCaps)
 {
     virArch hostarch = virArchFromHost();
-    char *cpu = NULL;
+    char *cpu = NULL, *cpu_flags = NULL;
     bool hasHwVirt = false;
     const char *default_model;
-    bool have_cpu = false;
     int ret = -1;
+    virBuffer cpu_buf = VIR_BUFFER_INITIALIZER;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     size_t i;
 
@@ -6729,9 +6729,8 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
 
     if (def->cpu &&
         (def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) {
-        if (qemuBuildCpuModelArgStr(driver, def, &buf, qemuCaps) < 0)
+        if (qemuBuildCpuModelArgStr(driver, def, &cpu_buf, qemuCaps) < 0)
             goto cleanup;
-        have_cpu = true;
 
         /* Only 'svm' requires --enable-nesting. The nested 'vmx' patches now
          * simply hook off the CPU features. */
@@ -6769,8 +6768,7 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
             ((hostarch == VIR_ARCH_X86_64 &&
               strstr(def->emulator, "kvm")) ||
              strstr(def->emulator, "x86_64"))) {
-            virBufferAdd(&buf, default_model, -1);
-            have_cpu = true;
+            virBufferAdd(&cpu_buf, default_model, -1);
         }
     }
 
@@ -6780,21 +6778,14 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
 
         if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK &&
             timer->present != -1) {
-            virBufferAsprintf(&buf, "%s,%ckvmclock",
-                              have_cpu ? "" : default_model,
+            virBufferAsprintf(&buf, ",%ckvmclock",
                               timer->present ? '+' : '-');
-            have_cpu = true;
         } else if (timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK &&
                    timer->present == 1) {
-            virBufferAsprintf(&buf, "%s,hv_time",
-                              have_cpu ? "" : default_model);
-            have_cpu = true;
+            virBufferAddLit(&buf, ",hv_time");
         } else if (timer->name == VIR_DOMAIN_TIMER_NAME_TSC &&
                    timer->frequency > 0) {
-            virBufferAsprintf(&buf, "%s,tsc-frequency=%lu",
-                              have_cpu ? "" : default_model,
-                              timer->frequency);
-            have_cpu = true;
+            virBufferAsprintf(&buf, ",tsc-frequency=%lu", timer->frequency);
         }
     }
 
@@ -6805,10 +6796,7 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
         else
             sign = '-';
 
-        virBufferAsprintf(&buf, "%s,%ckvm_pv_eoi",
-                          have_cpu ? "" : default_model,
-                          sign);
-        have_cpu = true;
+        virBufferAsprintf(&buf, ",%ckvm_pv_eoi", sign);
     }
 
     if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK]) {
@@ -6819,18 +6807,10 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
         else
             sign = '-';
 
-        virBufferAsprintf(&buf, "%s,%ckvm_pv_unhalt",
-                          have_cpu ? "" : default_model,
-                          sign);
-        have_cpu = true;
+        virBufferAsprintf(&buf, ",%ckvm_pv_unhalt", sign);
     }
 
     if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
-        if (!have_cpu) {
-            virBufferAdd(&buf, default_model, -1);
-            have_cpu = true;
-        }
-
         for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
             switch ((virDomainHyperv) i) {
             case VIR_DOMAIN_HYPERV_RELAXED:
@@ -6866,22 +6846,12 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
 
     for (i = 0; i < def->npanics; i++) {
         if (def->panics[i]->model == VIR_DOMAIN_PANIC_MODEL_HYPERV) {
-            if (!have_cpu) {
-                virBufferAdd(&buf, default_model, -1);
-                have_cpu = true;
-            }
-
             virBufferAddLit(&buf, ",hv_crash");
             break;
         }
     }
 
     if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
-        if (!have_cpu) {
-            virBufferAdd(&buf, default_model, -1);
-            have_cpu = true;
-        }
-
         for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) {
             switch ((virDomainKVM) i) {
             case VIR_DOMAIN_KVM_HIDDEN:
@@ -6898,12 +6868,8 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
 
     if (def->features[VIR_DOMAIN_FEATURE_PMU]) {
         virTristateSwitch pmu = def->features[VIR_DOMAIN_FEATURE_PMU];
-        if (!have_cpu)
-            virBufferAdd(&buf, default_model, -1);
-
         virBufferAsprintf(&buf, ",pmu=%s",
                           virTristateSwitchTypeToString(pmu));
-        have_cpu = true;
     }
 
     if (def->cpu && def->cpu->cache) {
@@ -6911,11 +6877,6 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
         bool hostOff = false;
         bool l3Off = false;
 
-        if (!have_cpu) {
-            virBufferAdd(&buf, default_model, -1);
-            have_cpu = true;
-        }
-
         switch (cache->mode) {
         case VIR_CPU_CACHE_MODE_EMULATE:
             virBufferAddLit(&buf, ",l3-cache=on");
@@ -6945,13 +6906,22 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
             virBufferAddLit(&buf, ",l3-cache=off");
     }
 
+    if (virBufferCheckError(&cpu_buf) < 0)
+        goto cleanup;
     if (virBufferCheckError(&buf) < 0)
         goto cleanup;
 
-    cpu = virBufferContentAndReset(&buf);
+    cpu = virBufferContentAndReset(&cpu_buf);
+    cpu_flags = virBufferContentAndReset(&buf);
+
+    if (cpu_flags && !cpu) {
+        if (VIR_STRDUP(cpu, default_model) < 0)
+            goto cleanup;
+    }
 
     if (cpu) {
-        virCommandAddArgList(cmd, "-cpu", cpu, NULL);
+        virCommandAddArg(cmd, "-cpu");
+        virCommandAddArgFormat(cmd, "%s%s", cpu, cpu_flags ? cpu_flags : "");
 
         if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NESTING) && hasHwVirt)
             virCommandAddArg(cmd, "-enable-nesting");
@@ -6961,7 +6931,9 @@  qemuBuildCpuCommandLine(virCommandPtr cmd,
 
  cleanup:
     VIR_FREE(cpu);
+    VIR_FREE(cpu_flags);
     virBufferFreeAndReset(&buf);
+    virBufferFreeAndReset(&cpu_buf);
     return ret;
 }
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args
index e708feece..d1718d1f9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.args
@@ -8,7 +8,6 @@  QEMU_AUDIO_DRV=none \
 -name QEMUGuest1 \
 -S \
 -M pc \
--cpu qemu32 \
 -m 214 \
 -smp 6,sockets=6,cores=1,threads=1 \
 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args b/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
index e708feece..d1718d1f9 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-features-off.args
@@ -8,7 +8,6 @@  QEMU_AUDIO_DRV=none \
 -name QEMUGuest1 \
 -S \
 -M pc \
--cpu qemu32 \
 -m 214 \
 -smp 6,sockets=6,cores=1,threads=1 \
 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \