[PATCHv2] Don't include non-migratable features in host-model

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
---
v2: added tests and comment, fixed a typo

 src/cpu/cpu_map.xml                              |  2 +-
 src/cpu/cpu_x86.c                                | 65 ++++++++++++++++++++++--
 tests/cputest.c                                  |  1 +
 tests/cputestdata/x86-host-invtsc+host-model.xml | 22 ++++++++
 tests/cputestdata/x86-host-invtsc.xml            | 27 ++++++++++
 5 files changed, 111 insertions(+), 6 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 af2e08e..7571f16 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,6 +740,14 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,
             goto error;
     }
 
+    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 {
@@ -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 94c0ebc..a507576 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>
-- 
1.8.5.5

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]