[libvirt] [PATCH v4 16/23] security_manager: Introduce metadata locking APIs

Michal Privoznik mprivozn at redhat.com
Tue Sep 18 15:17:33 UTC 2018


On 09/17/2018 11:14 PM, John Ferlan wrote:
> 
> 
> On 09/10/2018 05:36 AM, Michal Privoznik wrote:
>> Two new APIs are added so that security driver can lock and
>> unlock paths it wishes to touch. These APIs are not for other
>> drivers to call but security drivers (DAC and SELinux). That is
>> the reason these APIs are not exposed through our
>> libvirt_private.syms file.
>>
>> Three interesting things happen in this commit. The first is the
>> global @lockManagerMutex. Unfortunately, this has to exist so that
>> there is only on thread talking to virtlockd at a time. If there
> 
> s/on/one
> 
>> were more threads and one of them closed the connection
>> prematurely, it would cause virtlockd killing libvirtd. Instead
>> of complicated code that would handle that, let's have a mutex
>> and keep the code simple.
>>
>> The second interesting thing is keeping connection open between
>> lock and unlock API calls. This is achieved by duplicating client
>> FD and keeping it open until unlock is called. This trick is used
>> by regular disk content locking code when the FD is leaked to
>> qemu.
>>
>> Finally, the third thing is polling implemented at client side.
>> Since virtlockd has only one thread that handles locking
>> requests, all it can do is either acquire lock or error out.
>> Therefore, the polling has to be implemented in client. The
>> polling is capped at 60 second timeout, which should be plenty
>> since the metadata lock is held only for a fraction of a second.
> 
> I smell a configurable timeout instead of 60 seconds, but that's mainly
> because my senses are more acutely aware of such configurable timeout
> changes given some recent on list patches for client lock timeouts when
> (as I assume) there is a client gathering stats that takes a long time.

Hopefully not. This indeed is a lock that guards
chown()/setfilecon_raw(). I wouldn't expect them to took very long even
if there were dozens of daemons fighting over the lock.

Michal




More information about the libvir-list mailing list