On Tue, Aug 14, 2018 at 01:19:43PM +0200, Michal Privoznik wrote:
> Fortunately, we have qemu wrappers so it's sufficient to put
> lock/unlock call only there.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/qemu/qemu_security.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 107 insertions(+)
>
> diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
> index af3be42854..527563947c 100644
> --- a/src/qemu/qemu_security.c
> +++ b/src/qemu/qemu_security.c
> @@ -26,6 +26,7 @@
> #include "qemu_domain.h"
> #include "qemu_security.h"
> #include "virlog.h"
> +#include "locking/domain_lock.h"
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
> @@ -39,6 +40,12 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
> {
> int ret = -1;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> + bool locked = false;
> +
> + if (virDomainLockMetadataLock(driver->lockManager, vm) < 0)
> + goto cleanup;
> +
> + locked = true;
>
> if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
> virSecurityManagerTransactionStart(driver->securityManager) < 0)
> @@ -55,9 +62,17 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver,
> vm->pid) < 0)
> goto cleanup;
>
> + locked = false;
> +
> + if (virDomainLockMetadataUnlock(driver->lockManager, vm) < 0)
> + goto cleanup;
> +
> ret = 0;
> cleanup:
> virSecurityManagerTransactionAbort(driver->securityManager);
> + if (locked &&
> + virDomainLockMetadataUnlock(driver->lockManager, vm) < 0)
> + VIR_WARN("unable to release metadata lock");
> return ret;
> }
I'm wondering if this is really the right level to plug in the metadata
locking ? What about if we just pass a virLockManagerPtr to the security
drivers and let them lock each resource at the time they need to modify
its metadata. This will trivially guarantee that we always lock the exact
set of files that we'll be changing metadata on.
eg in SELinux driver the virSecuritySELinuxSetFileconHelper method
would directly call virLockManagerAcquire & virLockManagerRelease,
avoiding the entire virDomainLock abstraction which was really
focused around managing the content locks around lifecycle state
changes.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
