On Thu, 2020-07-09 at 13:44 +0200, Martin Kletzander wrote: > On Thu, Jul 09, 2020 at 12:50:38PM +0200, Andrea Bolognani wrote: > > it seems to me that this change entirely flipped the semantics of > > the lock. > > Yep, you're right. When you have a lock that has boolean as a parameter I think > the boolean should indicate whether the lock is writable, but maybe the proper > and most readable solution would be virFileFlockShareable() and > virFileFlockExclusive() to avoid any misconception in the future[2]. Given the > fact that there are no (and most probably won't be) any other users of flock(2) > and given the fact that resctrl is also pretty niche feature, I don't have any > preference. Also I feel like after some time I'm a little bit rusty with C and > the libvirt codebase (and most importantly the reasoning and decisions) has > changed a bit, so I'll leave the decision on how to deal with that on someone > else. I'm happy to provide the patches when clear decision is made. Either virFileFlockExclusive(fd) virFileFlockShared(fd) or virFileFlock(fd, VIR_FILE_FLOCK_EXCLUSIVE) virFileFlock(fd, VIR_FILE_FLOCK_SHARED) would work. I like the latter better because it's closer to the original flock(), which it ultimately acts as a very thin wrapper around of. I'm actually unclear why we'd have the last argument in the first place: personally I'd just use virFileFlock(fd, VIR_FILE_FLOCK_UNLOCK) and keep things as unambiguous as they can be. This is all bikeshedding, of course: what actually matters is making that lock exclusive once again :) -- Andrea Bolognani / Red Hat / Virtualization
