Re: [PATCH 2/2] qemu: tpm: Remove TPM state after successful migration

On 8/23/22 18: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 <stefanb(a)linux.ibm.com&gt; > --- > src/qemu/qemu_migration.c | 14 +++++++++++--- > src/qemu/qemu_tpm.h | 12 ++++++++++++ > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index 7f69991e2b..355a2cb5c7 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -38,6 +38,7 @@ > #include "qemu_security.h" > #include "qemu_slirp.h" > #include "qemu_block.h" > +#include "qemu_tpm.h" > > #include "domain_audit.h" > #include "virlog.h" > @@ -4006,6 +4007,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, > qemuMigrationJobPhase phase; > g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); > qemuDomainObjPrivate *priv = vm->privateData; > + virDomainUndefineFlagsValues undefFlags = 0;

> int ret = -1; > > VIR_DEBUG("vm=%p, flags=0x%x, cancelled=%d", vm, flags, cancelled); > @@ -4054,7 +4056,9 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, > virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); > vm->persistent = 0; > } > - qemuDomainRemoveInactive(driver, vm, 0); > + if (!qemuTPMCheckKeepTPMStateMigrationSrcSuccess(vm)) > + undefFlags |= VIR_DOMAIN_UNDEFINE_TPM; > + qemuDomainRemoveInactive(driver, vm, undefFlags); > } > > cleanup: > @@ -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; > +} Yeah, I know where you're headed with these two, but G_GNUC_UNUSED breaks syntax-check. And they can be added in shared storage state series.

Michal