[PATCH v3 0/6] qemu: tpm: Add support for migration across shared storage
This series of patches adds support for migrating vTPMs across hosts whose storage has been set up to share the directory structure holding the state of the TPM (swtpm). The existence of share storage influences the management of the directory structure holding the TPM state, which for example is only removed when a domain is undefined and not when a VM is removed on the migration source host. Further, when shared storage is used then security labeling on the destination side is skipped assuming that the labeling was already done on the source side. I have tested this with an NFS setup where I had to turn SELinux off on the hosts since the SELinux MLS range labeling is not supported by NFS. For shared storage support to work properly both sides of the migration need to be clients of the shared storage setup, meaning that they both have to have /var/lib/libvirt/swtpm mounted as shared storage, because other- wise the virFileIsSharedFS() may not detect shared storage and in the worst case may cause the TPM emulator (swtpm) to malfunction if for example the source side removed the TPM state directory structure. Shared storage migration requires (upcoming) swtpm v0.8. Stefan v3: - Relying entirely on virFileIsSharedFS() on migration source and destination sides to detect whether shared storage is set up between hosts; no more hint about shared storage from user via flag - Added support for virDomainTPMPrivate structure to store and persist TPM-related private data Stefan Berger (6): util: Add parsing support for swtpm's cmdarg-migration capability qemu: tpm: Conditionally create storage on incoming migration qemu: tpm: Add support for storing private TPM-related data qemu: tpm: Pass --migration option to swtpm if supported and needed qemu: tpm: Avoid security labels on incoming migration with shared storage qemu: tpm: Never remove state on outgoing migration and shared storage src/conf/domain_conf.c | 63 ++++++++++++++++++++-- src/conf/domain_conf.h | 9 ++++ src/qemu/qemu_domain.c | 85 +++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 17 +++++- src/qemu/qemu_driver.c | 20 +++---- src/qemu/qemu_extdevice.c | 10 ++-- src/qemu/qemu_extdevice.h | 6 ++- src/qemu/qemu_migration.c | 20 ++++--- src/qemu/qemu_process.c | 9 ++-- src/qemu/qemu_snapshot.c | 4 +- src/qemu/qemu_tpm.c | 111 ++++++++++++++++++++++++++++++++++---- src/qemu/qemu_tpm.h | 12 ++++- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + 14 files changed, 318 insertions(+), 50 deletions(-) -- 2.37.3
Add support for parsing swtpm 'cmdarg-migration' capability (since v0.8). Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 91db0f31eb..19850de1c8 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -39,6 +39,7 @@ VIR_LOG_INIT("util.tpm"); VIR_ENUM_IMPL(virTPMSwtpmFeature, VIR_TPM_SWTPM_FEATURE_LAST, "cmdarg-pwd-fd", + "cmdarg-migration", ); VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, diff --git a/src/util/virtpm.h b/src/util/virtpm.h index a873881b23..fb330effa8 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -30,6 +30,7 @@ bool virTPMHasSwtpm(void); typedef enum { VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD, + VIR_TPM_SWTPM_FEATURE_CMDARG_MIGRATION, VIR_TPM_SWTPM_FEATURE_LAST } virTPMSwtpmFeature; -- 2.37.3
Do not create storage if the TPM state files are on shared storage and there's an incoming migration since in this case the storage directory must already exist. Also do not run swtpm_setup in this case. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index dc09c94a4d..a45ad599aa 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -556,11 +556,19 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, int pwdfile_fd = -1; int migpwdfile_fd = -1; const unsigned char *secretuuid = NULL; + bool create_storage = true; if (!swtpm) return NULL; - if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) + /* Do not create storage and run swtpm_setup on incoming migration over + * shared storage + */ + if (incomingMigration && virFileIsSharedFS(tpm->data.emulator.storagepath)) + create_storage = false; + + if (create_storage && + qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) return NULL; if (tpm->data.emulator.hassecretuuid) -- 2.37.3
On 10/18/22 19:04, Stefan Berger wrote:
Do not create storage if the TPM state files are on shared storage and there's an incoming migration since in this case the storage directory must already exist. Also do not run swtpm_setup in this case.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index dc09c94a4d..a45ad599aa 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -556,11 +556,19 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, int pwdfile_fd = -1; int migpwdfile_fd = -1; const unsigned char *secretuuid = NULL; + bool create_storage = true;
if (!swtpm) return NULL;
- if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) + /* Do not create storage and run swtpm_setup on incoming migration over + * shared storage + */ + if (incomingMigration && virFileIsSharedFS(tpm->data.emulator.storagepath))
Here and everywhere else, this needs to be virFileIsSharedFS() == 1, because the function may return -1, 0, 1 and we do not want to treat -1 as 1.
+ create_storage = false; + + if (create_storage && + qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) return NULL;
if (tpm->data.emulator.hassecretuuid)
Michal
On 10/21/22 06:55, Michal Prívozník wrote:
On 10/18/22 19:04, Stefan Berger wrote:
Do not create storage if the TPM state files are on shared storage and there's an incoming migration since in this case the storage directory must already exist. Also do not run swtpm_setup in this case.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index dc09c94a4d..a45ad599aa 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -556,11 +556,19 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm, int pwdfile_fd = -1; int migpwdfile_fd = -1; const unsigned char *secretuuid = NULL; + bool create_storage = true;
if (!swtpm) return NULL;
- if (qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) + /* Do not create storage and run swtpm_setup on incoming migration over + * shared storage + */ + if (incomingMigration && virFileIsSharedFS(tpm->data.emulator.storagepath))
Here and everywhere else, this needs to be virFileIsSharedFS() == 1, because the function may return -1, 0, 1 and we do not want to treat -1 as 1.
Thanks, I will fix it.
+ create_storage = false; + + if (create_storage && + qemuTPMEmulatorCreateStorage(tpm, &created, swtpm_user, swtpm_group) < 0) return NULL;
if (tpm->data.emulator.hassecretuuid)
Michal
Stefan
Add support for storing private TPM-related data. The first private data will be related to the capability of the started swtpm indicating whether it is capable of migration with a shared storage setup since that requires support for certain command line flags that were only becoming available in v0.8. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 63 +++++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 9 ++++++ src/qemu/qemu_domain.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 14 ++++++++ 4 files changed, 154 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dba65cfeb..4178583950 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3276,6 +3276,22 @@ void virDomainHostdevDefClear(virDomainHostdevDef *def) } } +static virDomainTPMDef * +virDomainTPMDefNew(virDomainXMLOption *xmlopt) +{ + virDomainTPMDef *def; + + def = g_new0(virDomainTPMDef, 1); + + if (xmlopt && xmlopt->privateData.tpmNew && + !(def->privateData = xmlopt->privateData.tpmNew())) { + VIR_FREE(def); + return NULL; + } + + return def; +} + void virDomainTPMDefFree(virDomainTPMDef *def) { if (!def) @@ -3296,6 +3312,7 @@ void virDomainTPMDefFree(virDomainTPMDef *def) } virDomainDeviceInfoClear(&def->info); + virObjectUnref(def->privateData); g_free(def); } @@ -10238,7 +10255,8 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, g_autofree xmlNodePtr *nodes = NULL; int bank; - def = g_new0(virDomainTPMDef, 1); + if (!(def = virDomainTPMDefNew(xmlopt))) + return NULL; if (virXMLPropEnum(node, "model", virDomainTPMModelTypeFromString, @@ -10329,6 +10347,14 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt, if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) < 0) goto error; + if (flags & VIR_DOMAIN_DEF_PARSE_STATUS && + xmlopt && xmlopt->privateData.tpmParse) { + if ((ctxt->node = virXPathNode("./privateData", ctxt))) { + if (xmlopt->privateData.tpmParse(ctxt, def) < 0) + goto error; + } + } + return def; error: @@ -24049,10 +24075,32 @@ virDomainSoundCodecDefFormat(virBuffer *buf, return 0; } -static void +static int +virDomainTPMDefFormatPrivateData(virBuffer *buf, + const virDomainTPMDef *tpm, + unsigned int flags, + virDomainXMLOption *xmlopt) +{ + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + + if (!(flags & VIR_DOMAIN_DEF_FORMAT_STATUS) || + !xmlopt || + !xmlopt->privateData.tpmFormat) + return 0; + + if (xmlopt->privateData.tpmFormat(tpm, &childBuf) < 0) + return -1; + + virXMLFormatElement(buf, "privateData", NULL, &childBuf); + return 0; +} + + +static int virDomainTPMDefFormat(virBuffer *buf, const virDomainTPMDef *def, - unsigned int flags) + unsigned int flags, + virDomainXMLOption *xmlopt) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); @@ -24101,8 +24149,12 @@ virDomainTPMDefFormat(virBuffer *buf, virXMLFormatElement(&childBuf, "backend", &backendAttrBuf, &backendChildBuf); virDomainDeviceInfoFormat(&childBuf, &def->info, flags); + if (virDomainTPMDefFormatPrivateData(&childBuf, def, flags, xmlopt) < 0) + return -1; virXMLFormatElement(buf, "tpm", &attrBuf, &childBuf); + + return 0; } @@ -27188,7 +27240,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, } for (n = 0; n < def->ntpms; n++) { - virDomainTPMDefFormat(buf, def->tpms[n], flags); + if (virDomainTPMDefFormat(buf, def->tpms[n], flags, xmlopt) < 0) + return -1; } for (n = 0; n < def->ngraphics; n++) { @@ -28454,7 +28507,7 @@ virDomainDeviceDefCopy(virDomainDeviceDef *src, rc = virDomainChrDefFormat(&buf, src->data.chr, flags); break; case VIR_DOMAIN_DEVICE_TPM: - virDomainTPMDefFormat(&buf, src->data.tpm, flags); + virDomainTPMDefFormat(&buf, src->data.tpm, flags, xmlopt); rc = 0; break; case VIR_DOMAIN_DEVICE_PANIC: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8f8a54bc41..82f71f8853 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1445,6 +1445,8 @@ typedef enum { #define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0" struct _virDomainTPMDef { + virObject *privateData; + virDomainTPMModel model; virDomainTPMBackendType type; virDomainDeviceInfo info; @@ -3248,6 +3250,10 @@ typedef int (*virDomainXMLPrivateDataStorageSourceParseFunc)(xmlXPathContextPtr typedef int (*virDomainXMLPrivateDataStorageSourceFormatFunc)(virStorageSource *src, virBuffer *buf); +typedef int (*virDomainXMLPrivateDataTPMParseFunc)(xmlXPathContextPtr ctxt, + virDomainTPMDef *disk); +typedef int (*virDomainXMLPrivateDataTPMFormatFunc)(const virDomainTPMDef *tpm, + virBuffer *buf); struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataAllocFunc alloc; @@ -3264,6 +3270,9 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataNewFunc networkNew; virDomainXMLPrivateDataNewFunc videoNew; virDomainXMLPrivateDataNewFunc fsNew; + virDomainXMLPrivateDataTPMParseFunc tpmParse; + virDomainXMLPrivateDataTPMFormatFunc tpmFormat; + virDomainXMLPrivateDataNewFunc tpmNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; /* following function shall return a pointer which will be used as the diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4c14fc2aef..97c62e2c9e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1139,6 +1139,76 @@ qemuDomainVideoPrivateDispose(void *obj) } +static virClass *qemuDomainTPMPrivateClass; +static void qemuDomainTPMPrivateDispose(void *obj); + + +static int +qemuDomainTPMPrivateOnceInit(void) +{ + if (!VIR_CLASS_NEW(qemuDomainTPMPrivate, virClassForObject())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(qemuDomainTPMPrivate); + + +static virObject * +qemuDomainTPMPrivateNew(void) +{ + qemuDomainTPMPrivate *priv; + + if (qemuDomainTPMPrivateInitialize() < 0) + return NULL; + + if (!(priv = virObjectNew(qemuDomainTPMPrivateClass))) + return NULL; + + return (virObject *) priv; +} + + +static void +qemuDomainTPMPrivateDispose(void *obj G_GNUC_UNUSED) +{ +} + + +static int +qemuDomainTPMPrivateParse(xmlXPathContextPtr ctxt, + virDomainTPMDef *tpm) +{ + qemuDomainTPMPrivate *priv = QEMU_DOMAIN_TPM_PRIVATE(tpm); + + priv->swtpm.can_migrate_shared_storage = + virXPathBoolean("string(./swtpm/@can_migrate_shared_storage)", ctxt); + + return 0; +} + + +static int +qemuDomainTPMPrivateFormat(const virDomainTPMDef *tpm, + virBuffer *buf) +{ + qemuDomainTPMPrivate *priv = QEMU_DOMAIN_TPM_PRIVATE(tpm); + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + if (priv->swtpm.can_migrate_shared_storage) + virBufferAddLit(buf, "<swtpm can_migrate_shared_storage='yes'/>\n"); + break; + + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + } + + return 0; +} + + /* qemuDomainSecretInfoSetup: * @priv: pointer to domain private object * @alias: alias of the secret @@ -3203,6 +3273,9 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .graphicsNew = qemuDomainGraphicsPrivateNew, .networkNew = qemuDomainNetworkPrivateNew, .videoNew = qemuDomainVideoPrivateNew, + .tpmNew = qemuDomainTPMPrivateNew, + .tpmParse = qemuDomainTPMPrivateParse, + .tpmFormat = qemuDomainTPMPrivateFormat, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, .getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a22deaf113..e7d3e1be40 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -407,6 +407,20 @@ struct _qemuDomainNetworkPrivate { qemuFDPass *vdpafd; }; + +#define QEMU_DOMAIN_TPM_PRIVATE(dev) \ + ((qemuDomainTPMPrivate *) (dev)->privateData) + +typedef struct _qemuDomainTPMPrivate qemuDomainTPMPrivate; +struct _qemuDomainTPMPrivate { + virObject parent; + + struct { + bool can_migrate_shared_storage; + } swtpm; +}; + + void qemuDomainNetworkPrivateClearFDs(qemuDomainNetworkPrivate *priv); -- 2.37.3
On 10/18/22 19:04, Stefan Berger wrote:
Add support for storing private TPM-related data. The first private data will be related to the capability of the started swtpm indicating whether it is capable of migration with a shared storage setup since that requires support for certain command line flags that were only becoming available in v0.8.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 63 +++++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 9 ++++++ src/qemu/qemu_domain.c | 73 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 14 ++++++++ 4 files changed, 154 insertions(+), 5 deletions(-)
I know I've pointed you in this direction, but now that I'm seeing the actual I code I wonder whether we can do without private data. See my other replies. Michal
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@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)) { + 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 (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"), + 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; + 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); -- 2.37.3
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@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
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@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.
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
On 10/21/22 13:36, Stefan Berger wrote:
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@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.
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.
Ah, good point. So let me fix the other small nits I've raised and merge these. Michal
On 10/21/22 15:31, Michal Prívozník wrote:
On 10/21/22 13:36, Stefan Berger wrote:
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@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.
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.
Ah, good point. So let me fix the other small nits I've raised and merge these.
Actually, I'll investigate whether we need the very last patch. I mean, we definitely do not want to remove the swtpm state from the source, BUT I wonder whether we can set @flags passed to qemuDomainRemoveInactiveCommon() (e.g. to VIR_DOMAIN_UNDEFINE_KEEP_TPM) instead of introducing another argument with basically the same functionality. Michal
When using shared storage there is no need to apply security labels on the storage since the files have to have been labeled already on the source side and we must assume that the source and destination side have been setup to use the same uid and gid for running swtpm as well as share the same security labels. Whether the security labels can be used at all depends on the shared storage and whether and how it supports them. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 7b0afe94ec..69410e36ff 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -933,10 +933,18 @@ qemuTPMEmulatorStart(virQEMUDriver *driver, virCommandSetPidFile(cmd, pidfile); virCommandSetErrorFD(cmd, &errfd); - if (qemuSecurityStartTPMEmulator(driver, vm, cmd, - cfg->swtpm_user, cfg->swtpm_group, - NULL, &cmdret) < 0) - return -1; + if (incomingMigration && virFileIsSharedFS(tpm->data.emulator.storagepath)) { + /* security labels must have been set up on source already */ + if (qemuSecurityCommandRun(driver, vm, cmd, + cfg->swtpm_user, cfg->swtpm_group, + NULL, &cmdret) < 0) { + goto error; + } + } else if (qemuSecurityStartTPMEmulator(driver, vm, cmd, + cfg->swtpm_user, cfg->swtpm_group, + NULL, &cmdret) < 0) { + goto error; + } if (cmdret < 0) { /* virCommandRun() hidden in qemuSecurityStartTPMEmulator() -- 2.37.3
Never remove the TPM state on outgoing migration if the storage setup has shared storage for the TPM state files. Also, do not do the security cleanup on outgoing migration if shared storage is detected. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_domain.c | 12 +++++++----- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_driver.c | 20 ++++++++++---------- src/qemu/qemu_extdevice.c | 10 ++++++---- src/qemu/qemu_extdevice.h | 6 ++++-- src/qemu/qemu_migration.c | 12 ++++++------ src/qemu/qemu_process.c | 9 ++++++--- src/qemu/qemu_snapshot.c | 4 ++-- src/qemu/qemu_tpm.c | 21 ++++++++++++++++----- src/qemu/qemu_tpm.h | 6 ++++-- 10 files changed, 63 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 97c62e2c9e..20cc2409fc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7245,7 +7245,8 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriver *driver, static void qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, virDomainObj *vm, - virDomainUndefineFlagsValues flags) + virDomainUndefineFlagsValues flags, + bool outgoingMigration) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *snapDir = NULL; @@ -7271,7 +7272,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, if (rmdir(chkDir) < 0 && errno != ENOENT) VIR_WARN("unable to remove checkpoint directory %s", chkDir); } - qemuExtDevicesCleanupHost(driver, vm->def, flags); + qemuExtDevicesCleanupHost(driver, vm->def, flags, outgoingMigration); } @@ -7283,14 +7284,15 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, void qemuDomainRemoveInactive(virQEMUDriver *driver, virDomainObj *vm, - virDomainUndefineFlagsValues flags) + virDomainUndefineFlagsValues flags, + bool outgoingMigration) { if (vm->persistent) { /* Short-circuit, we don't want to remove a persistent domain */ return; } - qemuDomainRemoveInactiveCommon(driver, vm, flags); + qemuDomainRemoveInactiveCommon(driver, vm, flags, outgoingMigration); virDomainObjListRemove(driver->domains, vm); } @@ -7312,7 +7314,7 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, return; } - qemuDomainRemoveInactiveCommon(driver, vm, 0); + qemuDomainRemoveInactiveCommon(driver, vm, 0, false); virDomainObjListRemoveLocked(driver->domains, vm); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e7d3e1be40..11ea52c32d 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -696,7 +696,8 @@ int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriver *driver, void qemuDomainRemoveInactive(virQEMUDriver *driver, virDomainObj *vm, - virDomainUndefineFlagsValues flags); + virDomainUndefineFlagsValues flags, + bool outgoingMigration); void qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5c75000742..017cda2a9c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1611,7 +1611,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, goto cleanup; if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, flags) < 0) { - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); goto cleanup; } @@ -1620,7 +1620,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags) < 0) { virDomainAuditStart(vm, "booted", false); - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); qemuProcessEndJob(vm); goto cleanup; } @@ -2103,7 +2103,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, ret = 0; endjob: if (ret == 0) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); virDomainObjEndJob(vm); cleanup: @@ -2723,7 +2723,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver, } virDomainObjEndAsyncJob(vm); if (ret == 0) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); cleanup: virQEMUSaveDataFree(data); @@ -3263,7 +3263,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, virDomainObjEndAsyncJob(vm); if (ret == 0 && flags & VIR_DUMP_CRASH) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); cleanup: virDomainObjEndAPI(&vm); @@ -3575,7 +3575,7 @@ processGuestPanicEvent(virQEMUDriver *driver, endjob: virDomainObjEndAsyncJob(vm); if (removeInactive) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); } @@ -4053,7 +4053,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); endjob: - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); virDomainObjEndJob(vm); } @@ -5985,7 +5985,7 @@ qemuDomainRestoreInternal(virConnectPtr conn, virFileWrapperFdFree(wrapperFd); virQEMUSaveDataFree(data); if (vm && ret < 0) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); virDomainObjEndAPI(&vm); return ret; } @@ -6675,7 +6675,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, } else { /* Brand new domain. Remove it */ VIR_INFO("Deleting domain '%s'", vm->def->name); - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); } } @@ -6824,7 +6824,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, */ vm->persistent = 0; if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, flags); + qemuDomainRemoveInactive(driver, vm, flags, false); ret = 0; endjob: diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 24a57b0f74..3eaf6571a2 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -152,7 +152,8 @@ qemuExtDevicesPrepareHost(virQEMUDriver *driver, void qemuExtDevicesCleanupHost(virQEMUDriver *driver, virDomainDef *def, - virDomainUndefineFlagsValues flags) + virDomainUndefineFlagsValues flags, + bool outgoingMigration) { size_t i; @@ -160,7 +161,7 @@ qemuExtDevicesCleanupHost(virQEMUDriver *driver, return; for (i = 0; i < def->ntpms; i++) { - qemuExtTPMCleanupHost(def->tpms[i], flags); + qemuExtTPMCleanupHost(def->tpms[i], flags, outgoingMigration); } } @@ -225,7 +226,8 @@ qemuExtDevicesStart(virQEMUDriver *driver, void qemuExtDevicesStop(virQEMUDriver *driver, - virDomainObj *vm) + virDomainObj *vm, + bool outgoingMigration) { virDomainDef *def = vm->def; size_t i; @@ -242,7 +244,7 @@ qemuExtDevicesStop(virQEMUDriver *driver, for (i = 0; i < def->ntpms; i++) { if (def->tpms[i]->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) - qemuExtTPMStop(driver, vm); + qemuExtTPMStop(driver, vm, outgoingMigration); } for (i = 0; i < def->nnets; i++) { diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 6b05b59cd6..86e7133a2a 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -42,7 +42,8 @@ int qemuExtDevicesPrepareHost(virQEMUDriver *driver, void qemuExtDevicesCleanupHost(virQEMUDriver *driver, virDomainDef *def, - virDomainUndefineFlagsValues flags) + virDomainUndefineFlagsValues flags, + bool outgoingMigration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuExtDevicesStart(virQEMUDriver *driver, @@ -52,7 +53,8 @@ int qemuExtDevicesStart(virQEMUDriver *driver, G_GNUC_WARN_UNUSED_RESULT; void qemuExtDevicesStop(virQEMUDriver *driver, - virDomainObj *vm) + virDomainObj *vm, + bool outgoingMigration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); bool qemuExtDevicesHasDevice(virDomainDef *def); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5b4f4615ee..990c7e6829 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3399,7 +3399,7 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, * and there is no 'goto cleanup;' in the middle of those */ VIR_FREE(priv->origname); virDomainObjRemoveTransientDef(vm); - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); } virDomainObjEndAPI(&vm); virErrorRestore(&origErr); @@ -4044,7 +4044,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM); + qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM, true); } cleanup: @@ -6047,7 +6047,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, true); } virErrorRestore(&orig_err); @@ -6174,7 +6174,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, } if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, true); return ret; } @@ -6710,7 +6710,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, } if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM); + qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM, false); virErrorRestore(&orig_err); return NULL; @@ -6847,7 +6847,7 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, qemuMigrationJobFinish(vm); if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1a9175f40f..26b2edb05f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8073,6 +8073,7 @@ void qemuProcessStop(virQEMUDriver *driver, g_autofree char *timestamp = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virConnect) conn = NULL; + bool outgoingMigration; VIR_DEBUG("Shutting down vm=%p name=%s id=%d pid=%lld, " "reason=%s, asyncJob=%s, flags=0x%x", @@ -8170,7 +8171,9 @@ void qemuProcessStop(virQEMUDriver *driver, qemuDomainCleanupRun(driver, vm); - qemuExtDevicesStop(driver, vm); + outgoingMigration = (flags & VIR_QEMU_PROCESS_STOP_MIGRATED) && + (asyncJob != VIR_ASYNC_JOB_MIGRATION_IN); + qemuExtDevicesStop(driver, vm, outgoingMigration); qemuDBusStop(driver, vm); @@ -8436,7 +8439,7 @@ qemuProcessAutoDestroy(virDomainObj *dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - qemuDomainRemoveInactive(driver, dom, 0); + qemuDomainRemoveInactive(driver, dom, 0, false); virDomainObjEndJob(dom); @@ -8899,7 +8902,7 @@ qemuProcessReconnect(void *opaque) if (jobStarted) virDomainObjEndJob(obj); if (!virDomainObjIsActive(obj)) - qemuDomainRemoveInactive(driver, obj, 0); + qemuDomainRemoveInactive(driver, obj, 0, false); virDomainObjEndAPI(&obj); virIdentitySetCurrent(NULL); return; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 06b5c180ff..d7983c134f 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2103,7 +2103,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, } if (qemuSnapshotInternalRevertInactive(driver, vm, snap) < 0) { - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); return -1; } @@ -2125,7 +2125,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); if (rc < 0) { - qemuDomainRemoveInactive(driver, vm, 0); + qemuDomainRemoveInactive(driver, vm, 0, false); return -1; } detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 69410e36ff..f7d1487111 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -729,13 +729,21 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, * qemuTPMEmulatorCleanupHost: * @tpm: TPM definition * @flags: flags indicating whether to keep or remove TPM persistent state + * @outgoingMigration: whether cleanup due to an outgoing migration * * Clean up persistent storage for the swtpm. */ static void qemuTPMEmulatorCleanupHost(virDomainTPMDef *tpm, - virDomainUndefineFlagsValues flags) + virDomainUndefineFlagsValues flags, + bool outgoingMigration) { + /* Never remove the state in case of outgoing migration with shared + * storage. + */ + if (outgoingMigration && virFileIsSharedFS(tpm->data.emulator.storagepath)) + return; + /* * remove TPM state if: * - persistent_state flag is set and the UNDEFINE_TPM flag is set @@ -1081,9 +1089,10 @@ qemuExtTPMPrepareHost(virQEMUDriver *driver, void qemuExtTPMCleanupHost(virDomainTPMDef *tpm, - virDomainUndefineFlagsValues flags) + virDomainUndefineFlagsValues flags, + bool outgoingMigration) { - qemuTPMEmulatorCleanupHost(tpm, flags); + qemuTPMEmulatorCleanupHost(tpm, flags, outgoingMigration); } @@ -1104,7 +1113,8 @@ qemuExtTPMStart(virQEMUDriver *driver, void qemuExtTPMStop(virQEMUDriver *driver, - virDomainObj *vm) + virDomainObj *vm, + bool outgoingMigration) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *shortName = virDomainDefGetShortName(vm->def); @@ -1113,7 +1123,8 @@ qemuExtTPMStop(virQEMUDriver *driver, return; qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); - qemuSecurityCleanupTPMEmulator(driver, vm); + if (!(outgoingMigration && qemuTPMHasSharedStorage(vm->def))) + qemuSecurityCleanupTPMEmulator(driver, vm); } diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 9daa3e14df..53ff51f1d0 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -36,7 +36,8 @@ int qemuExtTPMPrepareHost(virQEMUDriver *driver, G_GNUC_WARN_UNUSED_RESULT; void qemuExtTPMCleanupHost(virDomainTPMDef *tpm, - virDomainUndefineFlagsValues flags) + virDomainUndefineFlagsValues flags, + bool outgoingMigration) ATTRIBUTE_NONNULL(1); int qemuExtTPMStart(virQEMUDriver *driver, @@ -48,7 +49,8 @@ int qemuExtTPMStart(virQEMUDriver *driver, G_GNUC_WARN_UNUSED_RESULT; void qemuExtTPMStop(virQEMUDriver *driver, - virDomainObj *vm) + virDomainObj *vm, + bool outgoingMigration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuExtTPMSetupCgroup(virQEMUDriver *driver, -- 2.37.3
On 10/18/22 19:04, Stefan Berger wrote:
This series of patches adds support for migrating vTPMs across hosts whose storage has been set up to share the directory structure holding the state of the TPM (swtpm). The existence of share storage influences the management of the directory structure holding the TPM state, which for example is only removed when a domain is undefined and not when a VM is removed on the migration source host. Further, when shared storage is used then security labeling on the destination side is skipped assuming that the labeling was already done on the source side.
I have tested this with an NFS setup where I had to turn SELinux off on the hosts since the SELinux MLS range labeling is not supported by NFS.
For shared storage support to work properly both sides of the migration need to be clients of the shared storage setup, meaning that they both have to have /var/lib/libvirt/swtpm mounted as shared storage, because other- wise the virFileIsSharedFS() may not detect shared storage and in the worst case may cause the TPM emulator (swtpm) to malfunction if for example the source side removed the TPM state directory structure.
Shared storage migration requires (upcoming) swtpm v0.8.
Stefan
v3: - Relying entirely on virFileIsSharedFS() on migration source and destination sides to detect whether shared storage is set up between hosts; no more hint about shared storage from user via flag - Added support for virDomainTPMPrivate structure to store and persist TPM-related private data
Stefan Berger (6): util: Add parsing support for swtpm's cmdarg-migration capability qemu: tpm: Conditionally create storage on incoming migration qemu: tpm: Add support for storing private TPM-related data qemu: tpm: Pass --migration option to swtpm if supported and needed qemu: tpm: Avoid security labels on incoming migration with shared storage qemu: tpm: Never remove state on outgoing migration and shared storage
src/conf/domain_conf.c | 63 ++++++++++++++++++++-- src/conf/domain_conf.h | 9 ++++ src/qemu/qemu_domain.c | 85 +++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 17 +++++- src/qemu/qemu_driver.c | 20 +++---- src/qemu/qemu_extdevice.c | 10 ++-- src/qemu/qemu_extdevice.h | 6 ++- src/qemu/qemu_migration.c | 20 ++++--- src/qemu/qemu_process.c | 9 ++-- src/qemu/qemu_snapshot.c | 4 +- src/qemu/qemu_tpm.c | 111 ++++++++++++++++++++++++++++++++++---- src/qemu/qemu_tpm.h | 12 ++++- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + 14 files changed, 318 insertions(+), 50 deletions(-)
Patches look good. I did an experiment and tried to rework the code that patch 3/6 is not needed. You can find it here: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/tpm_migration_rework If you don't like it, I can merge your patches as they are. Or, let me know if you're okay with my changes. No need to resend either way. Michal
On 10/21/22 06:55, Michal Prívozník wrote:
On 10/18/22 19:04, Stefan Berger wrote:
This series of patches adds support for migrating vTPMs across hosts whose storage has been set up to share the directory structure holding the state of the TPM (swtpm). The existence of share storage influences the management of the directory structure holding the TPM state, which for example is only removed when a domain is undefined and not when a VM is removed on the migration source host. Further, when shared storage is used then security labeling on the destination side is skipped assuming that the labeling was already done on the source side.
I have tested this with an NFS setup where I had to turn SELinux off on the hosts since the SELinux MLS range labeling is not supported by NFS.
For shared storage support to work properly both sides of the migration need to be clients of the shared storage setup, meaning that they both have to have /var/lib/libvirt/swtpm mounted as shared storage, because other- wise the virFileIsSharedFS() may not detect shared storage and in the worst case may cause the TPM emulator (swtpm) to malfunction if for example the source side removed the TPM state directory structure.
Shared storage migration requires (upcoming) swtpm v0.8.
Stefan
v3: - Relying entirely on virFileIsSharedFS() on migration source and destination sides to detect whether shared storage is set up between hosts; no more hint about shared storage from user via flag - Added support for virDomainTPMPrivate structure to store and persist TPM-related private data
Stefan Berger (6): util: Add parsing support for swtpm's cmdarg-migration capability qemu: tpm: Conditionally create storage on incoming migration qemu: tpm: Add support for storing private TPM-related data qemu: tpm: Pass --migration option to swtpm if supported and needed qemu: tpm: Avoid security labels on incoming migration with shared storage qemu: tpm: Never remove state on outgoing migration and shared storage
src/conf/domain_conf.c | 63 ++++++++++++++++++++-- src/conf/domain_conf.h | 9 ++++ src/qemu/qemu_domain.c | 85 +++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 17 +++++- src/qemu/qemu_driver.c | 20 +++---- src/qemu/qemu_extdevice.c | 10 ++-- src/qemu/qemu_extdevice.h | 6 ++- src/qemu/qemu_migration.c | 20 ++++--- src/qemu/qemu_process.c | 9 ++-- src/qemu/qemu_snapshot.c | 4 +- src/qemu/qemu_tpm.c | 111 ++++++++++++++++++++++++++++++++++---- src/qemu/qemu_tpm.h | 12 ++++- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + 14 files changed, 318 insertions(+), 50 deletions(-)
Patches look good. I did an experiment and tried to rework the code that patch 3/6 is not needed. You can find it here:
https://gitlab.com/MichalPrivoznik/libvirt/-/commits/tpm_migration_rework
If you don't like it, I can merge your patches as they are. Or, let me know if you're okay with my changes. No need to resend either way.
I think we need a v4. I have mine here: https://github.com/stefanberger/libvirt-tpm/tree/swtpm_shared_storage.v4
Michal
On 10/21/22 17:27, Stefan Berger wrote:
On 10/21/22 06:55, Michal Prívozník wrote:
On 10/18/22 19:04, Stefan Berger wrote:
This series of patches adds support for migrating vTPMs across hosts whose storage has been set up to share the directory structure holding the state of the TPM (swtpm). The existence of share storage influences the management of the directory structure holding the TPM state, which for example is only removed when a domain is undefined and not when a VM is removed on the migration source host. Further, when shared storage is used then security labeling on the destination side is skipped assuming that the labeling was already done on the source side.
I have tested this with an NFS setup where I had to turn SELinux off on the hosts since the SELinux MLS range labeling is not supported by NFS.
For shared storage support to work properly both sides of the migration need to be clients of the shared storage setup, meaning that they both have to have /var/lib/libvirt/swtpm mounted as shared storage, because other- wise the virFileIsSharedFS() may not detect shared storage and in the worst case may cause the TPM emulator (swtpm) to malfunction if for example the source side removed the TPM state directory structure.
Shared storage migration requires (upcoming) swtpm v0.8.
Stefan
v3: - Relying entirely on virFileIsSharedFS() on migration source and destination sides to detect whether shared storage is set up between hosts; no more hint about shared storage from user via flag - Added support for virDomainTPMPrivate structure to store and persist TPM-related private data
Stefan Berger (6): util: Add parsing support for swtpm's cmdarg-migration capability qemu: tpm: Conditionally create storage on incoming migration qemu: tpm: Add support for storing private TPM-related data qemu: tpm: Pass --migration option to swtpm if supported and needed qemu: tpm: Avoid security labels on incoming migration with shared storage qemu: tpm: Never remove state on outgoing migration and shared storage
src/conf/domain_conf.c | 63 ++++++++++++++++++++-- src/conf/domain_conf.h | 9 ++++ src/qemu/qemu_domain.c | 85 +++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 17 +++++- src/qemu/qemu_driver.c | 20 +++---- src/qemu/qemu_extdevice.c | 10 ++-- src/qemu/qemu_extdevice.h | 6 ++- src/qemu/qemu_migration.c | 20 ++++--- src/qemu/qemu_process.c | 9 ++-- src/qemu/qemu_snapshot.c | 4 +- src/qemu/qemu_tpm.c | 111 ++++++++++++++++++++++++++++++++++---- src/qemu/qemu_tpm.h | 12 ++++- src/util/virtpm.c | 1 + src/util/virtpm.h | 1 + 14 files changed, 318 insertions(+), 50 deletions(-)
Patches look good. I did an experiment and tried to rework the code that patch 3/6 is not needed. You can find it here:
https://gitlab.com/MichalPrivoznik/libvirt/-/commits/tpm_migration_rework
If you don't like it, I can merge your patches as they are. Or, let me know if you're okay with my changes. No need to resend either way.
I think we need a v4. I have mine here: https://github.com/stefanberger/libvirt-tpm/tree/swtpm_shared_storage.v4
Yep. Also I realized that we need that extra argument to qemuDomainRemoveInactive(), because in some cases we want to pass VIR_DOMAIN_UNDEFINE_TPM flag, and yet not remove any TPM state if it's on a shared storage. Alright, I've uploaded my fixup commits here: https://gitlab.com/MichalPrivoznik/libvirt/-/tree/tpm_migration_rework Can you work in those fixup commits and resend? Michal
participants (2)
-
Michal Prívozník -
Stefan Berger