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

Re: [libvirt] [PATCH 05/10] qemu_security: Use more transactions



On Fri, Jan 20, 2017 at 10:42:45AM +0100, Michal Privoznik wrote:
The idea is to move all the seclabel setting to security driver.
Having the relabel code spread all over the place looks very
messy.

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

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 13d99cdbd..9d1a87971 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -90,14 +90,26 @@ qemuSecuritySetDiskLabel(virQEMUDriverPtr driver,
                         virDomainObjPtr vm,
                         virDomainDiskDefPtr disk)
{
-    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) {
-        /* Already handled by namespace code. */
-        return 0;
-    }
+    int ret = -1;

-    return virSecurityManagerSetDiskLabel(driver->securityManager,
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
+        virSecurityManagerTransactionStart(driver->securityManager) < 0)
+        goto cleanup;
+
+    if (virSecurityManagerSetDiskLabel(driver->securityManager,
                                          vm->def,
-                                          disk);
+                                          disk) < 0)

indentation

+        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;
}


So much code duplication.  Wasn't there an idea to have a callback in
the security manager that would be called before/after the labelling?
Since this is only for a disk and hostdev, let's leave it like this, I
guess, but I'm expecting this much code duplication to bite us back in a
while.  None of my ideas for how to make this better are finalized, but
I have some, if you want to discuss.

ACK for the sake of fixing stuff if you fix the indentation as well.

Attachment: signature.asc
Description: Digital signature


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