qemu: Allow enabling/disabling features with host-passthrough

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@redhat.com>

docs/formatdomain.html.in

@@ -982,6 +982,7 @@
 
 <pre>
   &lt;cpu mode='host-passthrough'/&gt;
+    &lt;feature policy='disable' name='lahf_lm'/&gt;
   ...</pre>
 
     <p>
@@ -1083,8 +1084,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>

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>
 

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 (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);
+            if (!feature->name) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("Missing CPU feature name"));
                 return -1;
             }
-            virBufferAsprintf(buf, "<feature policy='%s' name='%s'/>\n",
-                              policy, feature->name);
-        } else {
-            virBufferAsprintf(buf, "<feature name='%s'/>\n",
-                              feature->name);
+
+            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;
+                }
+                virBufferAsprintf(buf, "<feature policy='%s' name='%s'/>\n",
+                                  policy, feature->name);
+            } else {
+                virBufferAsprintf(buf, "<feature name='%s'/>\n",
+                                  feature->name);
+            }
         }
     }
 

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);
         }
     }
 

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 \

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>

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);

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");