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

Trond Myklebust
2003-06-22T07:15:13Z

>>>>> " " == Frank Cusack <fcusack@fcusack.com> 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