Re: [PATCH] VFS: Unlink should revoke all outstanding leases on file

[Jamie Lokier <jamie@shareable.org>]

J. Bruce Fields wrote:
> On Fri, May 14, 2010 at 02:31:12PM -0400, Trond Myklebust wrote:
> > On Fri, 2010-05-14 at 13:59 -0400, Trond Myklebust wrote: 
> > > Note that the server should also recall the delegation if someone
> > > attempts to violate the guarantees that are listed in section 9.4: Open
> > > Delegation
> > > 
> > >    When a client has a read open delegation, it may not make any changes
> > >    to the contents or attributes of the file but it is assured that no
> > >    other client may do so.  When a client has a write open delegation,
> > >    it may modify the file data since no other client will be accessing
> > >    the file's data.  The client holding a write delegation may only
> > >    affect file attributes which are intimately connected with the file
> > >    data:  size, time_modify, change.
> > > 
> > > IOW: even if you hold a write delegation you are not allowed to change
> > > the file mode bits, owner, group or acls...
> > 
> > ...or the nlink value. So technically, we should also recall the
> > delegation when someone creates or deletes a hard link. I think I need
> > to remind Tom that he should add that to the RFC3530bis draft...
> 
> Yep.  And fixing all these cases is required before our the server's
> NFSv4 server is ready for much of anything.
> 
> I'm not sure ading break_lease() to may_delete() is right, but maybe
> it's better than nothing.
> 
> One problem is that there's a race: nothing I can see stops anyone from
> getting another lease after may_delete() but before the delete happens.

Presumably the intent is that the NFSv4 REMOVE request _acquires_ the
lease, and releases it after the delete is done.

Same pattern with renames, attribute changes, etc.

Imho it would all be much tidier if leases had the same set of flag
bits as inotify/dnotify, to say what changes they block.  (Maybe the
flags would be slightly different - a detail).

All operations (read, write, open, link, rename, etc.) would follow a
pattern like this pseudo-code:

   do_write(file)
   {
       err = lease_acquire(file, IN_MODIFY);
       if (err < 0)
           return err;

       /* Do the modifying. */

       lease_release_and_inotify_event(file, IN_MODIFY);
   }

I think that would provide the semantics needed by NFS, Samba, also
fanotify for free, and more or less any kind of userspace caching,
coherent or not.  It's clean and orthogonal.  (Good value for money isn't it?)

The nlink value is missing from inotify (or "linked from" if you look
at it differently), but that's a problem needing to be fixed anyway.

-- Jamie