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

[Stefan Berger]

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 <stefanb at 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);
>      }
> }
>