[PATCH v3 4/6] qemu: tpm: Pass --migration option to swtpm if supported and needed

[Michal Prívozník]

On 10/18/22 19:04, Stefan Berger wrote:
> Pass the --migration option to swtpm if swptm supports it (starting
> with v0.8) and if the TPM's state is written on shared storage. If this
> is the case apply the 'release-lock-outgoing' parameter with this
> option and apply the 'incoming' parameter for incoming migration so that
> swtpm releases the file lock on the source side when the state is migrated
> and locks the file on the destination side when the state is received.
> 
> If a started swtpm instance is running with the necessary options of
> migrating with share storage then remember this with a flag in the
> virDomainTPMPrivateDef.
> 
> Report an error if swtpm does not support the --migration option and an
> incoming migration across shared storage is requested.
> 
> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
> ---
>  src/qemu/qemu_migration.c |  8 +++++
>  src/qemu/qemu_tpm.c       | 66 ++++++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_tpm.h       |  6 ++++
>  3 files changed, 79 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 33105cf07b..5b4f4615ee 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"
> @@ -2789,6 +2790,13 @@ qemuMigrationSrcBegin(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> +    if (qemuTPMHasSharedStorage(vm->def) &&
> +        !qemuTPMCanMigrateSharedStorage(vm->def)) {

This works, but only because qemuValidateDomainDefTPMs() denies two
emulated TPMs. I think we can call just qemuTPMCanMigrateSharedStorage()
since it already iterates over all TPMs and checks each one individually.

> +        virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +                       _("the running swtpm does not support migration with shared storage"));
> +        goto cleanup;
> +    }
> +
>      if (flags & VIR_MIGRATE_POSTCOPY_RESUME) {
>          ret = qemuMigrationSrcBeginResumePhase(conn, driver, vm, xmlin,
>                                                 cookieout, cookieoutlen, flags);
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index a45ad599aa..7b0afe94ec 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -557,6 +557,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>      int migpwdfile_fd = -1;
>      const unsigned char *secretuuid = NULL;
>      bool create_storage = true;
> +    bool on_shared_storage;
>  
>      if (!swtpm)
>          return NULL;
> @@ -564,7 +565,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>      /* Do not create storage and run swtpm_setup on incoming migration over
>       * shared storage
>       */
> -    if (incomingMigration && virFileIsSharedFS(tpm->data.emulator.storagepath))
> +    on_shared_storage = virFileIsSharedFS(tpm->data.emulator.storagepath);
> +    if (incomingMigration && on_shared_storage)
>          create_storage = false;
>  
>      if (create_storage &&
> @@ -642,6 +644,31 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
>          virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", migpwdfile_fd);
>      }
>  
> +    /* If swtpm supports it and the TPM state is stored on shared storage,
> +     * start swtpm with --migration release-lock-outgoing so it can migrate
> +     * across shared storage if needed.
> +     */
> +    QEMU_DOMAIN_TPM_PRIVATE(tpm)->swtpm.can_migrate_shared_storage = false;

If we don't introduce private data, then this line should be deleted.

> +    if (on_shared_storage &&
> +        virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION)) {
> +
> +        virCommandAddArg(cmd, "--migration");
> +        virCommandAddArgFormat(cmd, "release-lock-outgoing%s",
> +                               incomingMigration ? ",incoming": "");
> +        QEMU_DOMAIN_TPM_PRIVATE(tpm)->swtpm.can_migrate_shared_storage = true;
> +    } else {
> +        /* Report an error if there's an incoming migration across shared
> +         * storage and swtpm does not support the --migration option.
> +         */
> +        if (incomingMigration && on_shared_storage) {
> +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> +                _("%s (on destination side) does not support the --migration option "
> +                  "needed for migration with shared storage"),

Don't hesitate to put whole error message onto one line. It's way easier
to grep then.

> +                   swtpm);
> +            goto error;
> +        }
> +    }
> +
>      return g_steal_pointer(&cmd);
>  
>   error:
> @@ -962,6 +989,43 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
>  }
>  
>  
> +bool
> +qemuTPMHasSharedStorage(virDomainDef *def)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->ntpms; i++) {
> +        virDomainTPMDef *tpm = def->tpms[i];
> +        switch (tpm->type) {
> +        case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +            return virFileIsSharedFS(tpm->data.emulator.storagepath);
> +        case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +        case VIR_DOMAIN_TPM_TYPE_LAST:
> +        }
> +    }
> +
> +    return false;
> +}
> +
> +
> +bool
> +qemuTPMCanMigrateSharedStorage(virDomainDef *def)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->ntpms; i++) {
> +        virDomainTPMDef *tpm = def->tpms[i];
> +        switch (tpm->type) {
> +        case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +            return QEMU_DOMAIN_TPM_PRIVATE(tpm)->swtpm.can_migrate_shared_storage;

If we don't introduce private data, then how about just:

if (virFileIsSharedFS(tpm->data.emulator.storagepath) == 1 &&
    !virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION))
     return false;

> +        case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +        case VIR_DOMAIN_TPM_TYPE_LAST:
> +        }
> +    }
> +    return true;
> +}
> +
> +
>  /* ---------------------
>   *  Module entry points
>   * ---------------------
> diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
> index f068f3ca5a..9daa3e14df 100644
> --- a/src/qemu/qemu_tpm.h
> +++ b/src/qemu/qemu_tpm.h
> @@ -56,3 +56,9 @@ int qemuExtTPMSetupCgroup(virQEMUDriver *driver,
>                            virCgroup *cgroup)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
>      G_GNUC_WARN_UNUSED_RESULT;
> +
> +bool qemuTPMHasSharedStorage(virDomainDef *def)
> +    ATTRIBUTE_NONNULL(1);
> +
> +bool qemuTPMCanMigrateSharedStorage(virDomainDef *def)
> +    ATTRIBUTE_NONNULL(1);

Michal