[PATCH v6 13/13] qemu: Don't lock TPM state directory for incoming migration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



We will never be able to acquire the lock on the destination
host because the swtpm process that's running on the source
host is still holding on to it.

Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
---
 src/qemu/qemu_security.c | 10 ++++++----
 src/qemu/qemu_security.h |  6 ++++--
 src/qemu/qemu_tpm.c      | 20 +++++++++++++++++---
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 996c95acc0..11af9100fb 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -537,7 +537,8 @@ qemuSecurityRestoreNetdevLabel(virQEMUDriver *driver,
 int
 qemuSecuritySetTPMLabels(virQEMUDriver *driver,
                          virDomainObj *vm,
-                         bool setTPMStateLabel)
+                         bool setTPMStateLabel,
+                         bool attemptLocking)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
@@ -552,7 +553,7 @@ qemuSecuritySetTPMLabels(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            -1, priv->rememberOwner) < 0)
+                                            -1, priv->rememberOwner && attemptLocking) < 0)
         goto cleanup;
 
     ret = 0;
@@ -565,7 +566,8 @@ qemuSecuritySetTPMLabels(virQEMUDriver *driver,
 int
 qemuSecurityRestoreTPMLabels(virQEMUDriver *driver,
                              virDomainObj *vm,
-                             bool restoreTPMStateLabel)
+                             bool restoreTPMStateLabel,
+                             bool attemptLocking)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
@@ -580,7 +582,7 @@ qemuSecurityRestoreTPMLabels(virQEMUDriver *driver,
         goto cleanup;
 
     if (virSecurityManagerTransactionCommit(driver->securityManager,
-                                            -1, priv->rememberOwner) < 0)
+                                            -1, priv->rememberOwner && attemptLocking) < 0)
         goto cleanup;
 
     ret = 0;
diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index 32f29bc210..4e3186a2af 100644
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -87,11 +87,13 @@ int qemuSecurityRestoreNetdevLabel(virQEMUDriver *driver,
 
 int qemuSecuritySetTPMLabels(virQEMUDriver *driver,
                              virDomainObj *vm,
-                             bool setTPMStateLabel);
+                             bool setTPMStateLabel,
+                             bool attemptLocking);
 
 int qemuSecurityRestoreTPMLabels(virQEMUDriver *driver,
                                  virDomainObj *vm,
-                                 bool restoreTPMStateLabel);
+                                 bool restoreTPMStateLabel,
+                                 bool attemptLocking);
 
 int qemuSecuritySetSavedStateLabel(virQEMUDriver *driver,
                                    virDomainObj *vm,
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 55927b4582..6ab4fc9d01 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -934,6 +934,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
     virTimeBackOffVar timebackoff;
     const unsigned long long timeout = 1000; /* ms */
     pid_t pid = -1;
+    bool attemptLocking = true;
 
     cfg = virQEMUDriverGetConfig(driver);
 
@@ -959,7 +960,20 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
     virCommandSetPidFile(cmd, pidfile);
     virCommandSetErrorFD(cmd, &errfd);
 
-    if (qemuSecuritySetTPMLabels(driver, vm, true) < 0)
+    if (incomingMigration && qemuTPMHasSharedStorage(driver, vm->def)) {
+        /* If the TPM is being migrated over shared storage, we can't
+         * lock the files before labeling them: the source swtpm
+         * process is still holding a lock on some of them, and it
+         * will only release it after negotiation with the target
+         * swtpm process, which we can't start until labeling has
+         * been performed.
+         *
+         * So we run with fewer guarantees in this specific, narrow
+         * scenario in order to make the migration possible at all */
+        attemptLocking = false;
+    }
+
+    if (qemuSecuritySetTPMLabels(driver, vm, true, attemptLocking) < 0)
         return -1;
 
     if (qemuSecurityCommandRun(driver, vm, cmd, cfg->swtpm_user,
@@ -1008,7 +1022,7 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
         virProcessKillPainfully(pid, true);
     if (pidfile)
         unlink(pidfile);
-    qemuSecurityRestoreTPMLabels(driver, vm, true);
+    qemuSecurityRestoreTPMLabels(driver, vm, true, attemptLocking);
     return -1;
 }
 
@@ -1144,7 +1158,7 @@ qemuExtTPMStop(virQEMUDriver *driver,
     if (outgoingMigration && qemuTPMHasSharedStorage(driver, vm->def))
         restoreTPMStateLabel = false;
 
-    if (qemuSecurityRestoreTPMLabels(driver, vm, restoreTPMStateLabel) < 0)
+    if (qemuSecurityRestoreTPMLabels(driver, vm, restoreTPMStateLabel, true) < 0)
         VIR_WARN("Unable to restore labels on TPM state and/or log file");
 }
 
-- 
2.46.0




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux