Blob Blame History Raw
From 038e97c940dd3eba1faf114b3559f32129110f72 Mon Sep 17 00:00:00 2001
Message-Id: <038e97c940dd3eba1faf114b3559f32129110f72@dist-git>
From: =?UTF-8?q?J=C3=A1n=20Tomko?= <jtomko@redhat.com>
Date: Fri, 5 Sep 2014 14:41:25 +0200
Subject: [PATCH] Don't include non-migratable features in host-model

Commit fba6bc4 introduced support for the 'invtsc' feature,
which blocks migration. We should not include it in the
host-model CPU by default, because it's intended to be used
with migration.

https://bugzilla.redhat.com/show_bug.cgi?id=1138221
(cherry picked from commit de0aeafe9ce3eb414c8b5d3aa8995d776a2952de)

Conflicts:
	src/cpu/cpu_x86.c - braces cleanup

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/cpu/cpu_map.xml                              |  2 +-
 src/cpu/cpu_x86.c                                | 69 +++++++++++++++++++++---
 tests/cputest.c                                  |  1 +
 tests/cputestdata/x86-host-invtsc+host-model.xml | 22 ++++++++
 tests/cputestdata/x86-host-invtsc.xml            | 27 ++++++++++
 5 files changed, 113 insertions(+), 8 deletions(-)
 create mode 100644 tests/cputestdata/x86-host-invtsc+host-model.xml
 create mode 100644 tests/cputestdata/x86-host-invtsc.xml

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 12987a0..18c7b0d 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -328,7 +328,7 @@
     </feature>
 
     <!-- Advanced Power Management edx features -->
-    <feature name='invtsc'>
+    <feature name='invtsc' migratable='no'>
       <cpuid function='0x80000007' edx='0x00000100'/>
     </feature>
 
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index b460e8d..affc882 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -89,6 +89,7 @@ struct x86_map {
     struct x86_vendor *vendors;
     struct x86_feature *features;
     struct x86_model *models;
+    struct x86_feature *migrate_blockers;
 };
 
 static struct x86_map* virCPUx86Map = NULL;
@@ -592,6 +593,28 @@ x86FeatureFree(struct x86_feature *feature)
 
 
 static struct x86_feature *
+x86FeatureCopy(const struct x86_feature *src)
+{
+    struct x86_feature *feature;
+
+    if (VIR_ALLOC(feature) < 0)
+        return NULL;
+
+    if (VIR_STRDUP(feature->name, src->name) < 0)
+        goto error;
+
+    if ((feature->data = x86DataCopy(src->data)) == NULL)
+        goto error;
+
+    return feature;
+
+ error:
+    x86FeatureFree(feature);
+    return NULL;
+}
+
+
+static struct x86_feature *
 x86FeatureFind(const struct x86_map *map,
                const char *name)
 {
@@ -677,6 +700,9 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,
     int ret = 0;
     size_t i;
     int n;
+    char *str = NULL;
+    bool migratable = true;
+    struct x86_feature *migrate_blocker = NULL;
 
     if (!(feature = x86FeatureNew()))
         goto error;
@@ -694,6 +720,10 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,
         goto ignore;
     }
 
+    str = virXPathString("string(@migratable)", ctxt);
+    if (STREQ_NULLABLE(str, "no"))
+        migratable = false;
+
     n = virXPathNodeSet("./cpuid", ctxt, &nodes);
     if (n < 0)
         goto ignore;
@@ -710,9 +740,17 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,
             goto error;
     }
 
-    if (map->features == NULL)
+    if (!migratable) {
+        if ((migrate_blocker = x86FeatureCopy(feature)) == NULL)
+            goto error;
+
+        migrate_blocker->next = map->migrate_blockers;
+        map->migrate_blockers = migrate_blocker;
+    }
+
+    if (map->features == NULL) {
         map->features = feature;
-    else {
+    } else {
         feature->next = map->features;
         map->features = feature;
     }
@@ -720,6 +758,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,
  out:
     ctxt->node = ctxt_node;
     VIR_FREE(nodes);
+    VIR_FREE(str);
 
     return ret;
 
@@ -728,6 +767,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,
 
  ignore:
     x86FeatureFree(feature);
+    x86FeatureFree(migrate_blocker);
     goto out;
 }
 
@@ -1093,6 +1133,12 @@ x86MapFree(struct x86_map *map)
         x86VendorFree(vendor);
     }
 
