Re: [NFS] [PATCH] Make UDF exportable

Re: [NFS] [PATCH] Make UDF exportable

[Christoph Hellwig]

On Wed, Jan 30, 2008 at 09:53:24PM +0100, Rasmus Rohde wrote:
> I've cooked together a patch for making UDF exportable.

Thanks, I know some people have been waiting for this for quite a while.
Please make sure Jan Kara who's the new udf maintainer and linux-fsdevel
where we discuss general filesystem related issue for future revisions
of the patch.

> It is based on the code found in ISO fs. Since I am far from an expert
> in this area bugs may be present.

isofs might not be the very best example since it's an odd filesystem,
but then so is udf.  I'll go through your patch in a little more detail
below, but it looks quite reasonable.

> +static struct dentry *udf_export_get_parent(struct dentry *child)

Any reason this is not called udf_get_parent to follow the scheme
in most filesystems?

> +	lock_kernel();
> +	if (udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi)) {
> +		if (fibh.sbh != fibh.ebh)
> +			brelse(fibh.ebh);
> +		brelse(fibh.sbh);
> +		
> +		inode = udf_iget(child->d_inode->i_sb,
> +				 lelb_to_cpu(cfi.icb.extLocation));
> +		if (!inode) {
> +			unlock_kernel();
> +			return ERR_PTR(-EACCES);
> +		}
> +	} else {
> +		unlock_kernel();
> +		return ERR_PTR(-EACCES);
> +	}
> +	unlock_kernel();

This if/else block looks little odd, and following the locking is a bit
hard.  What about doing it like the following:

	lock_kernel();
	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
		goto out_unlock;

	if (fibh.sbh != fibh.ebh)
		brelse(fibh.ebh);
	brelse(fibh.sbh);

	inode = udf_iget(child->d_inode->i_sb,
			 lelb_to_cpu(cfi.icb.extLocation));
	if (!inode)
		goto out_unlock;
	unlock_kernel();

	parent = d_alloc_anon(inode);
	if (!parent) {
		iput(inode);
		parent = ERR_PTR(-ENOMEM);
	}

	return parent;
 out_unlock:
	unlock_kernel();
	return ERR_PTR(-EACCESS);
}


> +static struct dentry *
> +udf_export_iget(struct super_block *sb, u32 block,
> +		u16 offset, __u32 generation)

to follow other filesystems this should be called
udf_nfs_get_inode.  Also please decide if you want to put the static
and return type on the same line or on a separate one.  Having it on
the same one is documented in Documentation/Codingstyle but separate
ones are acceptable aswell.  Just make sure to stick to either one :)

