qemu: Fix migration with custom XML

Ages ago origCPU in domain private data was introduced to provide
backward compatibility when migrating to an old libvirt, which did not
support fetching updated CPU definition from QEMU. Thus origCPU will
contain the original CPU definition before such update. But only if the
update actually changed anything. Let's always fill origCPU with the
original definition when starting a domain so that we can rely on it
being always set, even if it matches the updated definition.

This fixes migration or save operations with custom domain XML after
commit v10.1.0-88-g14d3517410, which expected origCPU to be always set
to the CPU definition from inactive XML to check features explicitly
requested by a user.

https://issues.redhat.com/browse/RHEL-30622

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Tested-by: Han Han <hhan@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6b869797a88063e1ac5af8e534bbf06ccae7752e..d91c7d478b8582cae17f18570ca6b1b4466d85fa 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10995,12 +10995,13 @@ virSaveCookieCallbacks virQEMUDriverDomainSaveCookie = {
  * @vm: domain which is being started
  * @cpu: CPU updated when the domain was running previously (before migration,
  *       snapshot, or save)
- * @origCPU: where to store the original CPU from vm->def in case @cpu was
- *           used instead
+ * @origCPU: where to store the original CPU from vm->def
  *
- * Replace the CPU definition with the updated one when QEMU is new enough to
- * allow us to check extra features it is about to enable or disable when
- * starting a domain. The original CPU is stored in @origCPU.
+ * Save the original CPU definition from inactive XML in @origCPU so that we
+ * can safely update it and still be able to check what exactly a user asked
+ * for. The domain definition will either contain a copy of the original CPU
+ * definition or a copy of @cpu in case the domain was already running and
+ * we're just restoring a saved state or preparing for incoming migration.
  *
  * Returns 0 on success, -1 on error.
  */
@@ -11013,18 +11014,24 @@ qemuDomainUpdateCPU(virDomainObj *vm,
 
     *origCPU = NULL;
 
-    if (!cpu || !vm->def->cpu ||
-        !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION) ||
-        virCPUDefIsEqual(vm->def->cpu, cpu, false))
+    if (!vm->def->cpu)
         return 0;
 
-    if (!(cpu = virCPUDefCopy(cpu)))
-        return -1;
+    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
+        return 0;
+
+    /* nothing to do if only topology part of CPU def is used */
+    if (vm->def->cpu->mode == VIR_CPU_MODE_CUSTOM && !vm->def->cpu->model)
+        return 0;
+
+    VIR_DEBUG("Replacing CPU definition");
 
-    VIR_DEBUG("Replacing CPU def with the updated one");
+    *origCPU = g_steal_pointer(&vm->def->cpu);
 
-    *origCPU = vm->def->cpu;
-    vm->def->cpu = cpu;
+    if (cpu)
+        vm->def->cpu = virCPUDefCopy(cpu);
+    else
+        vm->def->cpu = virCPUDefCopy(*origCPU);
 
     return 0;
 }

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e4bcb628cf299a7601a109b7734cc3e50aecb070..2bd2c424fe268b17b67822dafb484f873f7e87d2 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4437,8 +4437,6 @@ qemuProcessUpdateLiveGuestCPU(virDomainObj *vm,
                               virCPUData *disabled)
 {
     virDomainDef *def = vm->def;
-    qemuDomainObjPrivate *priv = vm->privateData;
-    g_autoptr(virCPUDef) orig = NULL;
     int rc;
 
     if (!enabled)
@@ -4449,19 +4447,11 @@ qemuProcessUpdateLiveGuestCPU(virDomainObj *vm,
          !def->cpu->model))
         return 0;
 
-    orig = virCPUDefCopy(def->cpu);
-
-    if ((rc = virCPUUpdateLive(def->os.arch, def->cpu, enabled, disabled)) < 0) {
+    if ((rc = virCPUUpdateLive(def->os.arch, def->cpu, enabled, disabled)) < 0)
         return -1;
-    } else if (rc == 0) {
-        /* Store the original CPU in priv if QEMU changed it and we didn't
-         * get the original CPU via migration, restore, or snapshot revert.
-         */
-        if (!priv->origCPU && !virCPUDefIsEqual(def->cpu, orig, false))
-            priv->origCPU = g_steal_pointer(&orig);
 
+    if (rc == 0)
         def->cpu->check = VIR_CPU_CHECK_FULL;
-    }
 
     return 0;
 }
@@ -9147,6 +9137,11 @@ qemuProcessReconnect(void *opaque)
 
     qemuDomainVcpuPersistOrder(obj->def);
 
+    /* Make sure the original CPU is always preserved in priv->origCPU. */
+    if (!priv->origCPU &&
+        qemuDomainUpdateCPU(obj, NULL, &priv->origCPU) < 0)
+        goto error;
+
     if (qemuProcessRefreshCPU(driver, obj) < 0)
         goto error;