+    while (map->migrate_blockers != NULL) {
+        struct x86_feature *migrate_blocker = map->migrate_blockers;
+        map->migrate_blockers = migrate_blocker->next;
+        x86FeatureFree(migrate_blocker);
+    }
+
     VIR_FREE(map);
 }
 
@@ -2025,16 +2071,15 @@ x86UpdateHostModel(virCPUDefPtr guest,
                    const virCPUDef *host)
 {
     virCPUDefPtr oldguest = NULL;
+    const struct x86_map *map;
+    const struct x86_feature *feat;
     size_t i;
     int ret = -1;
 
     guest->match = VIR_CPU_MATCH_EXACT;
 
-    /* no updates are required */
-    if (guest->nfeatures == 0) {
-        virCPUDefFreeModel(guest);
-        return virCPUDefCopyModel(guest, host, true);
-    }
+    if (!(map = virCPUx86GetMap()))
+        goto cleanup;
 
     /* update the host model according to the desired configuration */
     if (!(oldguest = virCPUDefCopy(guest)))
@@ -2044,6 +2089,16 @@ x86UpdateHostModel(virCPUDefPtr guest,
     if (virCPUDefCopyModel(guest, host, true) < 0)
         goto cleanup;
 
+    /* Remove non-migratable features by default
+     * Note: this only works as long as no CPU model contains non-migratable
+     * features directly */
+    for (i = 0; i < guest->nfeatures; i++) {
+        for (feat = map->migrate_blockers; feat; feat = feat->next) {
+            if (STREQ(feat->name, guest->features[i].name))
+                VIR_DELETE_ELEMENT_INPLACE(guest->features, i, guest->nfeatures);
+        }
+    }
+
     for (i = 0; i < oldguest->nfeatures; i++) {
         if (virCPUDefUpdateFeature(guest,
                                    oldguest->features[i].name,
diff --git a/tests/cputest.c b/tests/cputest.c
index 38cd71e..0ddf82a 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -599,6 +599,7 @@ mymain(void)
     DO_TEST_UPDATE("x86", "host", "host-model", VIR_CPU_COMPARE_IDENTICAL);
     DO_TEST_UPDATE("x86", "host", "host-model-nofallback", VIR_CPU_COMPARE_IDENTICAL);
     DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL);
+    DO_TEST_UPDATE("x86", "host-invtsc", "host-model", VIR_CPU_COMPARE_SUPERSET);
 
     /* computing baseline CPUs */
     DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1);
diff --git a/tests/cputestdata/x86-host-invtsc+host-model.xml b/tests/cputestdata/x86-host-invtsc+host-model.xml
new file mode 100644
index 0000000..ad1bbf8
--- /dev/null
+++ b/tests/cputestdata/x86-host-invtsc+host-model.xml
@@ -0,0 +1,22 @@
+<cpu mode='host-model' match='exact'>
+  <model fallback='allow'>SandyBridge</model>
+  <vendor>Intel</vendor>
+  <feature policy='require' name='osxsave'/>
+  <feature policy='require' name='pcid'/>
+  <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'/>
+</cpu>
diff --git a/tests/cputestdata/x86-host-invtsc.xml b/tests/cputestdata/x86-host-invtsc.xml
new file mode 100644
index 0000000..f558399
--- /dev/null
+++ b/tests/cputestdata/x86-host-invtsc.xml
@@ -0,0 +1,27 @@
+<cpu>
+  <arch>x86_64</arch>
+  <model>SandyBridge</model>
+  <vendor>Intel</vendor>
+  <topology sockets='1' cores='2' threads='2'/>
+  <feature name='invtsc'/>
+  <feature name='osxsave'/>
+  <feature name='pcid'/>
+  <feature name='pdcm'/>
+  <feature name='xtpr'/>
+  <feature name='tm2'/>
+  <feature name='est'/>
+  <feature name='smx'/>
+  <feature name='vmx'/>
+  <feature name='ds_cpl'/>
+  <feature name='monitor'/>
+  <feature name='dtes64'/>
+  <feature name='pbe'/>
+  <feature name='tm'/>
+  <feature name='ht'/>
+  <feature name='ss'/>
+  <feature name='acpi'/>
+  <feature name='ds'/>
+  <feature name='vme'/>
+  <pages unit='KiB' size='4'/>
+  <pages unit='KiB' size='2048'/>
+</cpu>
-- 
2.1.0