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@xxxxxxxxxxxxx>
> ---
> 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
