[libvirt] [PATCH v2 5/7] lock_driver_sanlock: Handle metadata flag gracefully
Daniel P. Berrangé
berrange at redhat.com
Mon Aug 20 14:59:23 UTC 2018
On Fri, Aug 17, 2018 at 10:49:12AM -0400, 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?
>
> 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.
Yeah, this doesn't feel right to me. I think we need to treat the
metadata locking as completely independant of the content locking.
This implies we should have a separate configuration for metadata
locking, where the only valid options are "lockd" or "nop".
eg our available matrix looks like
METADATA
| nop lockd sanlock
---------+--------------------
nop | Y Y N
CONTENT lockd | Y Y N
sanlock | Y Y N
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list