[libvirt] [PATCHv2] 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 --- 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