On Fri, Jun 16, 2000 at 04:48:58PM +1000, Neil Brown wrote:
> On Thursday June 15, vugluskr@unicorn.math.spbu.ru wrote:
> > Folks, it seems that now I have something that can be considered as a very
> > first attempt to implement proper file handles on a per-fs basis.
> >
> > The main idea behind all this stuff is that it should be up to each filesystem to
> > decide whether it wants to be exportable or not. And if yes -- provide
> > strong invariants for external representation of its objects to the knfsd
> > module via two (hope!) well defined procedures:
> > fhandle_to_dentry()
> > dentry_to_fhandle()
> >
> > As an example I took ext2 and hacked around namei.c. Take a look at it and
> > *please* express what do you think about the approach in general ( not
> > the coding style ;).
> >
> > Now for the implementation details:
> >
> > 0. include/linux/fs.h:
> > just a few declarations.
> > 1. knfsd part:
> > 1.1. export.c: as I've already said an fs is said to be exportable iff
> > it provides fhandle_to_dentry & dentry_to_fhandle.
> > 1.2. nfsfh.c: a lot of changes and not only in interface but also
> > in how subtrees are being checked. Trond, please take a look at
> > this beast ( it is from your forest ;). I'm asking because this
> > part, I mean knfsd, is pretty independent and should be purified
> > before we will start hacking filesystems.
>
> Probably more my neck of the woods than Trond's, though I'm sure he
> has valuable opinions
Well, sorry for quite a long delay. But even now I'm a bit too far away
from the regular linux lab machines ( frankly, from terminals too ). Well,
let's get back to the fhandles and stuff. ;)
> You seem to have completely missed the point of the new structure of
> the file handle.
I don't think so. :-~
> The idea was that the tail end of the filehandle was "owned" by the
> file system.
What idea ? From your proposal ? If so than see comments at the very
bottom of this letter.
> You pass a "char *" (or void*) to the filesystem, and it
> extracts what it wants, or fills in what it has as appropriate.
Remember last time I was trying to get cookies on board ? I can't say
it was a painless attempt. Well, from my side the interface you proposed
is too complicated and twisted. And the only reason that could ever
force me to implement something like that is an example of a *reasonable*
filesystem that would be "unexportable" via my fhandles.
> So extracting two longs, giving them names, and passing them to the
> filesystem just isn't the right idea. The only reason that the
> extracting-two-longs code is in there is for back-compatibility.
Why ? Just because it doesn't brew coffee for you ? Again: I see no
point in using opaque data structures where you can survive without it.
Please, if you have any reasonable counterexamples of where my scheme
wouldn't work *at all* -- show them. I can agree that sizeof(fhandle)
is too small and it is unacceptable for some filesystems, but, well,
show them up.
> The filesystem might want to store 64 bit numbers, or several
> different numbers which are each possible keys (in the case of a
> filesystem not having a guarnteed key). The knfsd layer really
> shouldn't care.
I can say that you are very knfsd-oriented. May be that's the main
driving force behind your critique.
> Also, ripping outv the "security" stuff is a bit unfriendly.
> I think
> it does have a place, even if you don't think it gives much extra
> security, and quoting RFC[1094|1813] isn't really the point. These,
> particularly 1094, are very bare-bones protocol descriptions. There
> is a lot that they don't say about implementation that can still be
> very important.
What does it mean "very important" ? "very important" for whom ?
Damn! We have *standards* some of them are very official some of them
isn't. But after all, we all know that RFCs _do_ dominate the world, don't
we? Moreover, as I've already said an existing way of doing security checks
*is* implementable. See technical comments bellow.
> You said in a separate Email that:
> But my strong opinion here is that this kind of trickery should stay on
> a protocol level and it should not be integrated into VFS filehandles.
>
> My perspective is:
> The need is "given a file(handle), find the parent". The NFSD layer
> can only do this by grabbing a filehandle for the directory as
> well.
Why in hell we need parents for plain files ? It is a waste of time,
it is a waste of memory. It is a place where dragons^H^H^H^H^H deadlocks and
races live. The only reason I've heard about is that kind of "security". I don't
think that it is a fair trade[off]. Can you show me any other reason
of why should we "connect" files to the VFS tree ?
> The filesystem could concievably find it much easier. It
> might have a record of the directory in the file object (I think the
> Domain/OS filesystem did this). So it is a task best left to the
> filesystem.
Did you see the patch ? It *is* up to filesystem.
> Quite possibly support routines could live somewhere
> so that each filesystem didn't have to duplicate code, but each file
> system should have control of how it identifies the parent.
What do you mean ?
> Also, from a project management perspective, I think it would be best
> to leave knfsd only requires read_inode OR the two new procedures, and
> leave the old code there to cope with filesystems that haven't defined
> the new procedures yet. That way you can have a graceful transition.
Oh, well. The "interna ideo" is to get rid of iget and read_inode.
Completely. And what you have proposed in a paragraph above is just
dangerous. quasi-exportable filesystem...it's like being quasi-pregnant.
> You asked by opinion on this a while ago and you seem to have ignored
> it? Maybe you just didn't recieve it, so here it is again.
No, I've received it. See my comments bellow.
> ----------------------------------------------------------------
> The interface that I would suggest would be to add to
> super_operations:
>
> struct dentry *(*resolv_fh)(struct super_block *sb,
> char *fh,
> int fh_len,
> char flavour,
> struct dentry *base);
>
> int (*build_fh)(struct dentry *target,
> char *fh,
> int fh_len,
> char *flavour,
> int flags);
Too complicated. Well if you don't like the idea of fixed-sized
fhandles, than why don't we use something like:
struct dentry *(*resolv_fh)(struct super_block *sb,
void *fh_cookie);
int (*build_fh)(struct dentry *target,
void *fh_cookie);
int (*stat_fh)(void *fh_cookie);
> #define NFSD_FH_LOCATABLE 1
>
> "fh" is a pointer to a 4-byte aligned point in the file handle where
> the filesystem specific part of the filehandle is.
> "fh_len" is the amount of space left in the filehandle. It may indicate
> more space than build_fh said was used.
> "flavour' is one byte which is stored elsewhere in the filehandle.
> 0 may not be used
> 1 must be interpreted as "4 byte inode number, 4 byte generation
> number"
> 2 must be interpreted as "4 byte inode number, 4 byte generation
> number, 4 byte inode number of parent directory"
>
> alternatively, 1 and 2 can simply not be used.
very NFS-oriented. and ( from my point of view ) not very clean.
> "base", if non-NULL, means "make sure that the returned dentry is a
> descendant of "base".
Why ? fs should not care. It's up to the caller to make assertions
like this.
> "flags" can be "NFSD_FH_LOCATABLE" meaning that "resolv_fh" will
> probably be called with a non-NULL "base", so you might want to
> include extra information in the file handle so that you can meet the
> requirements of base being non-NULL.
Very vague. Is it necessary ? If yes -- why ?
> resolv_fh returns a dentry, or an error status via ERR_PTR
reasonable
> build_fh returns the number of byte os the filehandle that was
> encoded, or a -ve errno code.
not necessary.
> I could probably live with the "flavour" field if it was thought to
> be ugly, but I think the rest if necessary.
no. Just because a given filesystem should not care abot things like
"make sure that the returned dentry is a descendant of "base".
> knfsd can get answers to such questions as:
>
> - can you support NFSD_FH_LOCATABLE
> - is file name comparison case insensitive (will be needed for NFSv4)
> - what is the maximum file size
May be all this stuff is necessary but it is a whole different story.
Roman.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Jun 23 2000 - 21:00:21 EST