lkml.org 
[lkml]   [2008]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] Add basic export support to HFS+.
From
Date
On Tue, 2008-12-09 at 05:06 -0500, Christoph Hellwig wrote:
> > +static struct inode *hfsplus_export_get_inode(struct super_block *sb,
> > + u64 ino, u32 generation)
> > +{
> > + struct inode *inode;
> > +
> > + if (ino < HFSPLUS_FIRSTUSER_CNID && ino != HFSPLUS_ROOT_CNID)
> > + return ERR_PTR(-ESTALE);
> > +
> > + /* iget isn't really right if the inode is currently unallocated!!
> > + */
>
> I don't understand this comment. This interface is only used when
> getting the FH for an already allocated inode. hfsplus_iget is what
> all other operations use in hfsplus, so it seems fine. hfs experts
> might chime in that something else might be needed due to the strange
> ways links work in hfs, but from the VFS POW it seems fine.

Sincerely, this is just blatantly copied over from the equivalent ext2
function.

>
> > + inode = hfsplus_iget(sb, ino);
> > + if (IS_ERR(inode))
> > + return ERR_CAST(inode);
> > + if (generation && inode->i_generation != generation) {
> > + /* we didn't find the right inode.. */
> > + iput(inode);
> > + return ERR_PTR(-ESTALE);
> > + }
>
> hfsplus never updates i_generation, so this check is superflous. Brad,
> Roman: is there something that can help to guard against inode number
> reuse on HFS/HFSplus?

As above, just copied over, I tried to keep the code as similar as
possible so to make it clear that it's a copy-paste.

>
>
> > +/* Yes, most of these are left as NULL!!
> > + * A NULL value implies the default, which (hopefully) works with
> > + * hfs+-like file systems, but can be improved upon.
> > + * Currently only fh_to_dentry is required.
> > + */
>
> You _do_ need a get_parent, otherwise access will magically fail once
> any directory inode in a path is out of the cache. This can be
> trivially reproduced by unexporting and unmounting a filesystem while
> a client has a cwd deep inside the exported filesystem.

Okay, I think I was able to reproduce this both in "production" (as in,
making use of this in my real system) and on my testing virtual machine.
I'll get it fixed.

> And otherwise I don't think this comment is helpful in the code,
> fh_to_parent / fh_To_dentry and get_parent is what most simpler
> filesystems have.

Yes I noticed, my reason to change the comment was that the code
actually only tests fh_to_dentry (while the comment on ext2 structure
says that get_parent is the only needed one).

--
Diego "Flameeyes" Pettenò
http://blog.flameeyes.eu/

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2008-12-09 15:29    [W:0.099 / U:3.488 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site