Re: [PATCH] resctrl: Do not open directory for writing

On 7/9/20 6:30 PM, Andrea Bolognani wrote: > 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 :) > Just realized that for exclusive (aka write) lock, the FD must be opened for writing (working on patches for the following report [1] and been experimenting a bit and that's what I'm seeing).

1: https://www.redhat.com/archives/libvir-list/2020-July/msg00451.html Michal