Re: [PATCH 2/3] NFS support for btrfs - v2

Christoph Hellwig
2008-08-12T18:51:41Z

On Tue, Aug 12, 2008 at 02:46:46PM +0100, David Woodhouse wrote:
> +static inline struct dentry *d_obtain_alias(struct inode *inode)
> +{
> + struct dentry *d = d_alloc_anon(inode);
> + if (!d)
> + iput(inode);
> + return d;
> +}
> +#endif

I'm not sure when al wants to merge with Linus, but the for-next naming
makes it sound like the tree is the .28 queue. Also please take the
full implementation of d_obtain_alias from

http://git.kernel.org/?p=linux/kernel/git/viro/vfs-2.6.git;a=commitdiff;h=10cdb734be3c4175b977ba18eafbaba8e5716291

please. Your light implementation will crash and burn when btrfs_iget
returns an error.

> +/* The size of encoded fh is the type number of the fh itself */
> +#define BTRFS_FID_NON_CONNECTABLE 5
> +#define BTRFS_FID_CONNECTABLE 8

That was an assumption in the very old code, but I think it's a very
bad idea. Just give your filehandles a uniqueue type number, e.g. in
the 0x4? range so that people looking at nfs traffic using a packet
analyzer know what kind of fhs they are actually dealing with.

> + if (IS_ERR(inode))
> + return (void *)inode;
> +
> + if (generation != inode->i_generation) {
> + iput(inode);
> + return ERR_PTR(-ESTALE);
> + }
> +
> + result = d_alloc_anon(inode);
> + if (!result) {
> + iput(inode);
> + return ERR_PTR(-ENOMEM);
> + }

Didn't you intend to use d_obtain_alias?