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

[Martin Kletzander]

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 at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170202/4614e468/attachment-0001.sig>