[libvirt] [PATCH] qemu: Allow enabling/disabling features with host-passthrough

Martin Kletzander mkletzan at redhat.com
Mon Jan 12 13:21:35 UTC 2015


QEMU supports feature specification with -cpu host and we just skip
using that.  Since QEMU developers themselves would like to use this
feature, this patch modifies the code to work.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1178850

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
 docs/formatdomain.html.in                          |  5 +-
 docs/schemas/domaincommon.rng                      | 67 ++++++++--------------
 src/conf/cpu_conf.c                                | 53 +++++++++--------
 src/qemu/qemu_command.c                            | 21 ++++---
 ...emuxml2argv-cpu-host-passthrough-features.args} |  2 +-
 ...qemuxml2argv-cpu-host-passthrough-features.xml} | 22 +------
 tests/qemuxml2argvtest.c                           |  2 +-
 tests/qemuxml2xmltest.c                            |  1 +
 8 files changed, 71 insertions(+), 102 deletions(-)
 rename tests/qemuxml2argvdata/{qemuxml2argv-cpu-host-passthrough-features-invalid.args => qemuxml2argv-cpu-host-passthrough-features.args} (91%)
 rename tests/qemuxml2argvdata/{qemuxml2argv-cpu-host-passthrough-features-invalid.xml => qemuxml2argv-cpu-host-passthrough-features.xml} (57%)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 499879e..21a557e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -970,6 +970,7 @@

 <pre>
   <cpu mode='host-passthrough'/>
+    <feature policy='disable' name='lahf_lm'/>
   ...</pre>

     <p>
@@ -1071,8 +1072,8 @@
           the same as the host CPU even in the aspects that libvirt does not
           understand. Though the downside of this mode is that the guest
           environment cannot be reproduced on different hardware. Thus, if you
-          hit any bugs, you are on your own. Neither <code>model</code> nor
-          <code>feature</code> elements are allowed in this mode.</dd>
+          hit any bugs, you are on your own. Further details of that CPU can
+          be changed using <code>feature</code> elements.</dd>
         </dl>

         In both <code>host-model</code> and <code>host-passthrough</code>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 879e064..85b3709 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4088,50 +4088,29 @@
       -->
   <define name="cpu">
     <element name="cpu">
-      <choice>
-        <group>
-          <interleave>
-            <optional>
-              <ref name="cpuTopology"/>
-            </optional>
-            <optional>
-              <ref name="cpuNuma"/>
-            </optional>
-          </interleave>
-        </group>
-        <group>
-          <ref name="cpuMode"/>
-          <interleave>
-            <optional>
-              <ref name="cpuModel"/>
-            </optional>
-            <optional>
-              <ref name="cpuNuma"/>
-            </optional>
-          </interleave>
-        </group>
-        <group>
-          <optional>
-            <ref name="cpuMode"/>
-          </optional>
-          <ref name="cpuMatch"/>
-          <interleave>
-            <ref name="cpuModel"/>
-            <optional>
-              <ref name="cpuVendor"/>
-            </optional>
-            <optional>
-              <ref name="cpuTopology"/>
-            </optional>
-            <zeroOrMore>
-              <ref name="cpuFeature"/>
-            </zeroOrMore>
-            <optional>
-              <ref name="cpuNuma"/>
-            </optional>
-          </interleave>
-        </group>
-      </choice>
+      <optional>
+        <ref name="cpuMode"/>
+      </optional>
+      <optional>
+        <ref name="cpuMatch"/>
+      </optional>
+      <interleave>
+        <optional>
+          <ref name="cpuModel"/>
+        </optional>
+        <optional>
+          <ref name="cpuVendor"/>
+        </optional>
+        <optional>
+          <ref name="cpuTopology"/>
+        </optional>
+        <zeroOrMore>
+          <ref name="cpuFeature"/>
+        </zeroOrMore>
+        <optional>
+          <ref name="cpuNuma"/>
+        </optional>
+      </interleave>
     </element>
   </define>

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 3af0f44..31fb458 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -1,7 +1,7 @@
 /*
  * cpu_conf.c: CPU XML handling
  *
- * Copyright (C) 2009-2014 Red Hat, Inc.
+ * Copyright (C) 2009-2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -366,12 +366,8 @@ virCPUDefParseXML(xmlNodePtr node,
         goto error;

     if (n > 0) {
-        if (!def->model && def->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
-            /* silently ignore incorrectly formatted features generated
-             * by older libvirt */
-            goto cleanup;
-        }
-        if (!def->model && def->mode != VIR_CPU_MODE_HOST_MODEL) {
+        if (!def->model && def->mode != VIR_CPU_MODE_HOST_MODEL &&
+            def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Non-empty feature list specified without "
                              "CPU model"));
