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

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(a)redhat.com&gt; > --- > 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? Something doesn't feel right about that - giving the impression that it's supported and the consumer is protected, but when push comes to shove they aren't. I'd be inclined to believe that we may want to do nothing with/for sanlock allowing the virCheckFlags above take care of the consumer.