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@xxxxxxxxxx>
---
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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