@@ -623,6 +619,7 @@ virCPUDefFormatBuf(virBufferPtr buf,

     if (!def->model &&
         def->mode != VIR_CPU_MODE_HOST_MODEL &&
+        def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
         def->nfeatures) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Non-empty feature list specified without CPU model"));
@@ -663,30 +660,32 @@ virCPUDefFormatBuf(virBufferPtr buf,
         virBufferAddLit(buf, "/>\n");
     }

-    for (i = 0; formatModel && i < def->nfeatures; i++) {
-        virCPUFeatureDefPtr feature = def->features + i;
+    if (formatModel || def->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
+        for (i = 0; i < def->nfeatures; i++) {
+            virCPUFeatureDefPtr feature = def->features + i;

-        if (!feature->name) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Missing CPU feature name"));
-            return -1;
-        }
+            if (!feature->name) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("Missing CPU feature name"));
+                return -1;
+            }

-        if (def->type == VIR_CPU_TYPE_GUEST) {
-            const char *policy;
+            if (def->type == VIR_CPU_TYPE_GUEST) {
+                const char *policy;

-            policy = virCPUFeaturePolicyTypeToString(feature->policy);
-            if (!policy) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Unexpected CPU feature policy %d"),
-                               feature->policy);
-                return -1;
+                policy = virCPUFeaturePolicyTypeToString(feature->policy);
+                if (!policy) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR,
+                                   _("Unexpected CPU feature policy %d"),
+                                   feature->policy);
+                    return -1;
+                }
+                virBufferAsprintf(buf, "<feature policy='%s' name='%s'/>\n",
+                                  policy, feature->name);
+            } else {
+                virBufferAsprintf(buf, "<feature name='%s'/>\n",
+                                  feature->name);
             }
-            virBufferAsprintf(buf, "<feature policy='%s' name='%s'/>\n",
-                              policy, feature->name);
-        } else {
-            virBufferAsprintf(buf, "<feature name='%s'/>\n",
-                              feature->name);
         }
     }

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1930abd..83833ab 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6115,6 +6115,7 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
     virCPUDefPtr host = NULL;
     virCPUDefPtr guest = NULL;
     virCPUDefPtr cpu = NULL;
+    virCPUDefPtr featCpu = NULL;
     size_t ncpus = 0;
     char **cpus = NULL;
     virCPUDataPtr data = NULL;
@@ -6122,8 +6123,9 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
     virCPUCompareResult cmp;
     const char *preferred;
     virCapsPtr caps = NULL;
-    bool compareAgainstHost = (def->virtType == VIR_DOMAIN_VIRT_KVM ||
-        def->cpu->mode != VIR_CPU_MODE_CUSTOM);
+    bool compareAgainstHost = ((def->virtType == VIR_DOMAIN_VIRT_KVM ||
+                                def->cpu->mode != VIR_CPU_MODE_CUSTOM) &&
+                               def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH);

     if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
         goto cleanup;
@@ -6141,7 +6143,7 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
     if (!(cpu = virCPUDefCopy(def->cpu)))
         goto cleanup;

