From 10be3da4947fc7923732e2056516a41514bd1695 Mon Sep 17 00:00:00 2001

Message-Id: <10be3da4947fc7923732e2056516a41514bd1695@dist-git>

From: =?UTF-8?q?J=C3=A1n=20Tomko?= <jtomko@redhat.com>

Date: Fri, 5 Sep 2014 09:50:36 +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)

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

Conflicts:

	src/cpu/cpu_x86.c - context -- missing {} (fixed by ff78ff7c)

---

 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 aa10019..28bc5ec 100644

--- a/src/cpu/cpu_map.xml

+++ b/src/cpu/cpu_map.xml

@@ -328,7 +328,7 @@

     </feature>

-    <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 4e77550..8ced69d 100644

--- a/src/cpu/cpu_x86.c

+++ b/src/cpu/cpu_x86.c

@@ -87,6 +87,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;

@@ -590,6 +591,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)

 {

@@ -675,6 +698,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;

@@ -692,6 +718,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;

@@ -708,6 +738,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 {

@@ -718,6 +756,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt,

 out:

     ctxt->node = ctxt_node;

     VIR_FREE(nodes);

+    VIR_FREE(str);

     return ret;

@@ -726,6 +765,7 @@ error:

 ignore:

     x86FeatureFree(feature);

+    x86FeatureFree(migrate_blocker);

     goto out;

 }

@@ -1061,6 +1101,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);

 }

@@ -1941,16 +1987,15 @@ x86UpdateHostModel(virCPUDefPtr guest,

                    const virCPUDefPtr 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)))

@@ -1960,6 +2005,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 6a1cfeb..153556a 100644

--- a/tests/cputest.c

+++ b/tests/cputest.c

@@ -591,6 +591,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", -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.2.2