On 8/23/22 12:54, Stefan Berger wrote:


On 8/23/22 12:28, Stefan Berger wrote:
This patch 'fixes' the behavior of the persistent_state TPM domain XML
attribute that intends to preserve the state of the TPM but should not
keep the state around on all the hosts a VM has been migrated to. It
removes the TPM state directory structure from the source host upon
successful migration when non-shared storage is used. Similarly, it
removes it from the destination host upon migration failure when
non-shared storage is used.

Signed-off-by: Stefan Berger <stef...@linux.ibm.com>
---

@@ -6590,6 +6594,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver,
      virObjectEvent *event;
      bool inPostCopy = false;
      bool doKill = priv->job.phase != QEMU_MIGRATION_PHASE_FINISH_RESUME;
+    virDomainUndefineFlagsValues undefFlags = 0;
      int rc;
      VIR_DEBUG("vm=%p, flags=0x%lx, retcode=%d",
@@ -6666,8 +6671,11 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver,
                                   jobPriv->migParams, priv->job.apiFlags);
      }
-    if (!virDomainObjIsActive(vm))
-        qemuDomainRemoveInactive(driver, vm, 0);
+    if (!virDomainObjIsActive(vm)) {
+        if (!qemuTPMCheckKeepTPMStateMigrationDstFailure(vm))
+            undefFlags |= VIR_DOMAIN_UNDEFINE_TPM;
+        qemuDomainRemoveInactive(driver, vm, undefFlags);
+    }
      virErrorRestore(&orig_err);
      return NULL;
diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
index f068f3ca5a..c5d8774f07 100644
--- a/src/qemu/qemu_tpm.h
+++ b/src/qemu/qemu_tpm.h
@@ -56,3 +56,15 @@ int qemuExtTPMSetupCgroup(virQEMUDriver *driver,
                            virCgroup *cgroup)
      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
      G_GNUC_WARN_UNUSED_RESULT;
+
+static inline bool
+qemuTPMCheckKeepTPMStateMigrationSrcSuccess(virDomainObj *vm G_GNUC_UNUSED)
+{
+    return false;
+}
+
+static inline bool
+qemuTPMCheckKeepTPMStateMigrationDstFailure(virDomainObj *vm G_GNUC_UNUSED)
+{
+    return false;
+}

These function names are obviously quite expressive but also need to be so that 
one knows what the reason is for having them called...
Any way to proceed on this series?

   Stefan


For shared storage support these will become a bit more involved due to this 
here:

struct _virDomainDef {
     ...
     /* At maximum 2 TPMs on the domain if a TPM Proxy is present. */
     size_t ntpms;
     virDomainTPMDef **tpms;

}

We'll have to loop over the devices (afaik only ppc64 can have 2) to find the 
emulator.

PS: There's a loop again here that will be called by 
qemuDomainRemoveInactive(driver, vm, 0)   :-/

void
qemuExtDevicesCleanupHost(virQEMUDriver *driver,
                           virDomainDef *def,
                           virDomainUndefineFlagsValues flags)
{
     size_t i;

     if (qemuExtDevicesInitPaths(driver, def) < 0)
         return;

     for (i = 0; i < def->ntpms; i++) {
         qemuExtTPMCleanupHost(def->tpms[i], flags);
     }
}


Reply via email to