Re: [libvirt] [PATCH v2 7/7] qemu_security: Lock metadata while relabelling

On 08/23/2018 06:14 PM, Michal Privoznik wrote: > On 08/23/2018 05:51 PM, Daniel P. Berrangé wrote: >> On Thu, Aug 23, 2018 at 05:45:42PM +0200, Michal Privoznik wrote: >>> On 08/23/2018 05:01 PM, Daniel P. Berrangé wrote: >>>> On Thu, Aug 23, 2018 at 04:54:01PM +0200, Michal Privoznik wrote: >>>>> On 08/20/2018 07:17 PM, Michal Prívozník wrote: >>>>>> On 08/20/2018 05:16 PM, Daniel P. Berrangé wrote: >>>>>>> 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(a)redhat.com&gt; >>>>>>>> --- >>>>>>>> src/qemu/qemu_security.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>>> 1 file changed, 107 insertions(+) >>>>>>>> >>>>> >>>>> > > >> >> Oh right, yes, that kills the idea of using a WAIT flag for lockspaces, >> unless we want to introduce threads ingto virtlockd, but we can't do >> that because Linux has totally fubard locking across execve() :-( Actually, it's problem with open() + close() not execve(). And no dumy implementation (as I'm suggesting below) will not help us with that.

> But we need WAIT. I guess we do not want to do: > > lockForMetadata(const char *path) { > int retries; > > while (retries) { > rc = try_lock(path, wait=false); > > if (!rc) break; > if (errno = EAGAIN && retries--) {sleep(.1); continue;} > > errorOut(); > } > } > > However, if we make sure that virtlockd never forks() nor execs() we > have nothing to fear about. Or am I missing something? And to be 100% > sure we can always provide dummy impl for fork() + execve() in > src/locking/lock_daemon.c which fails everytime. I work around this by putting a mutex into secdriver so that only one thread can do lock(). The others have to wait until the thread unlocks() the path. This way we leave virtlockd with just one thread. In my testing everything works like charm.