Re: [PATCH] nfs_unlink() again, and trivial nfs_fhget

[Trond Myklebust]

>>>>> " " == Frank Cusack <[e-mail]> writes:

     > It's to prevent RENAME of silly-renamed files.  Doing so in VFS
     > is a one-liner, and I agree that the VFS should be as clean as
     > possible, but let's face it, the VFS *must* have specific fs
     > knowledge.  eg, the ALWAYS_REVAL (something like that) patch
     > you recently submitted is just to treat NFS differently than
     > other fs's.  Just because the flag doesn't have "NFS" in it
     > doesn't make it generic.  (And so I repeat my earlier
     > suggestion that might make the change more palatable: rename
     > the NFSFS_RENAMED flag to DONT_UNLINK.)

On the contrary: The VFS should *avoid* specific fs knowledge
whereever possible.

In this case:

  - we *can* do this test in nfs_rename() itself without needing any
    extra VFS support.
  - No other filesystems have expressed a need for such a flag, so
    we're hardly covering a common need.

Those are 2 good reasons for doing the modification in the NFS code.

And no - NFSFS_RENAMED, and DONT_UNLINK are *not* the same concept.
Renaming sillyrenamed files would be quite acceptable as long as you
don't allow *cross-directory* renames. All other cases are fixable...

     > I hate to keep following up myself, but I forgot one point I
     > had in mind: other code in may_delete() is already fs-specific.
     >
     > IS_APPEND
     > IS_IMMUTABLE
     > check_sticky
     >
     > These things aren't generic, they require specific support from
     > the fs.

For starters, check_sticky has no business being in may_delete(), as
it breaks NFS (i.e. it should really be part of vfs_permission()).
All these comparisons of our local uid/gids or capabilities with
remote object uid/gids are broken.

That said, there is a huge leap between the 2 assertions that "some
special cases appear in the VFS." to "all special case must be done in
the VFS".

Cheers,
  Trond