[PATCH] resctrl: Do not open directory for writing

Martin Kletzander mkletzan at redhat.com
Fri Jul 10 13:13:34 UTC 2020


On Fri, Jul 10, 2020 at 10:47:22AM +0200, Michal Privoznik wrote:
>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).
>

Good point, but luckily not related to flock(2).

>1: https://www.redhat.com/archives/libvir-list/2020-July/msg00451.html
>
>Michal
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200710/30c9faef/attachment-0001.sig>


More information about the libvir-list mailing list