Re: The file_lock_operatoins.lock API seems to be a BAD API.

["J. Bruce Fields" <bfields@fieldses.org>]

On Fri, May 29, 2020 at 11:01:59AM +1000, NeilBrown wrote:
> On Thu, May 28 2020, J. Bruce Fields wrote:
> 
> > On Thu, May 28, 2020 at 04:14:44PM +1000, NeilBrown wrote:
> >> I don't think we should just fix all those bugs in those filesystems.
> >> I think that F_UNLCK should *always* remove the lock/lease.
> >> I imaging this happening by  *always* calling posix_lock_file() (or
> >> similar) in the unlock case - after calling f_op->lock() first if that
> >> is appropriate.
> >> 
> >> What do people think?  It there on obvious reason that is a non-starter?
> >
> > Isn't NFS unlock like close, in that it may be our only chance to return
> > IO errors?
> 
> Is it?  fcntl() isn't documented as returning ENOSPC, EDQUOT or EIO.

I'm probably wrong.  Writes have to be acknowledged before we return
from unlock, but that doesn't mean that's our only chance to return any
errors we find at that point.

> > But I guess you're not saying that unlock can't return errors, just that
> > it should always remove the lock whether it returns 0 or not.
> 
> No I wasn't, but I might.
> One approach that I was considering for making the API more robust was
> to propose a separate "unlock" file_operation.  All unlock requests
> would go through this, and it would have a 'void' return type.
> Would that be sufficient to encourage programmers to handle their own
> errors and not think they can punt?

Maybe I regret bringing up errors....  As you say, the important thing
is making sure the lock's cleaned up, and doing that in common code
sounds like the way to guarantee it.

> But yes - even if unlock returns an error, it should (locally) remove
> any locks - much as 'close()' will always close the fd (if it was
> actually open) even if it reports an error.

That makes sense to me.

--b.