-    if (cpu->mode != VIR_CPU_MODE_CUSTOM &&
+    if (cpu->mode == VIR_CPU_MODE_HOST_MODEL &&
         !migrating &&
         cpuUpdate(cpu, host) < 0)
         goto cleanup;
@@ -6200,6 +6202,8 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
         if (ARCH_IS_PPC64(def->os.arch) &&
             cpu->mode == VIR_CPU_MODE_HOST_MODEL) {
             virBufferAsprintf(buf, ",compat=%s", def->cpu->model);
+        } else {
+            featCpu = cpu;
         }

     } else {
@@ -6225,18 +6229,21 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
             if (VIR_STRDUP(guest->model, cpu->model) < 0)
                 goto cleanup;
         }
-
         virBufferAdd(buf, guest->model, -1);
         if (guest->vendor_id)
             virBufferAsprintf(buf, ",vendor=%s", guest->vendor_id);
-        for (i = 0; i < guest->nfeatures; i++) {
+        featCpu = guest;
+    }
+
+    if (featCpu) {
+        for (i = 0; i < featCpu->nfeatures; i++) {
             char sign;
-            if (guest->features[i].policy == VIR_CPU_FEATURE_DISABLE)
+            if (featCpu->features[i].policy == VIR_CPU_FEATURE_DISABLE)
                 sign = '-';
             else
                 sign = '+';

-            virBufferAsprintf(buf, ",%c%s", sign, guest->features[i].name);
+            virBufferAsprintf(buf, ",%c%s", sign, featCpu->features[i].name);
         }
     }

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features.args
similarity index 91%
rename from tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.args
rename to tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features.args
index 18c1dce..88a25bf 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features.args
@@ -8,7 +8,7 @@ IO_DRV=none \
 /usr/bin/qemu \
 -S \
 -M pc \
--cpu host \
+-cpu host,+abm,+ds,-invtsc \
 -m 214 \
 -smp 1 \
 -nographic \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features.xml
similarity index 57%
rename from tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.xml
rename to tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features.xml
index b5f5326..e807dbd 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features-invalid.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-passthrough-features.xml
@@ -15,26 +15,8 @@
   </os>
   <cpu mode='host-passthrough'>
     <feature policy='require' name='abm'/>
-    <feature policy='require' name='pdpe1gb'/>
-    <feature policy='require' name='rdrand'/>
-    <feature policy='require' name='f16c'/>
-    <feature policy='require' name='osxsave'/>
-    <feature policy='require' name='pdcm'/>
-    <feature policy='require' name='xtpr'/>
-    <feature policy='require' name='tm2'/>
-    <feature policy='require' name='est'/>
-    <feature policy='require' name='smx'/>
-    <feature policy='require' name='vmx'/>
-    <feature policy='require' name='ds_cpl'/>
-    <feature policy='require' name='monitor'/>
-    <feature policy='require' name='dtes64'/>
-    <feature policy='require' name='pbe'/>
-    <feature policy='require' name='tm'/>
-    <feature policy='require' name='ht'/>
-    <feature policy='require' name='ss'/>
-    <feature policy='require' name='acpi'/>
-    <feature policy='require' name='ds'/>
-    <feature policy='require' name='vme'/>
+    <feature policy='force' name='ds'/>
+    <feature policy='disable' name='invtsc'/>
   </cpu>
   <clock offset='utc'/>
   <on_poweroff>destroy</on_poweroff>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index bce88a8..1d0bd61 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1524,7 +1524,7 @@ mymain(void)
     DO_TEST_FAILURE("shmem-small-size", QEMU_CAPS_PCIDEVICE,
                     QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM);
     DO_TEST_PARSE_ERROR("shmem-msi-only", NONE);
-    DO_TEST("cpu-host-passthrough-features-invalid", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST);
+    DO_TEST("cpu-host-passthrough-features", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST);

     virObjectUnref(driver.config);
     virObjectUnref(driver.caps);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e1ec514..1166534 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -184,6 +184,7 @@ mymain(void)
     DO_TEST("clock-localtime");
     DO_TEST("cpu-kvmclock");
     DO_TEST("cpu-host-kvmclock");
+    DO_TEST("cpu-host-passthrough-features");
     DO_TEST("clock-catchup");
     DO_TEST("kvmclock");
     DO_TEST("clock-timer-hyperv-rtc");
-- 
2.2.1




More information about the libvir-list mailing list