> +{
> +	struct inode *inode;
> +	struct dentry *result;
> +	kernel_lb_addr loc;
> +
> +	if (block == 0)
> +		return ERR_PTR(-ESTALE);
> +
> +	loc.logicalBlockNum = block;
> +	loc.partitionReferenceNum = offset;
> +	inode = udf_iget(sb, loc);
> +
> +	if (inode == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (is_bad_inode(inode)
> +	    || (generation && inode->i_generation != generation))
> +	{

it would be better to introduce a version of udf_iget that can check
the generation and return an error instead of having to check this
later.  If you don't think you're up to modifying code we could do
this later on, though.  In this case please note this in the patch
description and fix up the above formatting to read something like:

	if (is_bad_inode(inode) ||
	    (generation && inode->i_generation != generation)) {

> +static struct dentry *udf_fh_to_dentry(struct super_block *sb,
> +	struct fid *fid, int fh_len, int fh_type)
> +{
> +	struct udf_fid *ufid = (struct udf_fid *)fid;
> +
> +	if (fh_len < 3 || fh_type > 2)
> +		return NULL;

It would be useful if you could add symbolic constants for the
two fh types you add and chose values not yet used by other filesystems,
e.g. 0x51 and 0x52.  This will help people sniffing the nfs on the
wire protocol to understand what file handle they're dealing with.

Also you migh want to make the fh_len check more explicit and check
that it's either 3 or 5 which is the only fhs you actually generate.

> +static struct dentry *udf_fh_to_parent(struct super_block *sb,
> +		struct fid *fid, int fh_len, int fh_type)
> +{
> +	struct udf_fid *ufid = (struct udf_fid *)fid;
> +
> +	if (fh_type != 2)
> +		return NULL;

and a check for len == 5 here.

> +
> +	return udf_export_iget(sb,
> +			fh_len > 2 ? ufid->parent_block : 0,
> +			ufid->parent_partref,
> +			fh_len > 4 ? ufid->parent_generation : 0);

and you can remove these checks as you only end up here with len == 5
fhs.

Also it would be nice if you could add your fid type to the union in
include/linux/exportfs.h and use the union member.  The symbolic names
for the FH types should go into enum fid_type with a short comment
describing them.

Thanks for all this work!

Re: [NFS] [PATCH] Make UDF exportable

[Rasmus Rohde]

> > +static struct dentry *udf_export_get_parent(struct dentry *child)
> Any reason this is not called udf_get_parent to follow the scheme
> in most filesystems?
Well - in isofs it was named isofs_export_get_parent, so I tried to stay consistent with that.

> > +	lock_kernel();
> > +	if (udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi)) {
> > +		if (fibh.sbh != fibh.ebh)
> > +			brelse(fibh.ebh);
> > +		brelse(fibh.sbh);
> > +		
> > +		inode = udf_iget(child->d_inode->i_sb,
> > +				 lelb_to_cpu(cfi.icb.extLocation));
> > +		if (!inode) {
> > +			unlock_kernel();
> > +			return ERR_PTR(-EACCES);
> > +		}
> > +	} else {
> > +		unlock_kernel();
> > +		return ERR_PTR(-EACCES);
> > +	}
> > +	unlock_kernel();
> 
> This if/else block looks little odd, and following the locking is a bit
> hard.  What about doing it like the following:
Most of the code is copied from udf_lookup(..)
To me the locking is simple. Before the two returns you have an unlock.
It's pretty clear that all control-paths are covered. However changing
to your scheme is fine with me if you find it more readable.

> > +static struct dentry *
> > +udf_export_iget(struct super_block *sb, u32 block,
> > +		u16 offset, __u32 generation)
> to follow other filesystems this should be called
> udf_nfs_get_inode.  Also please decide if you want to put the static
> and return type on the same line or on a separate one.  Having it on
> the same one is documented in Documentation/Codingstyle but separate
> ones are acceptable aswell.  Just make sure to stick to either one :)
Again - this was just copied from isofs.

> > +{
> > +	struct inode *inode;
> > +	struct dentry *result;
> > +	kernel_lb_addr loc;
> > +
> > +	if (block == 0)
> > +		return ERR_PTR(-ESTALE);
> > +
> > +	loc.logicalBlockNum = block;
> > +	loc.partitionReferenceNum = offset;
> > +	inode = udf_iget(sb, loc);
> > +
> > +	if (inode == NULL)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	if (is_bad_inode(inode)
> > +	    || (generation && inode->i_generation != generation))
> > +	{
> it would be better to introduce a version of udf_iget that can check
> the generation and return an error instead of having to check this
> later.  If you don't think you're up to modifying code we could do
> this later on, though.  In this case please note this in the patch
> description and fix up the above formatting to read something like:
> 
> 	if (is_bad_inode(inode) ||
> 	    (generation && inode->i_generation != generation)) {
is_bad_inode(..) is actually already checked so I'll just remove that.


> > +static struct dentry *udf_fh_to_dentry(struct super_block *sb,
> > +	struct fid *fid, int fh_len, int fh_type)
> > +{
> > +	struct udf_fid *ufid = (struct udf_fid *)fid;
> > +
> > +	if (fh_len < 3 || fh_type > 2)
> > +		return NULL;
> 
> It would be useful if you could add symbolic constants for the
> two fh types you add and chose values not yet used by other filesystems,
> e.g. 0x51 and 0x52.  This will help people sniffing the nfs on the
> wire protocol to understand what file handle they're dealing with.
Hehe - need I say isofs? :)

> Also you migh want to make the fh_len check more explicit and check
> that it's either 3 or 5 which is the only fhs you actually generate.
Sounds reasonable including the other length-checks you mention.

> Also it would be nice if you could add your fid type to the union in
> include/linux/exportfs.h and use the union member.  The symbolic names
> for the FH types should go into enum fid_type with a short comment
> describing them.
Ok.

Attached is a new attempt at a patch.

Signed-off-by: Rasmus Rohde <rohde@duff.dk>

--- fs/udf/namei.c.orig	2007-10-10 16:22:30.000000000 +0200
+++ fs/udf/namei.c	2008-02-05 18:28:13.000000000 +0100
@@ -31,6 +31,7 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/sched.h>
+#include <linux/exportfs.h>
 
 static inline int udf_match(int len1, const char *name1, int len2,
 			    const char *name2)
@@ -315,9 +316,8 @@ static struct dentry *udf_lookup(struct 
 		}
 	}
 	unlock_kernel();
-	d_add(dentry, inode);
 
-	return NULL;
+	return d_splice_alias(inode, dentry);
 }
 
 static struct fileIdentDesc *udf_add_entry(struct inode *dir,
@@ -1231,6 +1231,135 @@ end_rename:
 	return retval;
 }
 
+static struct dentry *udf_get_parent(struct dentry *child)
+{
+	struct dentry *parent;
+	struct inode *inode = NULL;
+	struct dentry dotdot;
+	struct fileIdentDesc cfi;
+	struct udf_fileident_bh fibh;
+
+	dotdot.d_name.name = "..";
+	dotdot.d_name.len = 2;
+
+	lock_kernel();
+	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
+		goto out_unlock;
+	
+	if (fibh.sbh != fibh.ebh)
+		brelse(fibh.ebh);
+	brelse(fibh.sbh);
+		
+	inode = udf_iget(child->d_inode->i_sb,
+			 lelb_to_cpu(cfi.icb.extLocation));
+	if (!inode)
+		goto out_unlock;
+	unlock_kernel();
+
+	parent = d_alloc_anon(inode);
+	if (!parent) {
+		iput(inode);
+		parent = ERR_PTR(-ENOMEM);
+	}
+
+	return parent;
+out_unlock:
+	unlock_kernel();
+	return ERR_PTR(-EACCES);
+}
+
+
+static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
+					u16 partref, __u32 generation)
+{
+	struct inode *inode;
+	struct dentry *result;
+	kernel_lb_addr loc;
+
+	if (block == 0)
+		return ERR_PTR(-ESTALE);
+
+	loc.logicalBlockNum = block;
+	loc.partitionReferenceNum = partref;
+	inode = udf_iget(sb, loc);
+
+	if (inode == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	if (generation && inode->i_generation != generation) {
+		iput(inode);
+		return ERR_PTR(-ESTALE);
+	}
+	result = d_alloc_anon(inode);
+	if (!result) {
+		iput(inode);
+		return ERR_PTR(-ENOMEM);
+	}
+	return result;
+}
+
+static struct dentry *udf_fh_to_dentry(struct super_block *sb,
+				       struct fid *fid, int fh_len, int fh_type)
+{
+	if ((fh_len != 3 && fh_len != 5) ||
+	    (fh_type != FILEID_UDF_WITH_PARENT &&
+	     fh_type != FILEID_UDF_WITHOUT_PARENT))
+		return NULL;
+
+	return udf_nfs_get_inode(sb, fid->udf.block, fid->udf.partref,
+			fid->udf.generation);
+}
+
+static struct dentry *udf_fh_to_parent(struct super_block *sb,
+				       struct fid *fid, int fh_len, int fh_type)
+{
+	if (fh_len != 5 || fh_type != FILEID_UDF_WITHOUT_PARENT)
+		return NULL;
+
+	return udf_nfs_get_inode(sb, fid->udf.parent_block,
+				 fid->udf.parent_partref,
+				 fid->udf.parent_generation);
+}
+static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
+			 int connectable)
+{
+	int len = *lenp;
+	struct inode *inode =  de->d_inode;
+	kernel_lb_addr location = UDF_I_LOCATION(inode);
+	struct fid *fid = (struct fid *)fh;
+	int type = FILEID_UDF_WITHOUT_PARENT;
+
+	if (len < 3 || (connectable && len < 5))
+		return 255;
+
+	*lenp = 3;
+	fid->udf.block = location.logicalBlockNum;
+	fid->udf.partref = location.partitionReferenceNum;
+	fid->udf.generation = inode->i_generation;
+
+	if (connectable && !S_ISDIR(inode->i_mode)) {
+		spin_lock(&de->d_lock);
+		inode = de->d_parent->d_inode;
+		location = UDF_I_LOCATION(inode);
+		fid->udf.parent_block = location.logicalBlockNum;
+		fid->udf.parent_partref = location.partitionReferenceNum;	
+		fid->udf.parent_generation = inode->i_generation;
+		spin_unlock(&de->d_lock);
+		*lenp = 5;
+		type = FILEID_UDF_WITH_PARENT;
+	}
+
+	return type;
+}
+
+struct export_operations udf_export_ops = {
+	.encode_fh	= udf_encode_fh,
+	.fh_to_dentry   = udf_fh_to_dentry,
+	.fh_to_parent   = udf_fh_to_parent,
+	.get_parent     = udf_get_parent,
+};
+
 const struct inode_operations udf_dir_inode_operations = {
 	.lookup				= udf_lookup,
 	.create				= udf_create,

--- fs/udf/super.c.orig	2008-01-30 17:57:23.000000000 +0100
+++ fs/udf/super.c	2008-01-30 21:38:10.000000000 +0100
@@ -71,7 +71,7 @@
 #define VDS_POS_LENGTH			7
 
 static char error_buf[1024];
-
+extern struct export_operations udf_export_ops;
 /* These are the "meat" - everything else is stuffing */
 static int udf_fill_super(struct super_block *, void *, int);
 static void udf_put_super(struct super_block *);
@@ -1490,6 +1490,7 @@ static int udf_fill_super(struct super_b
 
 	/* Fill in the rest of the superblock */
 	sb->s_op = &udf_sb_ops;
+	sb->s_export_op = &udf_export_ops;
 	sb->dq_op = NULL;
 	sb->s_dirt = 0;
 	sb->s_magic = UDF_SUPER_MAGIC;

--- include/linux/exportfs.h.orig	2008-02-05 18:28:44.000000000 +0100
+++ include/linux/exportfs.h	2008-02-05 18:28:51.000000000 +0100
@@ -33,6 +33,19 @@ enum fid_type {
 	 * 32 bit parent directory inode number.
 	 */
 	FILEID_INO32_GEN_PARENT = 2,
+
+	/*
+	 * 32 bit block number, 16 bit partition reference,
+	 * 16 bit unused, 32 bit generation number.
+	 */
+	FILEID_UDF_WITHOUT_PARENT = 0x51,
+
+	/*
+	 * 32 bit block number, 16 bit partition reference,
+	 * 16 bit unused, 32 bit generation number,
+	 * 32 bit parent block number, 32 bit parent generation number
+	 */
+	FILEID_UDF_WITH_PARENT = 0x52,
 };
 
 struct fid {
@@ -43,6 +56,14 @@ struct fid {
 			u32 parent_ino;
 			u32 parent_gen;
 		} i32;
+		struct {
+			u32 block;
+			u16 partref;
+			u16 parent_partref;
+			u32 generation;
+			u32 parent_block;
+			u32 parent_generation;
+		} udf;
 		__u32 raw[6];
 	};
 };


-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [NFS] [PATCH] Make UDF exportable

[Rasmus Rohde]

> +static struct dentry *udf_fh_to_parent(struct super_block *sb,
> +				       struct fid *fid, int fh_len, int fh_type)
> +{
> +	if (fh_len != 5 || fh_type != FILEID_UDF_WITHOUT_PARENT)
                                                   ^^^^^^^

Argh - this should have been WITH so the line reads:

if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT)

I'll just hold back a little reposting a new patch if other comments
should happen to show up.

Re: [NFS] [PATCH] Make UDF exportable

[Jan Kara]

  Hi,

> Signed-off-by: Rasmus Rohde <rohde@duff.dk>
  Interesting character before Signed-off-by :).

> --- fs/udf/namei.c.orig	2007-10-10 16:22:30.000000000 +0200
> +++ fs/udf/namei.c	2008-02-05 18:28:13.000000000 +0100
> @@ -31,6 +31,7 @@
>  #include <linux/smp_lock.h>
>  #include <linux/buffer_head.h>
>  #include <linux/sched.h>
> +#include <linux/exportfs.h>
>  
>  static inline int udf_match(int len1, const char *name1, int len2,
>  			    const char *name2)
> @@ -315,9 +316,8 @@ static struct dentry *udf_lookup(struct 
>  		}
>  	}
>  	unlock_kernel();
> -	d_add(dentry, inode);
>  
> -	return NULL;
> +	return d_splice_alias(inode, dentry);
>  }
>  
>  static struct fileIdentDesc *udf_add_entry(struct inode *dir,
> @@ -1231,6 +1231,135 @@ end_rename:
>  	return retval;
>  }
>  
> +static struct dentry *udf_get_parent(struct dentry *child)
> +{
> +	struct dentry *parent;
> +	struct inode *inode = NULL;
> +	struct dentry dotdot;
> +	struct fileIdentDesc cfi;
> +	struct udf_fileident_bh fibh;
> +
> +	dotdot.d_name.name = "..";
> +	dotdot.d_name.len = 2;
> +
> +	lock_kernel();
> +	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
> +		goto out_unlock;
  Have you ever tried this? I think this could never work. UDF doesn't have
entry named .. in a directory. You have to search for an entry that has
in fileCharacteristics set bit FID_FILE_CHAR_PARENT. Maybe you could
hack-around udf_find_entry() to recognize .. dentry and do the search
accordingly.

> +	
> +	if (fibh.sbh != fibh.ebh)
> +		brelse(fibh.ebh);
> +	brelse(fibh.sbh);
> +		
> +	inode = udf_iget(child->d_inode->i_sb,
> +			 lelb_to_cpu(cfi.icb.extLocation));
> +	if (!inode)
> +		goto out_unlock;
> +	unlock_kernel();
> +
> +	parent = d_alloc_anon(inode);
> +	if (!parent) {
> +		iput(inode);
> +		parent = ERR_PTR(-ENOMEM);
> +	}
> +
> +	return parent;
> +out_unlock:
> +	unlock_kernel();
> +	return ERR_PTR(-EACCES);
> +}
> +
> +
> +static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
> +					u16 partref, __u32 generation)
> +{
> +	struct inode *inode;
> +	struct dentry *result;
> +	kernel_lb_addr loc;
> +
> +	if (block == 0)
> +		return ERR_PTR(-ESTALE);
> +
> +	loc.logicalBlockNum = block;
> +	loc.partitionReferenceNum = partref;
> +	inode = udf_iget(sb, loc);
> +
> +	if (inode == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (generation && inode->i_generation != generation) {
> +		iput(inode);
> +		return ERR_PTR(-ESTALE);
> +	}
> +	result = d_alloc_anon(inode);
> +	if (!result) {
> +		iput(inode);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	return result;
> +}
> +
> +static struct dentry *udf_fh_to_dentry(struct super_block *sb,
> +				       struct fid *fid, int fh_len, int fh_type)
> +{
> +	if ((fh_len != 3 && fh_len != 5) ||
> +	    (fh_type != FILEID_UDF_WITH_PARENT &&
> +	     fh_type != FILEID_UDF_WITHOUT_PARENT))
> +		return NULL;
> +
> +	return udf_nfs_get_inode(sb, fid->udf.block, fid->udf.partref,
> +			fid->udf.generation);
> +}
> +
> +static struct dentry *udf_fh_to_parent(struct super_block *sb,
> +				       struct fid *fid, int fh_len, int fh_type)
> +{
> +	if (fh_len != 5 || fh_type != FILEID_UDF_WITHOUT_PARENT)
> +		return NULL;
> +
> +	return udf_nfs_get_inode(sb, fid->udf.parent_block,
> +				 fid->udf.parent_partref,
> +				 fid->udf.parent_generation);
> +}
> +static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
> +			 int connectable)
> +{
> +	int len = *lenp;
> +	struct inode *inode =  de->d_inode;
> +	kernel_lb_addr location = UDF_I_LOCATION(inode);
> +	struct fid *fid = (struct fid *)fh;
> +	int type = FILEID_UDF_WITHOUT_PARENT;
> +
> +	if (len < 3 || (connectable && len < 5))
> +		return 255;
> +
> +	*lenp = 3;
> +	fid->udf.block = location.logicalBlockNum;
> +	fid->udf.partref = location.partitionReferenceNum;
> +	fid->udf.generation = inode->i_generation;
> +
> +	if (connectable && !S_ISDIR(inode->i_mode)) {
> +		spin_lock(&de->d_lock);
> +		inode = de->d_parent->d_inode;
> +		location = UDF_I_LOCATION(inode);
> +		fid->udf.parent_block = location.logicalBlockNum;
> +		fid->udf.parent_partref = location.partitionReferenceNum;	
> +		fid->udf.parent_generation = inode->i_generation;
> +		spin_unlock(&de->d_lock);
> +		*lenp = 5;
> +		type = FILEID_UDF_WITH_PARENT;
> +	}
> +
> +	return type;
> +}
> +
> +struct export_operations udf_export_ops = {
> +	.encode_fh	= udf_encode_fh,
> +	.fh_to_dentry   = udf_fh_to_dentry,
> +	.fh_to_parent   = udf_fh_to_parent,
> +	.get_parent     = udf_get_parent,
> +};
> +
>  const struct inode_operations udf_dir_inode_operations = {
>  	.lookup				= udf_lookup,
>  	.create				= udf_create,
> 
> --- fs/udf/super.c.orig	2008-01-30 17:57:23.000000000 +0100
> +++ fs/udf/super.c	2008-01-30 21:38:10.000000000 +0100
> @@ -71,7 +71,7 @@
>  #define VDS_POS_LENGTH			7
>  
>  static char error_buf[1024];
> -
> +extern struct export_operations udf_export_ops;
>  /* These are the "meat" - everything else is stuffing */
>  static int udf_fill_super(struct super_block *, void *, int);
>  static void udf_put_super(struct super_block *);
> @@ -1490,6 +1490,7 @@ static int udf_fill_super(struct super_b
>  
>  	/* Fill in the rest of the superblock */
>  	sb->s_op = &udf_sb_ops;
> +	sb->s_export_op = &udf_export_ops;
>  	sb->dq_op = NULL;
>  	sb->s_dirt = 0;
>  	sb->s_magic = UDF_SUPER_MAGIC;
> 
> --- include/linux/exportfs.h.orig	2008-02-05 18:28:44.000000000 +0100
> +++ include/linux/exportfs.h	2008-02-05 18:28:51.000000000 +0100
> @@ -33,6 +33,19 @@ enum fid_type {
>  	 * 32 bit parent directory inode number.
>  	 */
>  	FILEID_INO32_GEN_PARENT = 2,
> +
> +	/*
> +	 * 32 bit block number, 16 bit partition reference,
> +	 * 16 bit unused, 32 bit generation number.
> +	 */
> +	FILEID_UDF_WITHOUT_PARENT = 0x51,
> +
> +	/*
> +	 * 32 bit block number, 16 bit partition reference,
> +	 * 16 bit unused, 32 bit generation number,
> +	 * 32 bit parent block number, 32 bit parent generation number
> +	 */
> +	FILEID_UDF_WITH_PARENT = 0x52,
>  };
>  
>  struct fid {
> @@ -43,6 +56,14 @@ struct fid {
>  			u32 parent_ino;
>  			u32 parent_gen;
>  		} i32;
> +		struct {
> +			u32 block;
> +			u16 partref;
> +			u16 parent_partref;
> +			u32 generation;
> +			u32 parent_block;
> +			u32 parent_generation;
> +		} udf;
>  		__u32 raw[6];
>  	};
>  };
  Otherwise the patch looks fine. But please rediff the patch against
Andrew's development tree (or -mm) because there are some cleanups there...
Thanks.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [NFS] [PATCH] Make UDF exportable

[Rasmus Rohde]

> > +	dotdot.d_name.name = "..";
> > +	dotdot.d_name.len = 2;
> > +
> > +	lock_kernel();
> > +	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
> > +		goto out_unlock;
>   Have you ever tried this? I think this could never work. UDF doesn't have
> entry named .. in a directory. You have to search for an entry that has
> in fileCharacteristics set bit FID_FILE_CHAR_PARENT. Maybe you could
> hack-around udf_find_entry() to recognize .. dentry and do the search
> accordingly.
Probably not. I just tested that I could read files and navigate the
directory structure. However looking into UDF I think you are right - it
will fail.
I have extended udf_find_entry() to do an explicit check based on
fileCharacteristics as you propose.
How do I actually test this case?

>   Otherwise the patch looks fine. But please rediff the patch against
> Andrew's development tree (or -mm) because there are some cleanups there...
> Thanks.
Certainly there are. New patch against 2.6.24-mm1:

Signed-off-by: Rasmus Rohde <rohde@duff.dk>

diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/namei.c linux-2.6.24-mm1/fs/udf/namei.c
--- linux-2.6.24-mm1-vanilla/fs/udf/namei.c	2008-02-06 21:23:36.000000000 +0100
+++ linux-2.6.24-mm1/fs/udf/namei.c	2008-02-06 21:41:38.000000000 +0100
@@ -31,6 +31,7 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/sched.h>
+#include <linux/exportfs.h>
 
 static inline int udf_match(int len1, const char *name1, int len2,
 			    const char *name2)
@@ -159,6 +160,8 @@ static struct fileIdentDesc *udf_find_en
 	sector_t offset;
 	struct extent_position epos = {};
 	struct udf_inode_info *dinfo = UDF_I(dir);
+	int isdotdot = dentry->d_name.len == 2 &&
+		dentry->d_name.name[0] == '.' && dentry->d_name.name[1] == '.';
 
 	size = udf_ext0_offset(dir) + dir->i_size;
 	f_pos = udf_ext0_offset(dir);
@@ -232,6 +235,12 @@ static struct fileIdentDesc *udf_find_en
 				continue;
 		}
 
+		if ((cfi->fileCharacteristics & FID_FILE_CHAR_PARENT) &&
+		    isdotdot) {
+			brelse(epos.bh);
+			return fi;
+		}
+
 		if (!lfi)
 			continue;
 
@@ -324,9 +333,8 @@ static struct dentry *udf_lookup(struct 
 		}
 	}
 	unlock_kernel();
-	d_add(dentry, inode);
 
-	return NULL;
+	return d_splice_alias(inode, dentry);
 }
 
 static struct fileIdentDesc *udf_add_entry(struct inode *dir,
@@ -1298,6 +1306,134 @@ end_rename:
 	return retval;
 }
 
