> > +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@xxxxxxx>
--- 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
