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

Re: [libvirt] [PATCH 04/10] qemuSecurityRestoreAllLabel: Don't use transactions



On Fri, Jan 20, 2017 at 10:42:44AM +0100, Michal Privoznik wrote:
Because of the nature of security driver transactions, it is
impossible to use them properly. The thing is, transactions enter
the domain namespace and commit all the seclabel changes.
However, in RestoreAllLabel() this is impossible - the qemu
process, the only process running in the namespace, is gone. And
thus is the namespace. Therefore we shouldn't use the transactions
as there is no namespace to enter.

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


ACK, no need to duplicate the same information in the commit message and
the comment below IMHO.

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 544feeb4a..13d99cdbd 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -73,22 +73,15 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver,
                            virDomainObjPtr vm,
                            bool migrated)
{
-    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
-        virSecurityManagerTransactionStart(driver->securityManager) < 0)
-        goto cleanup;
-
-    if (virSecurityManagerRestoreAllLabel(driver->securityManager,
-                                          vm->def,
-                                          migrated) < 0)
-        goto cleanup;
-
-    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
-        virSecurityManagerTransactionCommit(driver->securityManager,
-                                            vm->pid) < 0)
-        goto cleanup;
-
- cleanup:
-    virSecurityManagerTransactionAbort(driver->securityManager);
+    /* In contrast to qemuSecuritySetAllLabel, do not use
+     * secdriver transactions here. This function is called from
+     * qemuProcessStop() which is meant to do cleanup after qemu
+     * process died. If it did do, the namespace is gone as qemu
+     * was the only process running there. We would not succeed
+     * in entering the namespace then. */
+    virSecurityManagerRestoreAllLabel(driver->securityManager,
+                                      vm->def,
+                                      migrated);
}


--
2.11.0

--
libvir-list mailing list
libvir-list redhat com
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature


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