From 095f042ed68b0162c784ad5d0eb30c289d2fcd90 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 15 Dec 2016 16:47:15 +0100
Subject: [PATCH] qemu: Use transactions from security driver

So far if qemu is spawned under separate mount namespace in order
to relabel everything it needs an access to the security driver
to run in that namespace too. This has a very nasty down side -
it is being run in a separate process, so any internal state
transition is NOT reflected in the daemon. This can lead to many
sleepless nights. Therefore, use the transaction APIs so that
libvirt developers can sleep tight again.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_security.c | 106 ++++++++++++---------------------------
 1 file changed, 33 insertions(+), 73 deletions(-)

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 9ab91e9f2e..544feeb4ae 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -40,66 +40,31 @@ struct qemuSecuritySetRestoreAllLabelData {
 };
 
 
-static int
-qemuSecuritySetRestoreAllLabelHelper(pid_t pid,
-                                     void *opaque)
-{
-    struct qemuSecuritySetRestoreAllLabelData *data = opaque;
-
-    virSecurityManagerPostFork(data->driver->securityManager);
-
-    if (data->set) {
-        VIR_DEBUG("Setting up security labels inside namespace pid=%lld",
-                  (long long) pid);
-        if (virSecurityManagerSetAllLabel(data->driver->securityManager,
-                                          data->vm->def,
-                                          data->stdin_path) < 0)
-            return -1;
-    } else {
-        VIR_DEBUG("Restoring security labels inside namespace pid=%lld",
-                  (long long) pid);
-        if (virSecurityManagerRestoreAllLabel(data->driver->securityManager,
-                                              data->vm->def,
-                                              data->migrated) < 0)
-            return -1;
-    }
-
-    return 0;
-}
-
-
 int
 qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
                         virDomainObjPtr vm,
                         const char *stdin_path)
 {
-    struct qemuSecuritySetRestoreAllLabelData data;
-
-    memset(&data, 0, sizeof(data));
-
-    data.set = true;
-    data.driver = driver;
-    data.vm = vm;
-    data.stdin_path = stdin_path;
-
-    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) {
-        if (virSecurityManagerPreFork(driver->securityManager) < 0)
-            return -1;
-        if (virProcessRunInMountNamespace(vm->pid,
-                                          qemuSecuritySetRestoreAllLabelHelper,
-                                          &data) < 0) {
-            virSecurityManagerPostFork(driver->securityManager);
-            return -1;
-        }
-        virSecurityManagerPostFork(driver->securityManager);
-
-    } else {
-        if (virSecurityManagerSetAllLabel(driver->securityManager,
-                                          vm->def,
-                                          stdin_path) < 0)
-            return -1;
-    }
-    return 0;
+    int ret = -1;
+
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+        virSecurityManagerTransactionStart(driver->securityManager) < 0)
+        goto cleanup;
+
+    if (virSecurityManagerSetAllLabel(driver->securityManager,
+                                      vm->def,
+                                      stdin_path) < 0)
+        goto cleanup;
+
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+        virSecurityManagerTransactionCommit(driver->securityManager,
+                                            vm->pid) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    virSecurityManagerTransactionAbort(driver->securityManager);
+    return ret;
 }
 
 
@@ -108,27 +73,22 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
                             virDomainObjPtr vm,
                             bool migrated)
 {
-    struct qemuSecuritySetRestoreAllLabelData data;
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+        virSecurityManagerTransactionStart(driver->securityManager) < 0)
+        goto cleanup;
 
-    memset(&data, 0, sizeof(data));
+    if (virSecurityManagerRestoreAllLabel(driver->securityManager,
+                                          vm->def,
+                                          migrated) < 0)
+        goto cleanup;
 
-    data.driver = driver;
-    data.vm = vm;
-    data.migrated = migrated;
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+        virSecurityManagerTransactionCommit(driver->securityManager,
+                                            vm->pid) < 0)
+        goto cleanup;
 
-    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) {
-        if (virSecurityManagerPreFork(driver->securityManager) < 0)
-            return;
-
-        virProcessRunInMountNamespace(vm->pid,
-                                      qemuSecuritySetRestoreAllLabelHelper,
-                                      &data);
-        virSecurityManagerPostFork(driver->securityManager);
-    } else {
-        virSecurityManagerRestoreAllLabel(driver->securityManager,
-                                          vm->def,
-                                          migrated);
-    }
+ cleanup:
+    virSecurityManagerTransactionAbort(driver->securityManager);
 }
 
 
-- 
GitLab