cpu_x86: Make sure removed features are always mentioned in CPU def

For backward compatibility with older versions of libvirt CPU models in
our CPU map are mostly immutable. We only changed them in a few specific
cases after showing it was safe. Sometimes QEMU developers realize a
specific feature should not be part of a particular (or any) CPU model
because it can never be enabled automatically without further
configuration. But we couldn't follow them because doing so would break
migration to older libvirt.

If QEMU drops feature F from CPU model M because F could not be enabled
automatically anyway, asking for M would never enable F. Even with older
QEMU versions. Naively removing F from libvirt's definition of M would
seem to work nicely on a single host. Libvirt would consider M to be
compatible with hosts CPU that do not support F. However, trying to
migrate domains using M without explicitly enabling or disabling F could
fail, because older libvirt would think F was enabled (it is part of M
there), but QEMU reports it as disabled once started.

Thus we can remove such feature from a libvirt's CPU model, but we have
to make sure any CPU definition using the affected model will always
explicitly mention the state of the removed feature.

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 40bf5b68d0f2a713e5fb58f43f66b3dec115c764..2422e258ecee225224a6f3dc830ddbd0db0c7e06 100644 (file)
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -804,11 +804,37 @@ x86DataAddSignature(virCPUx86Data *data,
 }
 
 
+/*
+ * Disables features removed from the CPU @model unless they are already
+ * mentioned in @cpu to make sure these features will always be explicitly
+ * listed in the CPU definition.
+ */
+static int
+virCPUx86DisableRemovedFeatures(virCPUDefPtr cpu,
+                                virCPUx86ModelPtr model)
+{
+    char **feat = model->removedFeatures;
+
+    if (!feat)
+        return 0;
+
+    while (*feat) {
+        if (virCPUDefAddFeatureIfMissing(cpu, *feat, VIR_CPU_FEATURE_DISABLE) < 0)
+            return -1;
+
+        feat++;
+    }
+
+    return 0;
+}
+
+
 static virCPUDefPtr
 x86DataToCPU(const virCPUx86Data *data,
              virCPUx86ModelPtr model,
              virCPUx86MapPtr map,
-             virDomainCapsCPUModelPtr hvModel)
+             virDomainCapsCPUModelPtr hvModel,
+             virCPUType cpuType)
 {
     g_autoptr(virCPUDef) cpu = NULL;
     g_auto(virCPUx86Data) copy = VIR_CPU_X86_DATA_INIT;
@@ -851,6 +877,13 @@ x86DataToCPU(const virCPUx86Data *data,
         x86DataToCPUFeatures(cpu, VIR_CPU_FEATURE_DISABLE, &modelData, map))
         return NULL;
 
+    if (cpuType == VIR_CPU_TYPE_GUEST) {
+        if (virCPUx86DisableRemovedFeatures(cpu, model) < 0)
+            return NULL;
+    }
+
+    cpu->type = cpuType;
+
     return g_steal_pointer(&cpu);
 }
 
@@ -2197,9 +2230,9 @@ x86Decode(virCPUDefPtr cpu,
             continue;
         }
 
-        if (!(cpuCandidate = x86DataToCPU(&data, candidate, map, hvModel)))
+        if (!(cpuCandidate = x86DataToCPU(&data, candidate, map, hvModel,
+                                          cpu->type)))
             return -1;
-        cpuCandidate->type = cpu->type;
 
         if ((rc = x86DecodeUseCandidate(model, cpuModel,
                                         candidate, cpuCandidate,
@@ -2962,6 +2995,7 @@ virCPUx86Update(virCPUDefPtr guest,
                 bool relative)
 {
     g_autoptr(virCPUx86Model) model = NULL;
+    virCPUx86ModelPtr guestModel;
     virCPUx86MapPtr map;
     size_t i;
 
@@ -2998,6 +3032,15 @@ virCPUx86Update(virCPUDefPtr guest,
         }
     }
 
+    if (!(guestModel = x86ModelFind(map, guest->model))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unknown CPU model %s"), guest->model);
+        return -1;
+    }
+
+    if (virCPUx86DisableRemovedFeatures(guest, guestModel) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -3067,6 +3110,9 @@ virCPUx86UpdateLive(virCPUDefPtr cpu,
         }
     }
 
+    if (virCPUx86DisableRemovedFeatures(cpu, model) < 0)
+        return -1;
+
     virBufferTrim(&bufAdded, ",");
     virBufferTrim(&bufRemoved, ",");
 
@@ -3190,6 +3236,7 @@ virCPUx86ExpandFeatures(virCPUDefPtr cpu)
     }
 
     model = x86ModelCopy(model);
+
     if (x86DataToCPUFeatures(expanded, host ? -1 : VIR_CPU_FEATURE_REQUIRE,
                              &model->data, map) < 0)
         return -1;
@@ -3206,6 +3253,11 @@ virCPUx86ExpandFeatures(virCPUDefPtr cpu)
             return -1;
     }
 
+    if (!host) {
+        if (virCPUx86DisableRemovedFeatures(expanded, model) < 0)
+            return -1;
+    }
+
     virCPUDefFreeModel(cpu);
 
     return virCPUDefCopyModel(cpu, expanded, false);