On 10/21/22 06:55, Michal Prívozník wrote:
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.
I am only starting swtpm with the necessary command line options for shared storage migration if a) shared storage is detected at the time of the start of the VM and b) if swtpm supports shared storage migration at all. If it doesn't support it I let you start the VM but won't let you migrate.
qemuTPMCanMigrateSharedStorage() checks whether the running swtpm instance can do b). It doesn't check the swtpm executable that you may have upgraded or downgraded in the meantime. That's why I need the private data.
So the check above tests whether there is shared storage detected for the TPM_EMULATOR (swtpm; only one allowed) and if so whether the *running swtpm instance* supports shared storage migration. I think it is correct this way.
+ 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.
Yes, will change it
+ 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;
I'd rather not test the swtpm executable that may have been upgraded in the meantime but go for checking the running swtpm instance...
Stefan
+ 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
