2008-01-30 20:54:12

by Rasmus Rohde

[permalink] [raw]
Subject: [NFS] [PATCH] Make UDF exportable

I've cooked together a patch for making UDF exportable.

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

--- fs/udf/namei.c.orig 2007-10-10 16:22:30.000000000 +0200
+++ fs/udf/namei.c 2008-01-30 21:39:00.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,151 @@ end_rename:
return retval;
}

+static struct dentry *udf_export_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)) {
+ 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();
+
+ parent = d_alloc_anon(inode);
+ if (!parent) {
+ iput(inode);
+ parent = ERR_PTR(-ENOMEM);
+ }
+
+ return parent;
+}
+
+
+static struct dentry *
+udf_export_iget(struct super_block *sb, u32 block,
+ u16 offset, __u32 generation)
+{
+ 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))
+ {
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
+ result = d_alloc_anon(inode);
+ if (!result) {
+ iput(inode);
+ return ERR_PTR(-ENOMEM);
+ }
+ return result;
+}
+
+
+struct udf_fid {
+ u32 block;
+ u16 partref;
+ u16 parent_partref;
+ u32 generation;
+ u32 parent_block;
+ u32 parent_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;
+
+ return udf_export_iget(sb, ufid->block, ufid->partref,
+ ufid->generation);
+}
+
+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;
+
+ return udf_export_iget(sb,
+ fh_len > 2 ? ufid->parent_block : 0,
+ ufid->parent_partref,
+ fh_len > 4 ? ufid->parent_generation : 0);
+}
+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 udf_fid *ufid = (struct udf_fid *)fh;
+ int type = 1;
+
+ if (len < 3 || (connectable && len < 5))
+ return 255;
+
+ *lenp = 3;
+ ufid->block = location.logicalBlockNum;
+ ufid->partref = location.partitionReferenceNum;
+ ufid->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);
+ ufid->parent_block = location.logicalBlockNum;
+ ufid->parent_partref = location.partitionReferenceNum;
+ ufid->parent_generation = inode->i_generation;
+ spin_unlock(&de->d_lock);
+ *lenp = 5;
+ type = 2;
+ }
+
+ 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_export_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;



-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs



2008-02-05 10:30:15

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

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!

-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-02-05 19:26:39

by Rasmus Rohde

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

> +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.


-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-02-06 20:58:32

by Rasmus Rohde

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

> > + 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 <[email protected]>

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];
};
};



-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-02-07 03:38:13

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

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.


-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-02-07 03:44:19

by NeilBrown

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

On Wednesday February 6, [email protected] 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


-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-02-07 05:45:37

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

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


-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-02-07 07:06:59

by Rasmus Rohde

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

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 ..


-------------------------------------------------------------------------
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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-02-07 15:03:34

by Rasmus Rohde

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable


> > 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 <[email protected]>

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 - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-04-30 15:42:36

by Jan Kara

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

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 <[email protected]>
> >
> > 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 <[email protected]>
SUSE Labs, CR

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs


2008-04-29 14:58:07

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [NFS] [PATCH] Make UDF exportable

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 <[email protected]>
>
> 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---

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued.
Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs