[libvirt] [PATCH v2 5/7] lock_driver_sanlock: Handle metadata flag gracefully

[Michal Prívozník]

On 08/20/2018 09:25 AM, Michal Prívozník wrote:
> On 08/17/2018 04:49 PM, John Ferlan wrote:
>>
>>
>> On 08/14/2018 07:19 AM, Michal Privoznik wrote:
>>> No real support implemented here. But hey, at least we will not
>>> fail.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>  src/locking/lock_driver_sanlock.c | 25 ++++++++++++++++++-------
>>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
>>> index 3e5f0e37b0..c1996fb937 100644
>>> --- a/src/locking/lock_driver_sanlock.c
>>> +++ b/src/locking/lock_driver_sanlock.c
>>> @@ -791,7 +791,8 @@ static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
>>>      virLockManagerSanlockPrivatePtr priv = lock->privateData;
>>>  
>>>      virCheckFlags(VIR_LOCK_MANAGER_RESOURCE_READONLY |
>>> -                  VIR_LOCK_MANAGER_RESOURCE_SHARED, -1);
>>> +                  VIR_LOCK_MANAGER_RESOURCE_SHARED |
>>> +                  VIR_LOCK_MANAGER_RESOURCE_METADATA, -1);
>>>  
>>>      if (priv->res_count == SANLK_MAX_RESOURCES) {
>>>          virReportError(VIR_ERR_INTERNAL_ERROR,
>>> @@ -804,6 +805,11 @@ static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
>>>      if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY)
>>>          return 0;
>>>  
>>> +    /* No metadata locking support for now.
>>> +     * TODO: implement it. */
>>> +    if (flags & VIR_LOCK_MANAGER_RESOURCE_METADATA)
>>> +        return 0;
>>> +
>>
>> Doesn't this give someone the false impression that their resource is
>> locked if they choose METADATA?
> 
> Okay, I'll provide some implementation. Even though sanlock is not
> really my cup of coffee :-)

Actually, sanlock is doomed. It allows us to set the initial offset
(with posing some weird restrictions on it) and it doesn't allow us to
specify length we want to lock (it looks like it locks whole disk
sector). Therefore we can't lock offsets 0 and 1 independently. D'oh!
Maybe we could lock offsets 0 and 1MB but I'm unsure how sanlock behaves
when offset is outside of the file (we can't safely assume that every
file we will try to lock is at least 1MB big, can we? No we can't!
OVMF_VARS is only 128KB long).

https://linux.die.net/man/8/sanlock   and scroll down to offsets.

Therefore I rather stick with my dummy implementation and have somebody
else look at this. Somebody who understands how sanlock works.

Michal