+static struct dentry *udf_get_parent(struct dentry *child)
+{
+	struct dentry *parent;
+	struct inode *inode = NULL;
+	struct dentry dotdot;
+	struct fileIdentDesc cfi;
+	struct udf_fileident_bh fibh;
+
+	dotdot.d_name.name = "..";
+	dotdot.d_name.len = 2;
+
+	lock_kernel();
+	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
+		goto out_unlock;
+	
+	if (fibh.sbh != fibh.ebh)
+		brelse(fibh.ebh);
+	brelse(fibh.sbh);
+		
+	inode = udf_iget(child->d_inode->i_sb,
+			 lelb_to_cpu(cfi.icb.extLocation));
+	if (!inode)
+		goto out_unlock;
+	unlock_kernel();
+
+	parent = d_alloc_anon(inode);
+	if (!parent) {
+		iput(inode);
+		parent = ERR_PTR(-ENOMEM);
+	}
+
+	return parent;
+out_unlock:
+	unlock_kernel();
+	return ERR_PTR(-EACCES);
+}
+
+
+static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
+					u16 partref, __u32 generation)
+{
+	struct inode *inode;
+	struct dentry *result;
+	kernel_lb_addr loc;
+
+	if (block == 0)
+		return ERR_PTR(-ESTALE);
+
+	loc.logicalBlockNum = block;
+	loc.partitionReferenceNum = partref;
+	inode = udf_iget(sb, loc);
+
+	if (inode == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	if (generation && inode->i_generation != generation) {
+		iput(inode);
+		return ERR_PTR(-ESTALE);
+	}
+	result = d_alloc_anon(inode);
+	if (!result) {
+		iput(inode);
+		return ERR_PTR(-ENOMEM);
+	}
+	return result;
+}
+
+static struct dentry *udf_fh_to_dentry(struct super_block *sb,
+				       struct fid *fid, int fh_len, int fh_type)
+{
+	if ((fh_len != 3 && fh_len != 5) ||
+	    (fh_type != FILEID_UDF_WITH_PARENT &&
+	     fh_type != FILEID_UDF_WITHOUT_PARENT))
+		return NULL;
+
+	return udf_nfs_get_inode(sb, fid->udf.block, fid->udf.partref,
+			fid->udf.generation);
+}
+
+static struct dentry *udf_fh_to_parent(struct super_block *sb,
+				       struct fid *fid, int fh_len, int fh_type)
+{
+	if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT)
+		return NULL;
+
+	return udf_nfs_get_inode(sb, fid->udf.parent_block,
+				 fid->udf.parent_partref,
+				 fid->udf.parent_generation);
+}
+static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
+			 int connectable)
+{
+	int len = *lenp;
+	struct inode *inode =  de->d_inode;
+	kernel_lb_addr location = UDF_I(inode)->i_location;
+	struct fid *fid = (struct fid *)fh;
+	int type = FILEID_UDF_WITHOUT_PARENT;
+
+	if (len < 3 || (connectable && len < 5))
+		return 255;
+
+	*lenp = 3;
+	fid->udf.block = location.logicalBlockNum;
+	fid->udf.partref = location.partitionReferenceNum;
+	fid->udf.generation = inode->i_generation;
+
+	if (connectable && !S_ISDIR(inode->i_mode)) {
+		spin_lock(&de->d_lock);
+		inode = de->d_parent->d_inode;
+		location = UDF_I(inode)->i_location;
+		fid->udf.parent_block = location.logicalBlockNum;
+		fid->udf.parent_partref = location.partitionReferenceNum;	
+		fid->udf.parent_generation = inode->i_generation;
+		spin_unlock(&de->d_lock);
+		*lenp = 5;
+		type = FILEID_UDF_WITH_PARENT;
+	}
+
+	return type;
+}
+
+struct export_operations udf_export_ops = {
+	.encode_fh	= udf_encode_fh,
+	.fh_to_dentry   = udf_fh_to_dentry,
+	.fh_to_parent   = udf_fh_to_parent,
+	.get_parent     = udf_get_parent,
+};
+
 const struct inode_operations udf_dir_inode_operations = {
 	.lookup				= udf_lookup,
 	.create				= udf_create,
diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/super.c linux-2.6.24-mm1/fs/udf/super.c
--- linux-2.6.24-mm1-vanilla/fs/udf/super.c	2008-02-06 21:23:36.000000000 +0100
+++ linux-2.6.24-mm1/fs/udf/super.c	2008-02-06 21:41:38.000000000 +0100
@@ -76,6 +76,7 @@
 #define UDF_DEFAULT_BLOCKSIZE 2048
 
 static char error_buf[1024];
+extern struct export_operations udf_export_ops;
 
 /* These are the "meat" - everything else is stuffing */
 static int udf_fill_super(struct super_block *, void *, int);
@@ -1801,6 +1802,7 @@ static int udf_fill_super(struct super_b
 
 	/* Fill in the rest of the superblock */
 	sb->s_op = &udf_sb_ops;
+	sb->s_export_op = &udf_export_ops;
 	sb->dq_op = NULL;
 	sb->s_dirt = 0;
 	sb->s_magic = UDF_SUPER_MAGIC;
diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/include/linux/exportfs.h linux-2.6.24-mm1/include/linux/exportfs.h
--- linux-2.6.24-mm1-vanilla/include/linux/exportfs.h	2008-02-06 21:23:46.000000000 +0100
+++ linux-2.6.24-mm1/include/linux/exportfs.h	2008-02-06 21:25:06.000000000 +0100
@@ -33,6 +33,19 @@ enum fid_type {
 	 * 32 bit parent directory inode number.
 	 */
 	FILEID_INO32_GEN_PARENT = 2,
+
+	/*
+	 * 32 bit block number, 16 bit partition reference,
+	 * 16 bit unused, 32 bit generation number.
+	 */
+	FILEID_UDF_WITHOUT_PARENT = 0x51,
+
+	/*
+	 * 32 bit block number, 16 bit partition reference,
+	 * 16 bit unused, 32 bit generation number,
+	 * 32 bit parent block number, 32 bit parent generation number
+	 */
+	FILEID_UDF_WITH_PARENT = 0x52,
 };
 
 struct fid {
@@ -43,6 +56,14 @@ struct fid {
 			u32 parent_ino;
 			u32 parent_gen;
 		} i32;
+		struct {
+			u32 block;
+			u16 partref;
+			u16 parent_partref;
+			u32 generation;
+			u32 parent_block;
+			u32 parent_generation;
+		} udf;
 		__u32 raw[6];
 	};
 };

Re: [NFS] [PATCH] Make UDF exportable

[Christoph Hellwig]

On Wed, Feb 06, 2008 at 09:58:02PM +0100, Rasmus Rohde wrote:
> Probably not. I just tested that I could read files and navigate the
> directory structure. However looking into UDF I think you are right - it
> will fail.
> I have extended udf_find_entry() to do an explicit check based on
> fileCharacteristics as you propose.
> How do I actually test this case?

Testing this is pretty hard.  You export a filesystem, then cd somewhere
deep into a directory hiearchy in there.  Then unexport the filesystem
and unmount on the server.  mount it back on the server, export it again
and do something with a file from the directory you've cd into before
the unmount.  Make sure you have a printk in your get_parent method
to make sure you're really hitting it.

Btw, I think it would be nicer to opencode the .. lookup in get_parent
instead of changing udf_find_entry.  The lookup for .. is not needed
by anything else, and get_parent only looks for it so it's a natural
place to opencode it there.

Re: [NFS] [PATCH] Make UDF exportable

[Neil Brown]

On Wednesday February 6, rohde@duff.dk wrote:
> > > +	dotdot.d_name.name = "..";
> > > +	dotdot.d_name.len = 2;
> > > +
> > > +	lock_kernel();
> > > +	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
> > > +		goto out_unlock;
> >   Have you ever tried this? I think this could never work. UDF doesn't have
> > entry named .. in a directory. You have to search for an entry that has
> > in fileCharacteristics set bit FID_FILE_CHAR_PARENT. Maybe you could
> > hack-around udf_find_entry() to recognize .. dentry and do the search
> > accordingly.
> Probably not. I just tested that I could read files and navigate the
> directory structure. However looking into UDF I think you are right - it
> will fail.
> I have extended udf_find_entry() to do an explicit check based on
> fileCharacteristics as you propose.
> How do I actually test this case?

 - Mount the filesystem from the server.
 - 'cd' a few directories down into the filesystem.
 - reboot the server(1)
 - on the client 'ls -l'.

(1) A full reboot isn't needed.  Just unexport, unmount, remount,
    re-export on the server.

alternately, use a non-linux client and cd down into the filesystem
and
    ls -l ..

NeilBrown

Re: [NFS] [PATCH] Make UDF exportable

[Christoph Hellwig]

On Wed, Feb 06, 2008 at 09:58:02PM +0100, Rasmus Rohde wrote:
> > > +	dotdot.d_name.name = "..";
> > > +	dotdot.d_name.len = 2;
> > > +
> > > +	lock_kernel();
> > > +	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
> > > +		goto out_unlock;
> >   Have you ever tried this? I think this could never work. UDF doesn't have
> > entry named .. in a directory. You have to search for an entry that has
> > in fileCharacteristics set bit FID_FILE_CHAR_PARENT. Maybe you could
> > hack-around udf_find_entry() to recognize .. dentry and do the search
> > accordingly.
> Probably not. I just tested that I could read files and navigate the
> directory structure. However looking into UDF I think you are right - it
> will fail.
> I have extended udf_find_entry() to do an explicit check based on
> fileCharacteristics as you propose.
> How do I actually test this case?
> 
> >   Otherwise the patch looks fine. But please rediff the patch against
> > Andrew's development tree (or -mm) because there are some cleanups there...
> > Thanks.
> Certainly there are. New patch against 2.6.24-mm1:

There's still a few trivial warnings from scripts/checkpatch.pl that
should be fixed up:

ERROR: trailing whitespace
#88: FILE: fs/udf/namei.c:1323:
+^I$

ERROR: trailing whitespace
#92: FILE: fs/udf/namei.c:1327:
+^I^I$

ERROR: trailing whitespace
#185: FILE: fs/udf/namei.c:1420:
+^I^Ifid->udf.parent_partref = location.partitionReferenceNum;^I$

WARNING: externs should be avoided in .c files
#212: FILE: fs/udf/super.c:79:
+extern struct export_operations udf_export_ops;

total: 3 errors, 1 warnings, 218 lines checked

Re: [NFS] [PATCH] Make UDF exportable

[Rasmus Rohde]

Ok - I have checked get_parent and it works as expected.
I used the "Neil Brown"-test mentioned elsewhere in this thread and
added a few printk's to make sure we actually got the code covered.

> There's still a few trivial warnings from scripts/checkpatch.pl that
> should be fixed up:
Fixed that. Sorry for not running checkpatch.pl before submitting.

Before posting the last and hopefully final patch I'd like to know what
Jan says about open coding the lookup for ..
It will mean a lot of code duplication and I think it makes good sense
for udf_find_entry to be able to handle ..

Re: [NFS] [PATCH] Make UDF exportable

[Jan Kara]

On Thu 07-02-08 08:06:37, Rasmus Rohde wrote:
> Ok - I have checked get_parent and it works as expected.
> I used the "Neil Brown"-test mentioned elsewhere in this thread and
> added a few printk's to make sure we actually got the code covered.
> 
> > There's still a few trivial warnings from scripts/checkpatch.pl that
> > should be fixed up:
> Fixed that. Sorry for not running checkpatch.pl before submitting.
> 
> Before posting the last and hopefully final patch I'd like to know what
> Jan says about open coding the lookup for ..
> It will mean a lot of code duplication and I think it makes good sense
> for udf_find_entry to be able to handle ..
  Yes, I think opencoding it would really lead to larger code duplication
than I'd like so please keep the change in udf_find_entry(). Thanks.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [NFS] [PATCH] Make UDF exportable

[Rasmus Rohde]

> > Before posting the last and hopefully final patch I'd like to know what
> > Jan says about open coding the lookup for ..
> > It will mean a lot of code duplication and I think it makes good sense
> > for udf_find_entry to be able to handle ..
>   Yes, I think opencoding it would really lead to larger code duplication
> than I'd like so please keep the change in udf_find_entry(). Thanks.
Great - then I think we are hopefully at a patch that can be accepted.

Signed-off-by: Rasmus Rohde <rohde-3mLe3WyXyAE@public.gmane.org>

diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/namei.c linux-2.6.24-mm1/fs/udf/namei.c
--- linux-2.6.24-mm1-vanilla/fs/udf/namei.c	2008-02-06 21:23:36.000000000 +0100
+++ linux-2.6.24-mm1/fs/udf/namei.c	2008-02-07 07:13:04.000000000 +0100
@@ -31,6 +31,7 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/sched.h>
+#include <linux/exportfs.h>
 
 static inline int udf_match(int len1, const char *name1, int len2,
 			    const char *name2)
@@ -159,6 +160,8 @@ static struct fileIdentDesc *udf_find_en
 	sector_t offset;
 	struct extent_position epos = {};
 	struct udf_inode_info *dinfo = UDF_I(dir);
+	int isdotdot = dentry->d_name.len == 2 &&
+		dentry->d_name.name[0] == '.' && dentry->d_name.name[1] == '.';
 
 	size = udf_ext0_offset(dir) + dir->i_size;
 	f_pos = udf_ext0_offset(dir);
@@ -232,6 +235,12 @@ static struct fileIdentDesc *udf_find_en
 				continue;
 		}
 
