6:22 a.m.
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
6:22 a.m.
New subject: [PATCH 1/2] qemu: Rename outgoingMigration parameter in various TPM functions
From: Jiri Denemark <jdenemar(a)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(a)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
6:22 a.m.
New subject: [PATCH 2/2] qemu: Properly propagate migration state to TPM cleanup code
From: Jiri Denemark <jdenemar(a)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(a)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
7:04 a.m.
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(a)redhat.com>
Jano
40
days inactive
40
days old
3 comments
2 participants
participants (2)
-
Jiri Denemark -
Ján Tomko