[PATCH 0/3] qemu: Fix setting TPM state seclabels wrt save/restore
*** BLURB HERE *** Michal Prívozník (3): qemuProcessStop: Fix detection of outgoing migration for external devices qemuExtTPMStop: Restore TPM state label more often qemuProcessLaunch: Tighten rules for external devices wrt incoming migration src/qemu/qemu_process.c | 11 +++++++++-- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) -- 2.39.1
When cleaning up host in qemuProcessStop(), our external helper processes (e.g. swtpm) want to know whether the domain is being migrated out or not (so that they restore seclabels on a device state that's on a shared storage). This fact is reflected in the @outgoingMigration variable which is set to true if asyncJob is anything but VIR_ASYNC_JOB_MIGRATION_IN. Well, we have a specific job for outgoing migration (VIR_ASYNC_JOB_MIGRATION_OUT) and thus we should check for that. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4afd12e3fa..6718915293 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8398,7 +8398,7 @@ void qemuProcessStop(virQEMUDriver *driver, qemuDomainCleanupRun(driver, vm); outgoingMigration = (flags & VIR_QEMU_PROCESS_STOP_MIGRATED) && - (asyncJob != VIR_ASYNC_JOB_MIGRATION_IN); + (asyncJob == VIR_ASYNC_JOB_MIGRATION_OUT); qemuExtDevicesStop(driver, vm, outgoingMigration); qemuDBusStop(driver, vm); -- 2.39.1
When stopping swtpm we can restore the label either on just the swtpm's domain specific logfile (/var/log/swtpm/libvirt/qemu/...), or on the logfile and the state too (/var/lib/libvirt/swtpm/...). The deciding factor is whether the guest is stopped because of outgoing migration OR the state is on a shared filesystem. But this is not correct condition, because for instance saving the guest into a file (virsh save) is also an outgoing migration. Alternatively, when the swtpm state is stored on a shared filesystem, but the guest is destroyed (virsh destroy), i.e. stopped because of different reason than migration, we want to restore the seclabels. The correct condition is: skip restoring the state on outgoing migration AND shared filesystem. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2161557 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_tpm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index b2748eb6a4..5831ffc32e 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -1142,7 +1142,7 @@ qemuExtTPMStop(virQEMUDriver *driver, return; qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); - if (outgoingMigration || qemuTPMHasSharedStorage(vm->def)) + if (outgoingMigration && qemuTPMHasSharedStorage(vm->def)) restoreTPMStateLabel = false; if (qemuSecurityRestoreTPMLabels(driver, vm, restoreTPMStateLabel) < 0) -- 2.39.1
When starting a guest, helper processes are started first. But they need a bit of special handling. Just consider a regular cold boot and an incoming migration. For instance, in case of swtpm with its state on a shared volume, we want to set label on the state for the cold boot case, but don't want to touch the label in case of incoming migration (because the source very specifically did not restore it either). Until now, these two cases were differentiated by testing @incoming against NULL. And while that makes sense for other aspects of domain startup, for external devices we need a bit more, because a restore from a save file is also 'incoming migration'. Now, there is a difference between regular migration and restore from a save file. In the former case we do not want to set seclabels in the save state. BUT, in the latter case we do need to set them, because the code that saves the machine restored seclabels. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6718915293..f9789650bc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7621,6 +7621,7 @@ qemuProcessLaunch(virConnectPtr conn, size_t nnicindexes = 0; g_autofree int *nicindexes = NULL; unsigned long long maxMemLock = 0; + bool incomingMigrationExtDevices = false; VIR_DEBUG("conn=%p driver=%p vm=%p name=%s id=%d asyncJob=%d " "incoming.uri=%s " @@ -7675,7 +7676,13 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuDomainSchedCoreStart(cfg, vm) < 0) goto cleanup; - if (qemuExtDevicesStart(driver, vm, incoming != NULL) < 0) + /* For external devices the rules of incoming migration are a bit stricter, + * than plain @incoming != NULL. They need to differentiate between + * incoming migration and restore from a save file. */ + incomingMigrationExtDevices = incoming && + vmop == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START; + + if (qemuExtDevicesStart(driver, vm, incomingMigrationExtDevices) < 0) goto cleanup; if (!(cmd = qemuBuildCommandLine(vm, -- 2.39.1
On 1/27/23 16:05, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (3): qemuProcessStop: Fix detection of outgoing migration for external devices qemuExtTPMStop: Restore TPM state label more often qemuProcessLaunch: Tighten rules for external devices wrt incoming migration
src/qemu/qemu_process.c | 11 +++++++++-- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-)
Oops, I forgot to add BZ link: https://bugzilla.redhat.com/show_bug.cgi?id=2161557 and our QE did some preliminary testing and these patches solve the issue (see comment 3). Michal
On a Friday in 2023, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (3): qemuProcessStop: Fix detection of outgoing migration for external devices qemuExtTPMStop: Restore TPM state label more often qemuProcessLaunch: Tighten rules for external devices wrt incoming migration
src/qemu/qemu_process.c | 11 +++++++++-- src/qemu/qemu_tpm.c | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Ján Tomko -
Michal Privoznik -
Michal Prívozník