+		if ((cfi->fileCharacteristics & FID_FILE_CHAR_PARENT) &&
+		    isdotdot) {
+			brelse(epos.bh);
+			return fi;
+		}
+
 		if (!lfi)
 			continue;
 
@@ -324,9 +333,8 @@ static struct dentry *udf_lookup(struct 
 		}
 	}
 	unlock_kernel();
-	d_add(dentry, inode);
 
-	return NULL;
+	return d_splice_alias(inode, dentry);
 }
 
 static struct fileIdentDesc *udf_add_entry(struct inode *dir,
@@ -1298,6 +1306,134 @@ end_rename:
 	return retval;
 }
 
+static struct dentry *udf_get_parent(struct dentry *child)
+{
+	struct dentry *parent;
+	struct inode *inode = NULL;
+	struct dentry dotdot;
+	struct fileIdentDesc cfi;
+	struct udf_fileident_bh fibh;
+
+	dotdot.d_name.name = "..";
+	dotdot.d_name.len = 2;
+
+	lock_kernel();
+	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
+		goto out_unlock;
+
+	if (fibh.sbh != fibh.ebh)
+		brelse(fibh.ebh);
+	brelse(fibh.sbh);
+
+	inode = udf_iget(child->d_inode->i_sb,
+			 lelb_to_cpu(cfi.icb.extLocation));
+	if (!inode)
+		goto out_unlock;
+	unlock_kernel();
+
+	parent = d_alloc_anon(inode);
+	if (!parent) {
+		iput(inode);
+		parent = ERR_PTR(-ENOMEM);
+	}
+
+	return parent;
+out_unlock:
+	unlock_kernel();
+	return ERR_PTR(-EACCES);
+}
+
+
+static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
+					u16 partref, __u32 generation)
+{
+	struct inode *inode;
+	struct dentry *result;
+	kernel_lb_addr loc;
+
+	if (block == 0)
+		return ERR_PTR(-ESTALE);
+
+	loc.logicalBlockNum = block;
+	loc.partitionReferenceNum = partref;
+	inode = udf_iget(sb, loc);
+
+	if (inode == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	if (generation && inode->i_generation != generation) {
+		iput(inode);
+		return ERR_PTR(-ESTALE);
+	}
+	result = d_alloc_anon(inode);
+	if (!result) {
+		iput(inode);
+		return ERR_PTR(-ENOMEM);
+	}
+	return result;
+}
+
+static struct dentry *udf_fh_to_dentry(struct super_block *sb,
+				       struct fid *fid, int fh_len, int fh_type)
+{
+	if ((fh_len != 3 && fh_len != 5) ||
+	    (fh_type != FILEID_UDF_WITH_PARENT &&
+	     fh_type != FILEID_UDF_WITHOUT_PARENT))
+		return NULL;
+
+	return udf_nfs_get_inode(sb, fid->udf.block, fid->udf.partref,
+			fid->udf.generation);
+}
+
+static struct dentry *udf_fh_to_parent(struct super_block *sb,
+				       struct fid *fid, int fh_len, int fh_type)
+{
+	if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT)
+		return NULL;
+
+	return udf_nfs_get_inode(sb, fid->udf.parent_block,
+				 fid->udf.parent_partref,
+				 fid->udf.parent_generation);
+}
+static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
+			 int connectable)
+{
+	int len = *lenp;
+	struct inode *inode =  de->d_inode;
+	kernel_lb_addr location = UDF_I(inode)->i_location;
+	struct fid *fid = (struct fid *)fh;
+	int type = FILEID_UDF_WITHOUT_PARENT;
+
+	if (len < 3 || (connectable && len < 5))
+		return 255;
+
+	*lenp = 3;
+	fid->udf.block = location.logicalBlockNum;
+	fid->udf.partref = location.partitionReferenceNum;
+	fid->udf.generation = inode->i_generation;
+
+	if (connectable && !S_ISDIR(inode->i_mode)) {
+		spin_lock(&de->d_lock);
+		inode = de->d_parent->d_inode;
+		location = UDF_I(inode)->i_location;
+		fid->udf.parent_block = location.logicalBlockNum;
+		fid->udf.parent_partref = location.partitionReferenceNum;
+		fid->udf.parent_generation = inode->i_generation;
+		spin_unlock(&de->d_lock);
+		*lenp = 5;
+		type = FILEID_UDF_WITH_PARENT;
+	}
+
+	return type;
+}
+
+const struct export_operations udf_export_ops = {
+	.encode_fh	= udf_encode_fh,
+	.fh_to_dentry   = udf_fh_to_dentry,
+	.fh_to_parent   = udf_fh_to_parent,
+	.get_parent     = udf_get_parent,
+};
+
 const struct inode_operations udf_dir_inode_operations = {
 	.lookup				= udf_lookup,
 	.create				= udf_create,
diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/super.c linux-2.6.24-mm1/fs/udf/super.c
--- linux-2.6.24-mm1-vanilla/fs/udf/super.c	2008-02-06 21:23:36.000000000 +0100
+++ linux-2.6.24-mm1/fs/udf/super.c	2008-02-07 07:06:30.000000000 +0100
@@ -1801,6 +1801,7 @@ static int udf_fill_super(struct super_b
 
 	/* Fill in the rest of the superblock */
 	sb->s_op = &udf_sb_ops;
+	sb->s_export_op = &udf_export_ops;
 	sb->dq_op = NULL;
 	sb->s_dirt = 0;
 	sb->s_magic = UDF_SUPER_MAGIC;
diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h linux-2.6.24-mm1/fs/udf/udfdecl.h
--- linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h	2008-02-06 21:23:36.000000000 +0100
+++ linux-2.6.24-mm1/fs/udf/udfdecl.h	2008-02-07 07:11:24.000000000 +0100
@@ -45,6 +45,7 @@ struct task_struct;
 struct buffer_head;
 struct super_block;
 
+extern const struct export_operations udf_export_ops;
 extern const struct inode_operations udf_dir_inode_operations;
 extern const struct file_operations udf_dir_operations;
 extern const struct inode_operations udf_file_inode_operations;
diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/include/linux/exportfs.h linux-2.6.24-mm1/include/linux/exportfs.h
--- linux-2.6.24-mm1-vanilla/include/linux/exportfs.h	2008-02-06 21:23:46.000000000 +0100
+++ linux-2.6.24-mm1/include/linux/exportfs.h	2008-02-06 21:25:06.000000000 +0100
@@ -33,6 +33,19 @@ enum fid_type {
 	 * 32 bit parent directory inode number.
 	 */
 	FILEID_INO32_GEN_PARENT = 2,
+
+	/*
+	 * 32 bit block number, 16 bit partition reference,
+	 * 16 bit unused, 32 bit generation number.
+	 */
+	FILEID_UDF_WITHOUT_PARENT = 0x51,
+
+	/*
+	 * 32 bit block number, 16 bit partition reference,
+	 * 16 bit unused, 32 bit generation number,
+	 * 32 bit parent block number, 32 bit parent generation number
+	 */
+	FILEID_UDF_WITH_PARENT = 0x52,
 };
 
 struct fid {
@@ -43,6 +56,14 @@ struct fid {
 			u32 parent_ino;
 			u32 parent_gen;
 		} i32;
+		struct {
+			u32 block;
+			u16 partref;
+			u16 parent_partref;
+			u32 generation;
+			u32 parent_block;
+			u32 parent_generation;
+		} udf;
 		__u32 raw[6];
 	};
 };



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
NFS maillist  -  NFS-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that nfs-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org is being discontinued.
Please subscribe to linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org instead.
    http://vger.kernel.org/vger-lists.html#linux-nfs

-
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [NFS] [PATCH] Make UDF exportable

[Jan Kara]

On Thu 07-02-08 16:02:57, Rasmus Rohde wrote:
> 
> > > Before posting the last and hopefully final patch I'd like to know what
> > > Jan says about open coding the lookup for ..
> > > It will mean a lot of code duplication and I think it makes good sense
> > > for udf_find_entry to be able to handle ..
> >   Yes, I think opencoding it would really lead to larger code duplication
> > than I'd like so please keep the change in udf_find_entry(). Thanks.
> Great - then I think we are hopefully at a patch that can be accepted.
  Yeah, the patch looks fine. Thanks for your work. I'll send it to Andrew.

> Signed-off-by: Rasmus Rohde <rohde@duff.dk>
> 
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/namei.c linux-2.6.24-mm1/fs/udf/namei.c
> --- linux-2.6.24-mm1-vanilla/fs/udf/namei.c	2008-02-06 21:23:36.000000000 +0100
> +++ linux-2.6.24-mm1/fs/udf/namei.c	2008-02-07 07:13:04.000000000 +0100
> @@ -31,6 +31,7 @@
>  #include <linux/smp_lock.h>
>  #include <linux/buffer_head.h>
>  #include <linux/sched.h>
> +#include <linux/exportfs.h>
>  
>  static inline int udf_match(int len1, const char *name1, int len2,
>  			    const char *name2)
> @@ -159,6 +160,8 @@ static struct fileIdentDesc *udf_find_en
>  	sector_t offset;
>  	struct extent_position epos = {};
>  	struct udf_inode_info *dinfo = UDF_I(dir);
> +	int isdotdot = dentry->d_name.len == 2 &&
> +		dentry->d_name.name[0] == '.' && dentry->d_name.name[1] == '.';
>  
>  	size = udf_ext0_offset(dir) + dir->i_size;
>  	f_pos = udf_ext0_offset(dir);
> @@ -232,6 +235,12 @@ static struct fileIdentDesc *udf_find_en
>  				continue;
>  		}
>  
> +		if ((cfi->fileCharacteristics & FID_FILE_CHAR_PARENT) &&
> +		    isdotdot) {
> +			brelse(epos.bh);
> +			return fi;
> +		}
> +
>  		if (!lfi)
>  			continue;
>  
> @@ -324,9 +333,8 @@ static struct dentry *udf_lookup(struct 
>  		}
>  	}
>  	unlock_kernel();
> -	d_add(dentry, inode);
>  
> -	return NULL;
> +	return d_splice_alias(inode, dentry);
>  }
>  
>  static struct fileIdentDesc *udf_add_entry(struct inode *dir,
> @@ -1298,6 +1306,134 @@ end_rename:
>  	return retval;
>  }
>  
> +static struct dentry *udf_get_parent(struct dentry *child)
> +{
> +	struct dentry *parent;
> +	struct inode *inode = NULL;
> +	struct dentry dotdot;
> +	struct fileIdentDesc cfi;
> +	struct udf_fileident_bh fibh;
> +
> +	dotdot.d_name.name = "..";
> +	dotdot.d_name.len = 2;
> +
> +	lock_kernel();
> +	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
> +		goto out_unlock;
> +
> +	if (fibh.sbh != fibh.ebh)
> +		brelse(fibh.ebh);
> +	brelse(fibh.sbh);
> +
> +	inode = udf_iget(child->d_inode->i_sb,
> +			 lelb_to_cpu(cfi.icb.extLocation));
> +	if (!inode)
> +		goto out_unlock;
> +	unlock_kernel();
> +
> +	parent = d_alloc_anon(inode);
> +	if (!parent) {
> +		iput(inode);
> +		parent = ERR_PTR(-ENOMEM);
> +	}
> +
> +	return parent;
> +out_unlock:
> +	unlock_kernel();
> +	return ERR_PTR(-EACCES);
> +}
> +
> +
> +static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
> +					u16 partref, __u32 generation)
> +{
> +	struct inode *inode;
> +	struct dentry *result;
> +	kernel_lb_addr loc;
> +
> +	if (block == 0)
> +		return ERR_PTR(-ESTALE);
> +
> +	loc.logicalBlockNum = block;
> +	loc.partitionReferenceNum = partref;
> +	inode = udf_iget(sb, loc);
> +
> +	if (inode == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (generation && inode->i_generation != generation) {
> +		iput(inode);
> +		return ERR_PTR(-ESTALE);
> +	}
> +	result = d_alloc_anon(inode);
> +	if (!result) {
> +		iput(inode);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	return result;
> +}
> +
> +static struct dentry *udf_fh_to_dentry(struct super_block *sb,
> +				       struct fid *fid, int fh_len, int fh_type)
> +{
> +	if ((fh_len != 3 && fh_len != 5) ||
> +	    (fh_type != FILEID_UDF_WITH_PARENT &&
> +	     fh_type != FILEID_UDF_WITHOUT_PARENT))
> +		return NULL;
> +
> +	return udf_nfs_get_inode(sb, fid->udf.block, fid->udf.partref,
> +			fid->udf.generation);
> +}
> +
> +static struct dentry *udf_fh_to_parent(struct super_block *sb,
> +				       struct fid *fid, int fh_len, int fh_type)
> +{
> +	if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT)
> +		return NULL;
> +
> +	return udf_nfs_get_inode(sb, fid->udf.parent_block,
> +				 fid->udf.parent_partref,
> +				 fid->udf.parent_generation);
> +}
> +static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
> +			 int connectable)
> +{
> +	int len = *lenp;
> +	struct inode *inode =  de->d_inode;
> +	kernel_lb_addr location = UDF_I(inode)->i_location;
> +	struct fid *fid = (struct fid *)fh;
> +	int type = FILEID_UDF_WITHOUT_PARENT;
> +
> +	if (len < 3 || (connectable && len < 5))
> +		return 255;
> +
> +	*lenp = 3;
> +	fid->udf.block = location.logicalBlockNum;
> +	fid->udf.partref = location.partitionReferenceNum;
> +	fid->udf.generation = inode->i_generation;
> +
> +	if (connectable && !S_ISDIR(inode->i_mode)) {
> +		spin_lock(&de->d_lock);
> +		inode = de->d_parent->d_inode;
> +		location = UDF_I(inode)->i_location;
> +		fid->udf.parent_block = location.logicalBlockNum;
> +		fid->udf.parent_partref = location.partitionReferenceNum;
> +		fid->udf.parent_generation = inode->i_generation;
> +		spin_unlock(&de->d_lock);
> +		*lenp = 5;
> +		type = FILEID_UDF_WITH_PARENT;
> +	}
> +
> +	return type;
> +}
> +
> +const struct export_operations udf_export_ops = {
> +	.encode_fh	= udf_encode_fh,
> +	.fh_to_dentry   = udf_fh_to_dentry,
> +	.fh_to_parent   = udf_fh_to_parent,
> +	.get_parent     = udf_get_parent,
> +};
> +
>  const struct inode_operations udf_dir_inode_operations = {
>  	.lookup				= udf_lookup,
>  	.create				= udf_create,
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/super.c linux-2.6.24-mm1/fs/udf/super.c
> --- linux-2.6.24-mm1-vanilla/fs/udf/super.c	2008-02-06 21:23:36.000000000 +0100
> +++ linux-2.6.24-mm1/fs/udf/super.c	2008-02-07 07:06:30.000000000 +0100
> @@ -1801,6 +1801,7 @@ static int udf_fill_super(struct super_b
>  
>  	/* Fill in the rest of the superblock */
>  	sb->s_op = &udf_sb_ops;
> +	sb->s_export_op = &udf_export_ops;
>  	sb->dq_op = NULL;
>  	sb->s_dirt = 0;
>  	sb->s_magic = UDF_SUPER_MAGIC;
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h linux-2.6.24-mm1/fs/udf/udfdecl.h
> --- linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h	2008-02-06 21:23:36.000000000 +0100
> +++ linux-2.6.24-mm1/fs/udf/udfdecl.h	2008-02-07 07:11:24.000000000 +0100
> @@ -45,6 +45,7 @@ struct task_struct;
>  struct buffer_head;
>  struct super_block;
>  
> +extern const struct export_operations udf_export_ops;
>  extern const struct inode_operations udf_dir_inode_operations;
>  extern const struct file_operations udf_dir_operations;
>  extern const struct inode_operations udf_file_inode_operations;
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/include/linux/exportfs.h linux-2.6.24-mm1/include/linux/exportfs.h
> --- linux-2.6.24-mm1-vanilla/include/linux/exportfs.h	2008-02-06 21:23:46.000000000 +0100
> +++ linux-2.6.24-mm1/include/linux/exportfs.h	2008-02-06 21:25:06.000000000 +0100
> @@ -33,6 +33,19 @@ enum fid_type {
>  	 * 32 bit parent directory inode number.
>  	 */
>  	FILEID_INO32_GEN_PARENT = 2,
> +
> +	/*
> +	 * 32 bit block number, 16 bit partition reference,
> +	 * 16 bit unused, 32 bit generation number.
> +	 */
> +	FILEID_UDF_WITHOUT_PARENT = 0x51,
> +
> +	/*
> +	 * 32 bit block number, 16 bit partition reference,
> +	 * 16 bit unused, 32 bit generation number,
> +	 * 32 bit parent block number, 32 bit parent generation number
> +	 */
> +	FILEID_UDF_WITH_PARENT = 0x52,
>  };
>  
>  struct fid {
> @@ -43,6 +56,14 @@ struct fid {
>  			u32 parent_ino;
>  			u32 parent_gen;
>  		} i32;
> +		struct {
> +			u32 block;
> +			u16 partref;
> +			u16 parent_partref;
> +			u32 generation;
> +			u32 parent_block;
> +			u32 parent_generation;
> +		} udf;
>  		__u32 raw[6];
>  	};
>  };
> 
> 
									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [NFS] [PATCH] Make UDF exportable

[Christoph Hellwig]

On Thu, Feb 07, 2008 at 04:02:57PM +0100, Rasmus Rohde wrote:
> 
> > > Before posting the last and hopefully final patch I'd like to know what
> > > Jan says about open coding the lookup for ..
> > > It will mean a lot of code duplication and I think it makes good sense
> > > for udf_find_entry to be able to handle ..
> >   Yes, I think opencoding it would really lead to larger code duplication
> > than I'd like so please keep the change in udf_find_entry(). Thanks.
> Great - then I think we are hopefully at a patch that can be accepted.

Jan, any reason this patch didn't go in with the last merge?  I'd really
like to see it in .26.

> 
> Signed-off-by: Rasmus Rohde <rohde@duff.dk>
> 
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/namei.c linux-2.6.24-mm1/fs/udf/namei.c
> --- linux-2.6.24-mm1-vanilla/fs/udf/namei.c	2008-02-06 21:23:36.000000000 +0100
> +++ linux-2.6.24-mm1/fs/udf/namei.c	2008-02-07 07:13:04.000000000 +0100
> @@ -31,6 +31,7 @@
>  #include <linux/smp_lock.h>
>  #include <linux/buffer_head.h>
>  #include <linux/sched.h>
> +#include <linux/exportfs.h>
>  
>  static inline int udf_match(int len1, const char *name1, int len2,
>  			    const char *name2)
> @@ -159,6 +160,8 @@ static struct fileIdentDesc *udf_find_en
>  	sector_t offset;
>  	struct extent_position epos = {};
>  	struct udf_inode_info *dinfo = UDF_I(dir);
> +	int isdotdot = dentry->d_name.len == 2 &&
> +		dentry->d_name.name[0] == '.' && dentry->d_name.name[1] == '.';
>  
>  	size = udf_ext0_offset(dir) + dir->i_size;
>  	f_pos = udf_ext0_offset(dir);
> @@ -232,6 +235,12 @@ static struct fileIdentDesc *udf_find_en
>  				continue;
>  		}
>  
> +		if ((cfi->fileCharacteristics & FID_FILE_CHAR_PARENT) &&
> +		    isdotdot) {
> +			brelse(epos.bh);
> +			return fi;
> +		}
> +
>  		if (!lfi)
>  			continue;
>  
> @@ -324,9 +333,8 @@ static struct dentry *udf_lookup(struct 
>  		}
>  	}
>  	unlock_kernel();
> -	d_add(dentry, inode);
>  
> -	return NULL;
> +	return d_splice_alias(inode, dentry);
>  }
>  
>  static struct fileIdentDesc *udf_add_entry(struct inode *dir,
> @@ -1298,6 +1306,134 @@ end_rename:
>  	return retval;
>  }
>  
> +static struct dentry *udf_get_parent(struct dentry *child)
> +{
> +	struct dentry *parent;
> +	struct inode *inode = NULL;
> +	struct dentry dotdot;
> +	struct fileIdentDesc cfi;
> +	struct udf_fileident_bh fibh;
> +
> +	dotdot.d_name.name = "..";
> +	dotdot.d_name.len = 2;
> +
> +	lock_kernel();
> +	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
> +		goto out_unlock;
> +
> +	if (fibh.sbh != fibh.ebh)
> +		brelse(fibh.ebh);
> +	brelse(fibh.sbh);
> +
> +	inode = udf_iget(child->d_inode->i_sb,
> +			 lelb_to_cpu(cfi.icb.extLocation));
> +	if (!inode)
> +		goto out_unlock;
> +	unlock_kernel();
> +
> +	parent = d_alloc_anon(inode);
> +	if (!parent) {
> +		iput(inode);
> +		parent = ERR_PTR(-ENOMEM);
> +	}
> +
> +	return parent;
> +out_unlock:
> +	unlock_kernel();
> +	return ERR_PTR(-EACCES);
> +}
> +
> +
> +static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
> +					u16 partref, __u32 generation)
> +{
> +	struct inode *inode;
> +	struct dentry *result;
> +	kernel_lb_addr loc;
> +
> +	if (block == 0)
> +		return ERR_PTR(-ESTALE);
> +
> +	loc.logicalBlockNum = block;
> +	loc.partitionReferenceNum = partref;
> +	inode = udf_iget(sb, loc);
> +
> +	if (inode == NULL)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (generation && inode->i_generation != generation) {
> +		iput(inode);
> +		return ERR_PTR(-ESTALE);
> +	}
> +	result = d_alloc_anon(inode);
> +	if (!result) {
> +		iput(inode);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	return result;
> +}
> +
> +static struct dentry *udf_fh_to_dentry(struct super_block *sb,
> +				       struct fid *fid, int fh_len, int fh_type)
> +{
> +	if ((fh_len != 3 && fh_len != 5) ||
> +	    (fh_type != FILEID_UDF_WITH_PARENT &&
> +	     fh_type != FILEID_UDF_WITHOUT_PARENT))
> +		return NULL;
> +
> +	return udf_nfs_get_inode(sb, fid->udf.block, fid->udf.partref,
> +			fid->udf.generation);
> +}
> +
> +static struct dentry *udf_fh_to_parent(struct super_block *sb,
> +				       struct fid *fid, int fh_len, int fh_type)
> +{
> +	if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT)
> +		return NULL;
> +
> +	return udf_nfs_get_inode(sb, fid->udf.parent_block,
> +				 fid->udf.parent_partref,
> +				 fid->udf.parent_generation);
> +}
> +static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
> +			 int connectable)
> +{
> +	int len = *lenp;
> +	struct inode *inode =  de->d_inode;
> +	kernel_lb_addr location = UDF_I(inode)->i_location;
> +	struct fid *fid = (struct fid *)fh;
> +	int type = FILEID_UDF_WITHOUT_PARENT;
> +
> +	if (len < 3 || (connectable && len < 5))
> +		return 255;
> +
> +	*lenp = 3;
> +	fid->udf.block = location.logicalBlockNum;
> +	fid->udf.partref = location.partitionReferenceNum;
> +	fid->udf.generation = inode->i_generation;
> +
> +	if (connectable && !S_ISDIR(inode->i_mode)) {
> +		spin_lock(&de->d_lock);
> +		inode = de->d_parent->d_inode;
> +		location = UDF_I(inode)->i_location;
> +		fid->udf.parent_block = location.logicalBlockNum;
> +		fid->udf.parent_partref = location.partitionReferenceNum;
> +		fid->udf.parent_generation = inode->i_generation;
> +		spin_unlock(&de->d_lock);
> +		*lenp = 5;
> +		type = FILEID_UDF_WITH_PARENT;
> +	}
> +
> +	return type;
> +}
> +
> +const struct export_operations udf_export_ops = {
> +	.encode_fh	= udf_encode_fh,
> +	.fh_to_dentry   = udf_fh_to_dentry,
> +	.fh_to_parent   = udf_fh_to_parent,
> +	.get_parent     = udf_get_parent,
> +};
> +
>  const struct inode_operations udf_dir_inode_operations = {
>  	.lookup				= udf_lookup,
>  	.create				= udf_create,
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/super.c linux-2.6.24-mm1/fs/udf/super.c
> --- linux-2.6.24-mm1-vanilla/fs/udf/super.c	2008-02-06 21:23:36.000000000 +0100
> +++ linux-2.6.24-mm1/fs/udf/super.c	2008-02-07 07:06:30.000000000 +0100
> @@ -1801,6 +1801,7 @@ static int udf_fill_super(struct super_b
>  
>  	/* Fill in the rest of the superblock */
>  	sb->s_op = &udf_sb_ops;
> +	sb->s_export_op = &udf_export_ops;
>  	sb->dq_op = NULL;
>  	sb->s_dirt = 0;
>  	sb->s_magic = UDF_SUPER_MAGIC;
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h linux-2.6.24-mm1/fs/udf/udfdecl.h
> --- linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h	2008-02-06 21:23:36.000000000 +0100
> +++ linux-2.6.24-mm1/fs/udf/udfdecl.h	2008-02-07 07:11:24.000000000 +0100
> @@ -45,6 +45,7 @@ struct task_struct;
>  struct buffer_head;
>  struct super_block;
>  
> +extern const struct export_operations udf_export_ops;
>  extern const struct inode_operations udf_dir_inode_operations;
>  extern const struct file_operations udf_dir_operations;
>  extern const struct inode_operations udf_file_inode_operations;
> diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/include/linux/exportfs.h linux-2.6.24-mm1/include/linux/exportfs.h
> --- linux-2.6.24-mm1-vanilla/include/linux/exportfs.h	2008-02-06 21:23:46.000000000 +0100
> +++ linux-2.6.24-mm1/include/linux/exportfs.h	2008-02-06 21:25:06.000000000 +0100
> @@ -33,6 +33,19 @@ enum fid_type {
>  	 * 32 bit parent directory inode number.
>  	 */
>  	FILEID_INO32_GEN_PARENT = 2,
> +
> +	/*
> +	 * 32 bit block number, 16 bit partition reference,
> +	 * 16 bit unused, 32 bit generation number.
> +	 */
> +	FILEID_UDF_WITHOUT_PARENT = 0x51,
> +
> +	/*
> +	 * 32 bit block number, 16 bit partition reference,
> +	 * 16 bit unused, 32 bit generation number,
> +	 * 32 bit parent block number, 32 bit parent generation number
> +	 */
> +	FILEID_UDF_WITH_PARENT = 0x52,
>  };
>  
>  struct fid {
> @@ -43,6 +56,14 @@ struct fid {
>  			u32 parent_ino;
>  			u32 parent_gen;
>  		} i32;
> +		struct {
> +			u32 block;
> +			u16 partref;
> +			u16 parent_partref;
> +			u32 generation;
> +			u32 parent_block;
> +			u32 parent_generation;
> +		} udf;
>  		__u32 raw[6];
>  	};
>  };
> 
> 
---end quoted text---

Re: [NFS] [PATCH] Make UDF exportable

[Jan Kara]

On Tue 29-04-08 10:33:31, Christoph Hellwig wrote:
> On Thu, Feb 07, 2008 at 04:02:57PM +0100, Rasmus Rohde wrote:
> > 
> > > > Before posting the last and hopefully final patch I'd like to know what
> > > > Jan says about open coding the lookup for ..
> > > > It will mean a lot of code duplication and I think it makes good sense
> > > > for udf_find_entry to be able to handle ..
> > >   Yes, I think opencoding it would really lead to larger code duplication
> > > than I'd like so please keep the change in udf_find_entry(). Thanks.
> > Great - then I think we are hopefully at a patch that can be accepted.
> 
> Jan, any reason this patch didn't go in with the last merge?  I'd really
> like to see it in .26.
  Thanks for catching this. It seems I missed the patch when merging all
the UDF patches I had in my mailbox. I have it merged in my git tree and
will push it to Linus with other patches.

								Honza
> 
> > 
> > Signed-off-by: Rasmus Rohde <rohde@duff.dk>
> > 
> > diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/namei.c linux-2.6.24-mm1/fs/udf/namei.c
> > --- linux-2.6.24-mm1-vanilla/fs/udf/namei.c	2008-02-06 21:23:36.000000000 +0100
> > +++ linux-2.6.24-mm1/fs/udf/namei.c	2008-02-07 07:13:04.000000000 +0100
> > @@ -31,6 +31,7 @@
> >  #include <linux/smp_lock.h>
> >  #include <linux/buffer_head.h>
> >  #include <linux/sched.h>
> > +#include <linux/exportfs.h>
> >  
> >  static inline int udf_match(int len1, const char *name1, int len2,
> >  			    const char *name2)
> > @@ -159,6 +160,8 @@ static struct fileIdentDesc *udf_find_en
> >  	sector_t offset;
> >  	struct extent_position epos = {};
> >  	struct udf_inode_info *dinfo = UDF_I(dir);
> > +	int isdotdot = dentry->d_name.len == 2 &&
> > +		dentry->d_name.name[0] == '.' && dentry->d_name.name[1] == '.';
> >  
> >  	size = udf_ext0_offset(dir) + dir->i_size;
> >  	f_pos = udf_ext0_offset(dir);
> > @@ -232,6 +235,12 @@ static struct fileIdentDesc *udf_find_en
> >  				continue;
> >  		}
> >  
> > +		if ((cfi->fileCharacteristics & FID_FILE_CHAR_PARENT) &&
> > +		    isdotdot) {
> > +			brelse(epos.bh);
> > +			return fi;
> > +		}
> > +
> >  		if (!lfi)
> >  			continue;
> >  
> > @@ -324,9 +333,8 @@ static struct dentry *udf_lookup(struct 
> >  		}
> >  	}
> >  	unlock_kernel();
> > -	d_add(dentry, inode);
> >  
> > -	return NULL;
> > +	return d_splice_alias(inode, dentry);
> >  }
> >  
> >  static struct fileIdentDesc *udf_add_entry(struct inode *dir,
> > @@ -1298,6 +1306,134 @@ end_rename:
> >  	return retval;
> >  }
> >  
> > +static struct dentry *udf_get_parent(struct dentry *child)
> > +{
> > +	struct dentry *parent;
> > +	struct inode *inode = NULL;
> > +	struct dentry dotdot;
> > +	struct fileIdentDesc cfi;
> > +	struct udf_fileident_bh fibh;
> > +
> > +	dotdot.d_name.name = "..";
> > +	dotdot.d_name.len = 2;
> > +
> > +	lock_kernel();
> > +	if (!udf_find_entry(child->d_inode, &dotdot, &fibh, &cfi))
> > +		goto out_unlock;
> > +
> > +	if (fibh.sbh != fibh.ebh)
> > +		brelse(fibh.ebh);
> > +	brelse(fibh.sbh);
> > +
> > +	inode = udf_iget(child->d_inode->i_sb,
> > +			 lelb_to_cpu(cfi.icb.extLocation));
> > +	if (!inode)
> > +		goto out_unlock;
> > +	unlock_kernel();
> > +
> > +	parent = d_alloc_anon(inode);
> > +	if (!parent) {
> > +		iput(inode);
> > +		parent = ERR_PTR(-ENOMEM);
> > +	}
> > +
> > +	return parent;
> > +out_unlock:
> > +	unlock_kernel();
> > +	return ERR_PTR(-EACCES);
> > +}
> > +
> > +
> > +static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
> > +					u16 partref, __u32 generation)
> > +{
> > +	struct inode *inode;
> > +	struct dentry *result;
> > +	kernel_lb_addr loc;
> > +
> > +	if (block == 0)
> > +		return ERR_PTR(-ESTALE);
> > +
> > +	loc.logicalBlockNum = block;
> > +	loc.partitionReferenceNum = partref;
> > +	inode = udf_iget(sb, loc);
> > +
> > +	if (inode == NULL)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	if (generation && inode->i_generation != generation) {
> > +		iput(inode);
> > +		return ERR_PTR(-ESTALE);
> > +	}
> > +	result = d_alloc_anon(inode);
> > +	if (!result) {
> > +		iput(inode);
> > +		return ERR_PTR(-ENOMEM);
> > +	}
> > +	return result;
> > +}
> > +
> > +static struct dentry *udf_fh_to_dentry(struct super_block *sb,
> > +				       struct fid *fid, int fh_len, int fh_type)
> > +{
> > +	if ((fh_len != 3 && fh_len != 5) ||
> > +	    (fh_type != FILEID_UDF_WITH_PARENT &&
> > +	     fh_type != FILEID_UDF_WITHOUT_PARENT))
> > +		return NULL;
> > +
> > +	return udf_nfs_get_inode(sb, fid->udf.block, fid->udf.partref,
> > +			fid->udf.generation);
> > +}
> > +
> > +static struct dentry *udf_fh_to_parent(struct super_block *sb,
> > +				       struct fid *fid, int fh_len, int fh_type)
> > +{
> > +	if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT)
> > +		return NULL;
> > +
> > +	return udf_nfs_get_inode(sb, fid->udf.parent_block,
> > +				 fid->udf.parent_partref,
> > +				 fid->udf.parent_generation);
> > +}
> > +static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
> > +			 int connectable)
> > +{
> > +	int len = *lenp;
> > +	struct inode *inode =  de->d_inode;
> > +	kernel_lb_addr location = UDF_I(inode)->i_location;
> > +	struct fid *fid = (struct fid *)fh;
> > +	int type = FILEID_UDF_WITHOUT_PARENT;
> > +
> > +	if (len < 3 || (connectable && len < 5))
> > +		return 255;
> > +
> > +	*lenp = 3;
> > +	fid->udf.block = location.logicalBlockNum;
> > +	fid->udf.partref = location.partitionReferenceNum;
> > +	fid->udf.generation = inode->i_generation;
> > +
> > +	if (connectable && !S_ISDIR(inode->i_mode)) {
> > +		spin_lock(&de->d_lock);
> > +		inode = de->d_parent->d_inode;
> > +		location = UDF_I(inode)->i_location;
> > +		fid->udf.parent_block = location.logicalBlockNum;
> > +		fid->udf.parent_partref = location.partitionReferenceNum;
> > +		fid->udf.parent_generation = inode->i_generation;
> > +		spin_unlock(&de->d_lock);
> > +		*lenp = 5;
> > +		type = FILEID_UDF_WITH_PARENT;
> > +	}
> > +
> > +	return type;
> > +}
> > +
> > +const struct export_operations udf_export_ops = {
> > +	.encode_fh	= udf_encode_fh,
> > +	.fh_to_dentry   = udf_fh_to_dentry,
> > +	.fh_to_parent   = udf_fh_to_parent,
> > +	.get_parent     = udf_get_parent,
> > +};
> > +
> >  const struct inode_operations udf_dir_inode_operations = {
> >  	.lookup				= udf_lookup,
> >  	.create				= udf_create,
> > diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/super.c linux-2.6.24-mm1/fs/udf/super.c
> > --- linux-2.6.24-mm1-vanilla/fs/udf/super.c	2008-02-06 21:23:36.000000000 +0100
> > +++ linux-2.6.24-mm1/fs/udf/super.c	2008-02-07 07:06:30.000000000 +0100
> > @@ -1801,6 +1801,7 @@ static int udf_fill_super(struct super_b
> >  
> >  	/* Fill in the rest of the superblock */
> >  	sb->s_op = &udf_sb_ops;
> > +	sb->s_export_op = &udf_export_ops;
> >  	sb->dq_op = NULL;
> >  	sb->s_dirt = 0;
> >  	sb->s_magic = UDF_SUPER_MAGIC;
> > diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h linux-2.6.24-mm1/fs/udf/udfdecl.h
> > --- linux-2.6.24-mm1-vanilla/fs/udf/udfdecl.h	2008-02-06 21:23:36.000000000 +0100
> > +++ linux-2.6.24-mm1/fs/udf/udfdecl.h	2008-02-07 07:11:24.000000000 +0100
> > @@ -45,6 +45,7 @@ struct task_struct;
> >  struct buffer_head;
> >  struct super_block;
> >  
> > +extern const struct export_operations udf_export_ops;
> >  extern const struct inode_operations udf_dir_inode_operations;
> >  extern const struct file_operations udf_dir_operations;
> >  extern const struct inode_operations udf_file_inode_operations;
> > diff -uprN -X linux-2.6.24-mm1-vanilla/Documentation/dontdiff linux-2.6.24-mm1-vanilla/include/linux/exportfs.h linux-2.6.24-mm1/include/linux/exportfs.h
> > --- linux-2.6.24-mm1-vanilla/include/linux/exportfs.h	2008-02-06 21:23:46.000000000 +0100
> > +++ linux-2.6.24-mm1/include/linux/exportfs.h	2008-02-06 21:25:06.000000000 +0100
> > @@ -33,6 +33,19 @@ enum fid_type {
> >  	 * 32 bit parent directory inode number.
> >  	 */
> >  	FILEID_INO32_GEN_PARENT = 2,
> > +
> > +	/*
> > +	 * 32 bit block number, 16 bit partition reference,
> > +	 * 16 bit unused, 32 bit generation number.
> > +	 */
> > +	FILEID_UDF_WITHOUT_PARENT = 0x51,
> > +
> > +	/*
> > +	 * 32 bit block number, 16 bit partition reference,
> > +	 * 16 bit unused, 32 bit generation number,
> > +	 * 32 bit parent block number, 32 bit parent generation number
> > +	 */
> > +	FILEID_UDF_WITH_PARENT = 0x52,
> >  };
> >  
> >  struct fid {
> > @@ -43,6 +56,14 @@ struct fid {
> >  			u32 parent_ino;
> >  			u32 parent_gen;
> >  		} i32;
> > +		struct {
> > +			u32 block;
> > +			u16 partref;
> > +			u16 parent_partref;
> > +			u32 generation;
> > +			u32 parent_block;
> > +			u32 parent_generation;
> > +		} udf;
> >  		__u32 raw[6];
> >  	};
> >  };
> > 
> > 
> ---end quoted text---
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR