[PATCH 0/2] Fix migration with TPM on shared storage
Jiri Denemark (2): qemu: Rename outgoingMigration parameter in various TPM functions qemu: Properly propagate migration state to TPM cleanup code src/qemu/qemu_domain.c | 8 ++++---- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 7 +++++-- src/qemu/qemu_extdevice.c | 8 ++++---- src/qemu/qemu_extdevice.h | 4 ++-- src/qemu/qemu_migration.c | 6 +++--- src/qemu/qemu_process.c | 8 ++------ src/qemu/qemu_tpm.c | 19 +++++++++---------- src/qemu/qemu_tpm.h | 4 ++-- 9 files changed, 32 insertions(+), 34 deletions(-) -- 2.49.0
From: Jiri Denemark <jdenemar@redhat.com> The parameter is used to skip TPM state cleanup on outgoing migration with shared storage. But we also need to skip the cleanup after a failed incoming migration. Let's call the parameter "migration" to reflect its usage on both sides of migration. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++---- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_extdevice.c | 8 ++++---- src/qemu/qemu_extdevice.h | 4 ++-- src/qemu/qemu_tpm.c | 19 +++++++++---------- src/qemu/qemu_tpm.h | 4 ++-- 6 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c3ca4b3040..47c110c6d7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5790,7 +5790,7 @@ static void qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, virDomainObj *vm, virDomainUndefineFlagsValues flags, - bool outgoingMigration) + bool migration) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *snapDir = NULL; @@ -5816,7 +5816,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, if (rmdir(chkDir) < 0 && errno != ENOENT) VIR_WARN("unable to remove checkpoint directory %s", chkDir); } - qemuExtDevicesCleanupHost(driver, vm->def, flags, outgoingMigration); + qemuExtDevicesCleanupHost(driver, vm->def, flags, migration); } @@ -5829,14 +5829,14 @@ void qemuDomainRemoveInactive(virQEMUDriver *driver, virDomainObj *vm, virDomainUndefineFlagsValues flags, - bool outgoingMigration) + bool migration) { if (vm->persistent) { /* Short-circuit, we don't want to remove a persistent domain */ return; } - qemuDomainRemoveInactiveCommon(driver, vm, flags, outgoingMigration); + qemuDomainRemoveInactiveCommon(driver, vm, flags, migration); virDomainObjListRemove(driver->domains, vm); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 70e1fb187f..6fd47f16e7 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -693,7 +693,7 @@ int qemuDomainMomentDiscardAll(void *payload, void qemuDomainRemoveInactive(virQEMUDriver *driver, virDomainObj *vm, virDomainUndefineFlagsValues flags, - bool outgoingMigration); + bool migration); void qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 78e72b7c10..31c7a14156 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -154,7 +154,7 @@ void qemuExtDevicesCleanupHost(virQEMUDriver *driver, virDomainDef *def, virDomainUndefineFlagsValues flags, - bool outgoingMigration) + bool migration) { size_t i; @@ -165,7 +165,7 @@ qemuExtDevicesCleanupHost(virQEMUDriver *driver, virDomainTPMDef *tpm = def->tpms[i]; if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) - qemuExtTPMCleanupHost(driver, tpm, flags, outgoingMigration); + qemuExtTPMCleanupHost(driver, tpm, flags, migration); } } @@ -280,7 +280,7 @@ qemuExtDevicesStart(virQEMUDriver *driver, void qemuExtDevicesStop(virQEMUDriver *driver, virDomainObj *vm, - bool outgoingMigration) + bool migration) { virDomainDef *def = vm->def; size_t i; @@ -297,7 +297,7 @@ qemuExtDevicesStop(virQEMUDriver *driver, for (i = 0; i < def->ntpms; i++) { if (def->tpms[i]->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) - qemuExtTPMStop(driver, vm, outgoingMigration); + qemuExtTPMStop(driver, vm, migration); } for (i = 0; i < def->nnets; i++) { diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index d4ac9f395c..36f7fb77a8 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -48,7 +48,7 @@ int qemuExtDevicesPrepareHost(virQEMUDriver *driver, void qemuExtDevicesCleanupHost(virQEMUDriver *driver, virDomainDef *def, virDomainUndefineFlagsValues flags, - bool outgoingMigration) + bool migration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuExtDevicesStart(virQEMUDriver *driver, @@ -59,7 +59,7 @@ int qemuExtDevicesStart(virQEMUDriver *driver, void qemuExtDevicesStop(virQEMUDriver *driver, virDomainObj *vm, - bool outgoingMigration) + bool migration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); bool qemuExtDevicesHasDevice(virDomainDef *def); diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 3e97518c06..0d3be3971a 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -929,7 +929,8 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, * @driver: QEMU driver * @tpm: TPM definition * @flags: flags indicating whether to keep or remove TPM persistent state - * @outgoingMigration: whether cleanup is due to an outgoing migration + * @migration: whether cleanup is due to a successful outgoing or failed + * incoming migration * * Clean up persistent storage for the swtpm. */ @@ -937,14 +938,12 @@ static void qemuTPMEmulatorCleanupHost(virQEMUDriver *driver, virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, - bool outgoingMigration) + bool migration) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - /* Never remove the state in case of outgoing migration with shared - * storage. - */ - if (outgoingMigration && + /* Never remove the state in case of migration with shared storage. */ + if (migration && virFileIsSharedFS(tpm->data.emulator.source_path, cfg->sharedFilesystems) == 1) return; @@ -1315,9 +1314,9 @@ void qemuExtTPMCleanupHost(virQEMUDriver *driver, virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, - bool outgoingMigration) + bool migration) { - qemuTPMEmulatorCleanupHost(driver, tpm, flags, outgoingMigration); + qemuTPMEmulatorCleanupHost(driver, tpm, flags, migration); } @@ -1341,7 +1340,7 @@ qemuExtTPMStart(virQEMUDriver *driver, void qemuExtTPMStop(virQEMUDriver *driver, virDomainObj *vm, - bool outgoingMigration) + bool migration) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *shortName = virDomainDefGetShortName(vm->def); @@ -1351,7 +1350,7 @@ qemuExtTPMStop(virQEMUDriver *driver, return; qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); - if (outgoingMigration && qemuTPMHasSharedStorage(driver, vm->def)) + if (migration && qemuTPMHasSharedStorage(driver, vm->def)) restoreTPMStateLabel = false; if (qemuSecurityRestoreTPMLabels(driver, vm, restoreTPMStateLabel, false) < 0) diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 7096060a2a..37813087cf 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -38,7 +38,7 @@ int qemuExtTPMPrepareHost(virQEMUDriver *driver, void qemuExtTPMCleanupHost(virQEMUDriver *driver, virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, - bool outgoingMigration) + bool migration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuExtTPMStart(virQEMUDriver *driver, @@ -52,7 +52,7 @@ int qemuExtTPMStart(virQEMUDriver *driver, void qemuExtTPMStop(virQEMUDriver *driver, virDomainObj *vm, - bool outgoingMigration) + bool migration) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuExtTPMSetupCgroup(virQEMUDriver *driver, -- 2.49.0
From: Jiri Denemark <jdenemar@redhat.com> When migrating a domain with TPM state on a shared disk, we need to skip TPM cleanup on both ends. So far the code only handled successful migration and skipped the cleanup on the source host. But if the migration failed for some reason, the cleanup would be incorrectly called on the destination host removing the TPM files even though the domain was still running on the source host. https://issues.redhat.com/browse/RHEL-82411 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 7 +++++-- src/qemu/qemu_migration.c | 6 +++--- src/qemu/qemu_process.c | 8 ++------ 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ce949dd07..ebdec2c6fa 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3845,6 +3845,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, const char *auditReason = "shutdown"; unsigned int stopFlags = 0; virObjectEvent *event = NULL; + bool migration; if (vm->def->id != domid) { VIR_DEBUG("Domain %s was restarted, ignoring EOF", @@ -3855,6 +3856,8 @@ processMonitorEOFEvent(virQEMUDriver *driver, if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, true) < 0) return; + migration = vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN; + if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain %p '%s' is not active, ignoring EOF", vm, vm->def->name); @@ -3869,7 +3872,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, auditReason = "failed"; } - if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) { + if (migration) { stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; qemuMigrationDstErrorSave(driver, vm->def->name, qemuMonitorLastError(priv->mon)); @@ -3882,7 +3885,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); endjob: - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(driver, vm, 0, migration); qemuProcessEndStopJob(vm); } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e6056e9abc..bb4d74a65d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3609,7 +3609,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, false); + qemuDomainRemoveInactive(driver, vm, 0, true); } virDomainObjEndAPI(&vm); virErrorRestore(&origErr); @@ -6977,7 +6977,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, } if (!qemuDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM, false); + qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM, true); virErrorRestore(&orig_err); return NULL; @@ -7113,7 +7113,7 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, qemuMigrationJobFinish(vm); if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(driver, vm, 0, true); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 015a98d035..809029cefc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8913,7 +8913,6 @@ void qemuProcessStop(virQEMUDriver *driver, size_t i; g_autofree char *timestamp = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - bool outgoingMigration; VIR_DEBUG("Shutting down vm=%p name=%s id=%d pid=%lld, " "reason=%s, asyncJob=%s, flags=0x%x", @@ -8989,10 +8988,7 @@ void qemuProcessStop(virQEMUDriver *driver, qemuDomainCleanupRun(driver, vm); - outgoingMigration = (flags & VIR_QEMU_PROCESS_STOP_MIGRATED) && - (asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT); - - qemuExtDevicesStop(driver, vm, outgoingMigration); + qemuExtDevicesStop(driver, vm, !!(flags & VIR_QEMU_PROCESS_STOP_MIGRATED)); qemuDBusStop(driver, vm); @@ -9258,7 +9254,7 @@ qemuProcessAutoDestroy(virDomainObj *dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - qemuDomainRemoveInactive(driver, dom, 0, false); + qemuDomainRemoveInactive(driver, dom, 0, !!(stopFlags & VIR_QEMU_PROCESS_STOP_MIGRATED)); qemuProcessEndStopJob(dom); -- 2.49.0
On a Thursday in 2025, Jiri Denemark via Devel wrote:
Jiri Denemark (2): qemu: Rename outgoingMigration parameter in various TPM functions qemu: Properly propagate migration state to TPM cleanup code
src/qemu/qemu_domain.c | 8 ++++---- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 7 +++++-- src/qemu/qemu_extdevice.c | 8 ++++---- src/qemu/qemu_extdevice.h | 4 ++-- src/qemu/qemu_migration.c | 6 +++--- src/qemu/qemu_process.c | 8 ++------ src/qemu/qemu_tpm.c | 19 +++++++++---------- src/qemu/qemu_tpm.h | 4 ++-- 9 files changed, 32 insertions(+), 34 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Jiri Denemark -
Ján Tomko