[PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[David Howells]

Update the exportfs documentation and comments to fit the code, and add an
extra FID type to be used for error indication (FILEID_ERROR).

The following other code changes have been made:

 (*) The encode_fh() op and exportfs_encode_fh() return enum fid_type rather
     than int.

 (*) The exportfs_encode_fh() function has it's acceptable() function pointer
     typedef'd.

 (*) The fh_to_dentry() and fh_to_parent() ops and exportfs_decode_fh() take
     enum fid_type rather than int.

 (*) The generic_fh_to_dentry() and generic_fh_to_parent() functions now take
     enum fid_type rather than int.  Their get_inode() function pointer has
     been typedef'd.

 (*) The generic_fh_to_dentry() and generic_fh_to_parent() functions no longer
     return NULL, but return -ESTALE instead.  exportfs_decode_fh() does not
     check for NULL, but will try to dereference it as IS_ERR() does not check
     for it.

 (*) Returns from encode_fh() implementations of 255 are changed to
     FILEID_ERROR.

 (*) FAT has had its fh type #defined (FAT_FILEID).

 (*) FUSE has had its fh types #defined (FUSE_FILEID, FUSE_FILEID_PARENT).

 (*) ISOFS has had its fh types #defined (ISOFS_FILEID, ISOFS_FILEID_PARENT).

 (*) OCFS2 has had its fh types #defined (OCFS2_FILEID, OCFS2_FILEID_PARENT).

 (*) The following filesystems have had their fh_to_dentry() and fh_to_parent()
     implementations made to return -ESTALE rather than NULL: FUSE, GFS2,
     ISOFS, OCFS2, UDF and XFS.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/filesystems/Exporting |  400 +++++++++++++++++++++++------------
 fs/efs/efs.h                        |    5 
 fs/efs/namei.c                      |    4 
 fs/exportfs/expfs.c                 |   95 ++++++--
 fs/ext2/super.c                     |    4 
 fs/ext3/super.c                     |    4 
 fs/ext4/super.c                     |    4 
 fs/fat/inode.c                      |   12 +
 fs/fuse/inode.c                     |   22 +-
 fs/gfs2/ops_export.c                |   18 +-
 fs/isofs/export.c                   |   21 +-
 fs/jffs2/super.c                    |    4 
 fs/jfs/jfs_inode.h                  |    5 
 fs/jfs/namei.c                      |    4 
 fs/libfs.c                          |   14 +
 fs/ntfs/namei.c                     |    4 
 fs/ocfs2/export.c                   |   25 +-
 fs/reiserfs/inode.c                 |   10 -
 fs/udf/namei.c                      |   18 +-
 fs/xfs/linux-2.6/xfs_export.c       |   36 ++-
 include/linux/exportfs.h            |  117 +++++++---
 include/linux/reiserfs_fs.h         |    9 -
 mm/shmem.c                          |    6 -
 23 files changed, 538 insertions(+), 303 deletions(-)


diff --git a/Documentation/filesystems/Exporting b/Documentation/filesystems/Exporting
index 87019d2..5d9d769 100644
--- a/Documentation/filesystems/Exporting
+++ b/Documentation/filesystems/Exporting
@@ -1,147 +1,273 @@
+			 =============================
+			 MAKING FILESYSTEMS EXPORTABLE
+			 =============================
 
-Making Filesystems Exportable
-=============================
-
-Overview
---------
-
-All filesystem operations require a dentry (or two) as a starting
-point.  Local applications have a reference-counted hold on suitable
-dentries via open file descriptors or cwd/root.  However remote
-applications that access a filesystem via a remote filesystem protocol
-such as NFS may not be able to hold such a reference, and so need a
-different way to refer to a particular dentry.  As the alternative
-form of reference needs to be stable across renames, truncates, and
-server-reboot (among other things, though these tend to be the most
-problematic), there is no simple answer like 'filename'.
-
-The mechanism discussed here allows each filesystem implementation to
-specify how to generate an opaque (outside of the filesystem) byte
-string for any dentry, and how to find an appropriate dentry for any
-given opaque byte string.
-This byte string will be called a "filehandle fragment" as it
-corresponds to part of an NFS filehandle.
-
-A filesystem which supports the mapping between filehandle fragments
-and dentries will be termed "exportable".
-
-
-
-Dcache Issues
--------------
-
-The dcache normally contains a proper prefix of any given filesystem
-tree.  This means that if any filesystem object is in the dcache, then
-all of the ancestors of that filesystem object are also in the dcache.
-As normal access is by filename this prefix is created naturally and
-maintained easily (by each object maintaining a reference count on
-its parent).
-
-However when objects are included into the dcache by interpreting a
-filehandle fragment, there is no automatic creation of a path prefix
-for the object.  This leads to two related but distinct features of
-the dcache that are not needed for normal filesystem access.
-
-1/ The dcache must sometimes contain objects that are not part of the
-   proper prefix. i.e that are not connected to the root.
-2/ The dcache must be prepared for a newly found (via ->lookup) directory
-   to already have a (non-connected) dentry, and must be able to move
-   that dentry into place (based on the parent and name in the
-   ->lookup).   This is particularly needed for directories as
-   it is a dcache invariant that directories only have one dentry.
-
-To implement these features, the dcache has:
-
-a/ A dentry flag DCACHE_DISCONNECTED which is set on
-   any dentry that might not be part of the proper prefix.
-   This is set when anonymous dentries are created, and cleared when a
-   dentry is noticed to be a child of a dentry which is in the proper
-   prefix. 
-
-b/ A per-superblock list "s_anon" of dentries which are the roots of
-   subtrees that are not in the proper prefix.  These dentries, as
-   well as the proper prefix, need to be released at unmount time.  As
-   these dentries will not be hashed, they are linked together on the
-   d_hash list_head.
-
-c/ Helper routines to allocate anonymous dentries, and to help attach
-   loose directory dentries at lookup time. They are:
-    d_alloc_anon(inode) will return a dentry for the given inode.
-      If the inode already has a dentry, one of those is returned.
-      If it doesn't, a new anonymous (IS_ROOT and
-        DCACHE_DISCONNECTED) dentry is allocated and attached.
-      In the case of a directory, care is taken that only one dentry
-      can ever be attached.
-    d_splice_alias(inode, dentry) will make sure that there is a
-      dentry with the same name and parent as the given dentry, and
-      which refers to the given inode.
-      If the inode is a directory and already has a dentry, then that
-      dentry is d_moved over the given dentry.
-      If the passed dentry gets attached, care is taken that this is
-      mutually exclusive to a d_alloc_anon operation.
-      If the passed dentry is used, NULL is returned, else the used
-      dentry is returned.  This corresponds to the calling pattern of
-      ->lookup.
-  
- 
-Filesystem Issues
------------------
+Contents:
+
+ (*) Overview.
+
+ (*) Dealing with the directory entry cache.
+
+ (*) Making a filesystem exportable.
+
+ (*) Filehandle fragment format.
+
+ (*) Filehandle export operations.
+
+
+========
+OVERVIEW
+========
+
+All filesystem operations require a dentry (or two) as a starting point.  Local
+applications have a reference-counted hold on suitable dentries via open file
+descriptors, the current working directory and the current root directory.
+Remote applications that access a filesystem via a remote filesystem protocol
+such as NFS, however, may not be able to hold such a reference, and so need a
+different way to refer to a particular dentry.  As the alternative form of
+reference needs to be persistent across renames, truncates, and server reboots
+(among other events, though these tend to be the most problematic), there is no
+simple answer like 'filename'.
+
+The mechanism discussed here allows each filesystem implementation to specify
+how to generate an opaque (outside of the filesystem) byte string for any
+dentry, and how to find an appropriate dentry for any given opaque byte string.
+
+This byte string will be called a "filehandle fragment" as it corresponds to
+part of an NFS filehandle.
+
+A filesystem which supports the mapping between filehandle fragments and
+dentries will be termed "exportable".
+
+
+======================================
+DEALING WITH THE DIRECTORY ENTRY CACHE
+======================================
+
+Each superblock has a tree of directory entries (dentries) that is rooted at
+its s_root pointer.  For most filesystems, every dentry it currently has cached
+in RAM is connected to this tree, meaning that all the ancestors of most files
+are fully represented in the directory entry cache (dcache).
+
+As the normal method of accessing files is by filename, the superblock root
+tree builds up in a natural manner, and is maintained simply by each object
+having a reference count on its parent.  The pathwalk algorithm works by
+walking from the root and out along the branches to the desired object.
+
+However, when an object is brought into the dcache by interpreting a filehandle
+fragment, the entries for the directories that represent that object's ancestry
+are not automatically all brought into the cache as well.  This leads to two
+related but distinct issues in the dcache that are not ordinarily seen:
+
+ (1) Sometimes the dcache for a superblock must contain objects that are not
+     connected to that superblock's root tree.
+
+ (2) The dcache must be able to handle a lookup() operation that results in a
+     dentry that already exists.  In such a case, the already extant dentry
+     must be used instead of the candidate upon which the lookup was performed.
+
+     Furthermore, the lookup procedure must be able to handle unconnected
+     dentries so found by installing them in the dentry tree under the parent
+     specified directory.
+
+     This is particularly necessary for directories as it is a requirement of
+     the dcache that directories are represented by a single dentry and don't
+     have aliases.
+
+To manage the above, the dcache has the following features:
+
+ (A) A dentry flag DCACHE_DISCONNECTED which is set on any dentry that might
+     not be connected to the superblock root.  This is set when anonymous
+     dentries are created, and cleared when a dentry is noticed to be a child
+     of a dentry which is in the proper prefix.
+
+ (B) A per-superblock list of dentries (s_anon) which are the roots of subtrees
+     that are not connected to the superblock root.  These dentries, as well as
+     the superblock's rooted tree, need to be released at unmount time.
+
+     Note that as these dentries are unhashed, the s_anon list is linked
+     together using the d_hash list_head in the dentry struct.
+
+ (C) Helper routines to allocate anonymous dentries, and to help attach loose
+     directory dentries at lookup time. They are:
+
+     (*) Find or allocate a dentry for a given inode.
+
+		struct dentry *d_obtain_alias(struct inode *inode);
+
+	 This will return a dentry for the given inode.  If the inode already
+      	 has a dentry, one of those is returned.  If it doesn't, a new
+      	 anonymous (IS_ROOT() and DCACHE_DISCONNECTED) dentry is allocated and
+      	 attached.  In the case of a directory, care is taken that only one
+      	 dentry can ever be attached.
+
+     (*) Splice a disconnected dentry into the tree if one exists.
+
+		struct dentry *d_splice_alias(struct inode *inode,
+					      struct dentry *dentry);
+
+         This will make sure that there is a dentry with the same name and
+      	 parent as the given dentry, and which refers to the given inode.
+
+      	 If the inode is a directory and already has a dentry, then that dentry
+      	 is d_move()'d over the given dentry.  If the passed dentry gets
+      	 attached, care is taken that this is mutually exclusive to a
+      	 d_obtain_alias() operation.  If the passed dentry is used, NULL is
+      	 returned, else the used dentry is returned.  This corresponds to the
+      	 calling pattern of the lookup procedure.
+
+
+==============================
+MAKING A FILESYSTEM EXPORTABLE
+==============================
 
 For a filesystem to be exportable it must:
- 
-   1/ provide the filehandle fragment routines described below.
-   2/ make sure that d_splice_alias is used rather than d_add
-      when ->lookup finds an inode for a given parent and name.
-      Typically the ->lookup routine will end with a:
 
+ (1) Provide the filehandle export operations described below.
+
+ (2) Make sure that d_splice_alias() is used rather than d_add() when its
+     lookup() operation finds an inode for a given parent and name.  Typically
+     the lookup() routine will end with a:
+
+	{
+	...
 		return d_splice_alias(inode, dentry);
 	}
 
 
+==========================
+FILEHANDLE FRAGMENT FORMAT
+==========================
+
+A filehandle fragment consists of an array of 1 or more 32-bit words.  The
+contents and the layout of the data in the array are entirely at the whim of
+the filesystem that generated it.
+
+The filehandle fragment produced also has a 'type' associated with it.  This
+is a number between 0 and 254.  0 is a special reserved value (FILEID_ROOT)
+that encode_fh() may not produce.  The remaining values in that range can be
+chosen at will, though there are possible symbols in the fid_type enum that can
+be used if desired.  Beware, though, these may be decoded by protocol decoding
+programs, such as wireshark, so using a predefined type may confuse people if
+the corresponding format is not also adhered to (see the fid struct).
 
-  A file system implementation declares that instances of the filesystem
-are exportable by setting the s_export_op field in the struct
-super_block.  This field must point to a "struct export_operations"
-struct which has the following members:
-
- encode_fh  (optional)
-    Takes a dentry and creates a filehandle fragment which can later be used
-    to find or create a dentry for the same object.  The default
-    implementation creates a filehandle fragment that encodes a 32bit inode
-    and generation number for the inode encoded, and if necessary the
-    same information for the parent.
-
-  fh_to_dentry (mandatory)
-    Given a filehandle fragment, this should find the implied object and
-    create a dentry for it (possibly with d_alloc_anon).
-
-  fh_to_parent (optional but strongly recommended)
-    Given a filehandle fragment, this should find the parent of the
-    implied object and create a dentry for it (possibly with d_alloc_anon).
-    May fail if the filehandle fragment is too small.
-
-  get_parent (optional but strongly recommended)
-    When given a dentry for a directory, this should return  a dentry for
-    the parent.  Quite possibly the parent dentry will have been allocated
-    by d_alloc_anon.  The default get_parent function just returns an error
-    so any filehandle lookup that requires finding a parent will fail.
-    ->lookup("..") is *not* used as a default as it can leave ".." entries
-    in the dcache which are too messy to work with.
-
-  get_name (optional)
-    When given a parent dentry and a child dentry, this should find a name
-    in the directory identified by the parent dentry, which leads to the
-    object identified by the child dentry.  If no get_name function is
-    supplied, a default implementation is provided which uses vfs_readdir
-    to find potential names, and matches inode numbers to find the correct
-    match.
-
-
-A filehandle fragment consists of an array of 1 or more 4byte words,
-together with a one byte "type".
-The decode_fh routine should not depend on the stated size that is
-passed to it.  This size may be larger than the original filehandle
-generated by encode_fh, in which case it will have been padded with
-nuls.  Rather, the encode_fh routine should choose a "type" which
-indicates the decode_fh how much of the filehandle is valid, and how
+The fh_to_dentry() and fh_to_parent() routines should not depend on the stated
+size that is passed to them.  This size may be larger than the original
+filehandle generated by encode_fh(), in which case it will have been padded
+with NULs.  Rather, the encode_fh() routine should choose a "type" which
+indicates the fh_to_*() operations how much of the filehandle is valid, and how
 it should be interpreted.
+
+
+============================
+FILEHANDLE EXPORT OPERATIONS
+============================
+
+A filesystem implementation declares that instances of the filesystem are
+exportable by setting the s_export_op field in the super_block struct.  This
+field must point to an export_operations struct which has the following members
+filled in:
+
+ (*) enum fid_type (*encode_fh)(struct dentry *de, __u32 *fh, int *max_len,
+				int connectable);
+
+     This operation is used to generate a filehandle fragment that can later be
+     used to find or create a dentry for the same filesystem object.  It is
+     optional and there's a default encoder.
+
+     The fragment is written into the supplied buffer - fh - which can hold up
+     to *max_len lots of 4-byte words.  The fragment generated should represent
+     the given dentry, and if connectable is set, the parent of the given
+     dentry too.
+
+     If successful, this operation should return a value that represents to the
+     decoder operations the format of the opaque fragment produced and it
+     should set *max_len to indicate the amount of data it stored in the buffer
+     in units of 32-bit words.
+
+     The caller is required to save the type value for presentation to the
+     decoder operations.  The type value is arbitrary, but must be between 1
+     and 254.  There are some predefined types in the fid_type enum that can be
+     selected, but use of these is not mandatory.
+
+     Upon failure, a value of FILEID_ERROR should be returned.
+
+     The default encoder uses the i_ino and i_generation numbers from the
+     inode, both masked down to 32-bits.  It also adds both values from the
+     parent inode if connectable.
+
+ (*) struct dentry *(*fh_to_dentry)(struct super_block *sb, struct fid *fid,
+				    int fh_len, enum fid_type fh_type);
+
+     This operation is used to look up a file, given the filehandle fragment
+     that was generated by encode_fh().  The file, if it exists, will be one of
+     the set available through the specified superblock.  This operation is
+     mandatory.
+
+     The filehandle fragment is in the buffer specified by fid and is of up to
+     fh_len 32-bit words and is of type fh_type as indicated by encode_fh().
+     Note that there may be more data than expected in the buffer as the data
+     may have been padded out by the caller.  fh_type must be used to deal with
+     this.
+
+     If successful, this operation should return a pointer to a dcache entry
+     representing one of the aliases to the file.  This may be used by the
+     caller to query other aliases of the same file to find one it prefers.
+
+     Note that this operation is not required to produce an dentry that is
+     connected to the root of the superblock.  It is permitted to produce an
+     anonymous dentry, as might happen if encode_fh() was given connectable as
+     false.
+
+     On failure, a negative error code should be produced to indicate the
+     reason.  A NULL pointer must not be returned.
+
+ (*) struct dentry *(*fh_to_parent)(struct super_block *sb, struct fid *fid,
+				    int fh_len, enum fid_type fh_type);
+
+     This is very similar to fh_to_dentry(), the difference being that it
+     attempts to retrieve the parent of the file to which the filehandle
+     fragment corresponds, rather than the file itself.  This operation is
+     optional, but is strongly recommended.
+
+     This may fail if the filehandle does not contain information on the
+     original file's parentage.  It also need not produce a fully connected
+     dentry.
+
+ (*) struct dentry *(*get_parent)(struct dentry *child);
+
+     This operation should return a dentry to represent the parent directory of
+     the specified dentry (which should itself be a directory).  This operation
+     is optional, but strongly recommended.
+
+     An anonymous, unconnected dentry can be returned as the parent if so
+     desired.  This may be allocated with d_obtain_alias() or suchlike.
+
+     On success, the parent dentry should be returned.  On failure, a negative
+     error code should be returned.
+
+     The default operation just returns an error, thus making any filehandle
+     lookup that requires finding the parent fail.  The default operation does
+     not try to use 'lookup("..")' as that could leave ".." entries in the
+     dcache.
+
+ (*) int (*get_name)(struct dentry *parent, char *name, struct dentry *child);
+
+     This operation is provided to determine the name of the directory entry in
+     the parent directory that points to the child object.  This operation is
+     optional.
+
+     On success, the NUL-terminated name of the directory entry should be
+     written into the name buffer, and zero should be returned.  The caller
+     must guarantee that the buffer is at least NAME_MAX + 1 in size.
+
+     On failure, a negative error code should be returned.
+
+     The default operation is available that uses vfs_readdir() to find
+     potential names and match inode numbers to select the correct entry.
+
+This interface can be included in the code by:
+
+	#include <linux/exportfs.h>
+
+and making the user dependent on CONFIG_EXPORTFS.
diff --git a/fs/efs/efs.h b/fs/efs/efs.h
index d8305b5..db5e1c2 100644
--- a/fs/efs/efs.h
+++ b/fs/efs/efs.h
@@ -8,6 +8,7 @@
 #define _EFS_EFS_H_
 
 #include <linux/fs.h>
+#include <linux/exportfs.h>
 #include <asm/uaccess.h>
 
 #define EFS_VERSION "1.0a"
@@ -131,9 +132,9 @@ extern int efs_get_block(struct inode *, sector_t, struct buffer_head *, int);
 
 extern struct dentry *efs_lookup(struct inode *, struct dentry *, struct nameidata *);
 extern struct dentry *efs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type);
+		int fh_len, enum fid_type fh_type);
 extern struct dentry *efs_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type);
+		int fh_len, enum fid_type fh_type);
 extern struct dentry *efs_get_parent(struct dentry *);
 extern int efs_bmap(struct inode *, int);
 
diff --git a/fs/efs/namei.c b/fs/efs/namei.c
index c3fb5f9..6b9c118 100644
--- a/fs/efs/namei.c
+++ b/fs/efs/namei.c
@@ -97,14 +97,14 @@ static struct inode *efs_nfs_get_inode(struct super_block *sb, u64 ino,
 }
 
 struct dentry *efs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
 				    efs_nfs_get_inode);
 }
 
 struct dentry *efs_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
 				    efs_nfs_get_inode);
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index ec1fb91..c1f2399 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -35,12 +35,12 @@ static int exportfs_get_name(struct vfsmount *mnt, struct dentry *dir,
 }
 
 /*
- * Check if the dentry or any of it's aliases is acceptable.
+ * Check if the dentry or any of it's aliases are acceptable.
  */
 static struct dentry *
 find_acceptable_alias(struct dentry *result,
-		int (*acceptable)(void *context, struct dentry *dentry),
-		void *context)
+		      exportfs_acceptable_t acceptable,
+		      void *context)
 {
 	struct dentry *dentry, *toput = NULL;
 
@@ -303,25 +303,25 @@ out:
 
 /**
  * export_encode_fh - default export_operations->encode_fh function
- * @dentry:  the dentry to encode
- * @fh:      where to store the file handle fragment
- * @max_len: maximum length to store there
- * @connectable: whether to store parent information
+ * @dentry:  The dentry to encode
+ * @fid:     Where to store the file handle fragment
+ * @max_len: Maximum length to store there in 32-bit words
+ * @connectable: Whether to store parent information
  *
- * This default encode_fh function assumes that the 32 inode number
- * is suitable for locating an inode, and that the generation number
- * can be used to check that it is still valid.  It places them in the
- * filehandle fragment where export_decode_fh expects to find them.
+ * This default encode_fh function assumes that the 32 inode number is suitable
+ * for locating an inode, and that the generation number can be used to check
+ * that it is still valid.  It places them in the filehandle fragment where
+ * export_decode_fh expects to find them.
  */
-static int export_encode_fh(struct dentry *dentry, struct fid *fid,
-		int *max_len, int connectable)
+static enum fid_type export_encode_fh(struct dentry *dentry, struct fid *fid,
+				      int *max_len, int connectable)
 {
 	struct inode * inode = dentry->d_inode;
 	int len = *max_len;
 	int type = FILEID_INO32_GEN;
-	
+
 	if (len < 2 || (connectable && len < 4))
-		return 255;
+		return FILEID_ERROR;
 
 	len = 2;
 	fid->i32.ino = inode->i_ino;
@@ -341,24 +341,71 @@ static int export_encode_fh(struct dentry *dentry, struct fid *fid,
 	return type;
 }
 
-int exportfs_encode_fh(struct dentry *dentry, struct fid *fid, int *max_len,
-		int connectable)
+/**
+ * exportfs_encode_fh - Encode a dentry to a partial persistent file handle
+ * @dentry:  The dentry to encode
+ * @fid:     Where to store the file handle fragment
+ * @max_len: Maximum length to store there in 32-bit words
+ * @connectable: Whether to store parent information
+ *
+ * This function is used to get a partial persistent file handle to represent a
+ * file object as referred to by the given dentry.  The caller may also request
+ * that parentage information be noted in the file handle produced.
+ *
+ * The fid pointer should point to a buffer of *max_len size, where the maximum
+ * length is given in 32-bit words, _not_ bytes.  The contents of the returned
+ * file handle may or may not reflect the layout of the fid structure.  That is
+ * totally up to the filesystem being queried.
+ *
+ * On return, the file handle type will be returned, or FILEID_ERROR if the
+ * filesystem was unable to represent the file.  The caller is responsible for
+ * saving the type so that it can be passed to exportfs_decode_fh().
+ *
+ * If successful, *max_len will have been updated to specify the amount of
+ * buffer actually used.
+ */
+enum fid_type exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
+				 int *max_len, int connectable)
 {
 	const struct export_operations *nop = dentry->d_sb->s_export_op;
-	int error;
 
 	if (nop->encode_fh)
-		error = nop->encode_fh(dentry, fid->raw, max_len, connectable);
+		return nop->encode_fh(dentry, fid->raw, max_len, connectable);
 	else
-		error = export_encode_fh(dentry, fid, max_len, connectable);
-
-	return error;
+		return export_encode_fh(dentry, fid, max_len, connectable);
 }
 EXPORT_SYMBOL_GPL(exportfs_encode_fh);
 
+/**
+ * exportfs_decode_fh - Decode a partial persistent file handle to a dentry
+ * @mnt: The mountpoint with which the dentry should be associated
+ * @fid: The partial file handle
+ * @fh_len: The length of the partial file handle
+ * @fileid_type: The type of partial file handle
+ * @acceptable: A filter routine to pick amongst the aliases for an inode
+ * @context: Context information for the acceptability filter
+ *
+ * This function function is used to turn a partial persistent file handle back
+ * into the dentry it represents.  The caller must indicate the set of dentries
+ * from which the target is to be selected by the VFS mountpoint supplied.
+ *
+ * The partial file handle is in the buffer pointed to by the fid pointer, and
+ * is fh_len 32-bit words in length.  The fileid_type as returned by the encode
+ * routine must also be presented by the caller.
+ *
+ * If a suitable inode is found, its list of aliases will be passed to the
+ * acceptability function to select a suitable one.  The context information
+ * supplied will be passed to the acceptability filter to aid in selection.
+ *
+ * If successful, a pointer to a suitable dentry will be returned.  Note that
+ * dentry returned may not be connected to any of the superblock's roots.
+ *
+ * If unsuccessful, a negative error code will be returned.
+ */
 struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
-		int fh_len, int fileid_type,
-		int (*acceptable)(void *, struct dentry *), void *context)
+				  int fh_len, enum fid_type fileid_type,
+				  exportfs_acceptable_t acceptable,
+				  void *context)
 {
 	const struct export_operations *nop = mnt->mnt_sb->s_export_op;
 	struct dentry *result, *alias;
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 647cd88..a3ef820 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -340,14 +340,14 @@ static struct inode *ext2_nfs_get_inode(struct super_block *sb,
 }
 
 static struct dentry *ext2_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
 				    ext2_nfs_get_inode);
 }
 
 static struct dentry *ext2_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
 				    ext2_nfs_get_inode);
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index f6c94f2..b549bbd 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -669,14 +669,14 @@ static struct inode *ext3_nfs_get_inode(struct super_block *sb,
 }
 
 static struct dentry *ext3_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
 				    ext3_nfs_get_inode);
 }
 
 static struct dentry *ext3_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
 				    ext3_nfs_get_inode);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e4a241c..30e142b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -759,14 +759,14 @@ static struct inode *ext4_nfs_get_inode(struct super_block *sb,
 }
 
 static struct dentry *ext4_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
 				    ext4_nfs_get_inode);
 }
 
 static struct dentry *ext4_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
 				    ext4_nfs_get_inode);
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index d937aaf..742ee35 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -649,14 +649,16 @@ static const struct super_operations fat_sops = {
  * of i_logstart is used to store the directory entry offset.
  */
 
+#define FAT_FILEID	((enum fid_type) 3)
+
 static struct dentry *fat_fh_to_dentry(struct super_block *sb,
-		struct fid *fid, int fh_len, int fh_type)
+		struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct inode *inode = NULL;
 	struct dentry *result;
 	u32 *fh = fid->raw;
 
-	if (fh_len < 5 || fh_type != 3)
+	if (fh_len < 5 || fh_type != FAT_FILEID)
 		return NULL;
 
 	inode = ilookup(sb, fh[0]);
@@ -704,7 +706,7 @@ static struct dentry *fat_fh_to_dentry(struct super_block *sb,
 	return result;
 }
 
-static int
+static enum fid_type
 fat_encode_fh(struct dentry *de, __u32 *fh, int *lenp, int connectable)
 {
 	int len = *lenp;
@@ -712,7 +714,7 @@ fat_encode_fh(struct dentry *de, __u32 *fh, int *lenp, int connectable)
 	u32 ipos_h, ipos_m, ipos_l;
 
 	if (len < 5)
-		return 255; /* no room */
+		return FILEID_ERROR; /* no room */
 
 	ipos_h = MSDOS_I(inode)->i_pos >> 8;
 	ipos_m = (MSDOS_I(inode)->i_pos & 0xf0) << 24;
@@ -725,7 +727,7 @@ fat_encode_fh(struct dentry *de, __u32 *fh, int *lenp, int connectable)
 	spin_lock(&de->d_lock);
 	fh[4] = ipos_l | MSDOS_I(de->d_parent->d_inode)->i_logstart;
 	spin_unlock(&de->d_lock);
-	return 3;
+	return FAT_FILEID;
 }
 
 static struct dentry *fat_get_parent(struct dentry *child)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2e99f34..b0f6528 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -610,8 +610,11 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
 	return ERR_PTR(err);
 }
 
-static int fuse_encode_fh(struct dentry *dentry, u32 *fh, int *max_len,
-			   int connectable)
+#define FUSE_FILEID		((enum fid_type) 0x81)
+#define FUSE_FILEID_PARENT	((enum fid_type) 0x82)
+
+static enum fid_type fuse_encode_fh(struct dentry *dentry, u32 *fh,
+				    int *max_len, int connectable)
 {
 	struct inode *inode = dentry->d_inode;
 	bool encode_parent = connectable && !S_ISDIR(inode->i_mode);
@@ -620,7 +623,7 @@ static int fuse_encode_fh(struct dentry *dentry, u32 *fh, int *max_len,
 	u32 generation;
 
 	if (*max_len < len)
-		return  255;
+		return FILEID_ERROR;
 
 	nodeid = get_fuse_inode(inode)->nodeid;
 	generation = inode->i_generation;
@@ -644,16 +647,17 @@ static int fuse_encode_fh(struct dentry *dentry, u32 *fh, int *max_len,
 	}
 
 	*max_len = len;
-	return encode_parent ? 0x82 : 0x81;
+	return encode_parent ? FUSE_FILEID_PARENT : FUSE_FILEID;
 }
 
 static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
-		struct fid *fid, int fh_len, int fh_type)
+		struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct fuse_inode_handle handle;
 
-	if ((fh_type != 0x81 && fh_type != 0x82) || fh_len < 3)
-		return NULL;
+	if ((fh_type != FUSE_FILEID && fh_type != FUSE_FILEID_PARENT) ||
+	    fh_len < 3)
+		return ERR_PTR(-ESTALE);
 
 	handle.nodeid = (u64) fid->raw[0] << 32;
 	handle.nodeid |= (u64) fid->raw[1];
@@ -662,12 +666,12 @@ static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
 }
 
 static struct dentry *fuse_fh_to_parent(struct super_block *sb,
-		struct fid *fid, int fh_len, int fh_type)
+		struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct fuse_inode_handle parent;
 
 	if (fh_type != 0x82 || fh_len < 6)
-		return NULL;
+		return ERR_PTR(-ESTALE);
 
 	parent.nodeid = (u64) fid->raw[3] << 32;
 	parent.nodeid |= (u64) fid->raw[4];
diff --git a/fs/gfs2/ops_export.c b/fs/gfs2/ops_export.c
index bbb8c36..8d0d6cf 100644
--- a/fs/gfs2/ops_export.c
+++ b/fs/gfs2/ops_export.c
@@ -31,8 +31,8 @@
 #define GFS2_LARGE_FH_SIZE 8
 #define GFS2_OLD_FH_SIZE 10
 
-static int gfs2_encode_fh(struct dentry *dentry, __u32 *p, int *len,
-			  int connectable)
+static enum fid_type gfs2_encode_fh(struct dentry *dentry, __u32 *p,
+				       int *len, int connectable)
 {
 	__be32 *fh = (__force __be32 *)p;
 	struct inode *inode = dentry->d_inode;
@@ -41,7 +41,7 @@ static int gfs2_encode_fh(struct dentry *dentry, __u32 *p, int *len,
 
 	if (*len < GFS2_SMALL_FH_SIZE ||
 	    (connectable && *len < GFS2_LARGE_FH_SIZE))
-		return 255;
+		return FILEID_ERROR;
 
 	fh[0] = cpu_to_be32(ip->i_no_formal_ino >> 32);
 	fh[1] = cpu_to_be32(ip->i_no_formal_ino & 0xFFFFFFFF);
@@ -50,7 +50,7 @@ static int gfs2_encode_fh(struct dentry *dentry, __u32 *p, int *len,
 	*len = GFS2_SMALL_FH_SIZE;
 
 	if (!connectable || inode == sb->s_root->d_inode)
-		return *len;
+		return GFS2_SMALL_FH_SIZE;
 
 	spin_lock(&dentry->d_lock);
 	inode = dentry->d_parent->d_inode;
@@ -66,7 +66,7 @@ static int gfs2_encode_fh(struct dentry *dentry, __u32 *p, int *len,
 
 	iput(inode);
 
-	return *len;
+	return GFS2_LARGE_FH_SIZE;
 }
 
 struct get_name_filldir {
@@ -239,7 +239,7 @@ fail:
 }
 
 static struct dentry *gfs2_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	struct gfs2_inum_host this;
 	__be32 *fh = (__force __be32 *)fid->raw;
@@ -254,12 +254,12 @@ static struct dentry *gfs2_fh_to_dentry(struct super_block *sb, struct fid *fid,
 		this.no_addr |= be32_to_cpu(fh[3]);
 		return gfs2_get_dentry(sb, &this);
 	default:
-		return NULL;
+		return ERR_PTR(-ESTALE);
 	}
 }
 
 static struct dentry *gfs2_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	struct gfs2_inum_host parent;
 	__be32 *fh = (__force __be32 *)fid->raw;
@@ -273,7 +273,7 @@ static struct dentry *gfs2_fh_to_parent(struct super_block *sb, struct fid *fid,
 		parent.no_addr |= be32_to_cpu(fh[7]);
 		return gfs2_get_dentry(sb, &parent);
 	default:
-		return NULL;
+		return ERR_PTR(-ESTALE);
 	}
 }
 
diff --git a/fs/isofs/export.c b/fs/isofs/export.c
index e81a305..b66c083 100644
--- a/fs/isofs/export.c
+++ b/fs/isofs/export.c
@@ -106,7 +106,10 @@ static struct dentry *isofs_export_get_parent(struct dentry *child)
 	return rv;
 }
 
-static int
+#define ISOFS_FILEID		((enum fid_type) 1)
+#define ISOFS_FILEID_PARENT	((enum fid_type) 2)
+
+static enum fid_type
 isofs_export_encode_fh(struct dentry *dentry,
 		       __u32 *fh32,
 		       int *max_len,
@@ -115,7 +118,7 @@ isofs_export_encode_fh(struct dentry *dentry,
 	struct inode * inode = dentry->d_inode;
 	struct iso_inode_info * ei = ISOFS_I(inode);
 	int len = *max_len;
-	int type = 1;
+	enum fid_type type = ISOFS_FILEID;
 	__u16 *fh16 = (__u16*)fh32;
 
 	/*
@@ -126,7 +129,7 @@ isofs_export_encode_fh(struct dentry *dentry,
 	 */
 
 	if (len < 3 || (connectable && len < 5))
-		return 255;
+		return FILEID_ERROR;
 
 	len = 3;
 	fh32[0] = ei->i_iget5_block;
@@ -143,7 +146,7 @@ isofs_export_encode_fh(struct dentry *dentry,
 		fh32[4] = parent->i_generation;
 		spin_unlock(&dentry->d_lock);
 		len = 5;
-		type = 2;
+		type = ISOFS_FILEID_PARENT;
 	}
 	*max_len = len;
 	return type;
@@ -159,23 +162,23 @@ struct isofs_fid {
 };
 
 static struct dentry *isofs_fh_to_dentry(struct super_block *sb,
-	struct fid *fid, int fh_len, int fh_type)
+	struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct isofs_fid *ifid = (struct isofs_fid *)fid;
 
-	if (fh_len < 3 || fh_type > 2)
-		return NULL;
+	if (fh_len < 3 || fh_type > ISOFS_FILEID_PARENT)
+		return ERR_PTR(-ESTALE);
 
 	return isofs_export_iget(sb, ifid->block, ifid->offset,
 			ifid->generation);
 }
 
 static struct dentry *isofs_fh_to_parent(struct super_block *sb,
-		struct fid *fid, int fh_len, int fh_type)
+		struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct isofs_fid *ifid = (struct isofs_fid *)fid;
 
-	if (fh_type != 2)
+	if (fh_type != ISOFS_FILEID_PARENT)
 		return NULL;
 
 	return isofs_export_iget(sb,
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 4c4e18c..bd4998e 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -73,14 +73,14 @@ static struct inode *jffs2_nfs_get_inode(struct super_block *sb, uint64_t ino,
 }
 
 static struct dentry *jffs2_fh_to_dentry(struct super_block *sb, struct fid *fid,
-					 int fh_len, int fh_type)
+					 int fh_len, enum fid_type fh_type)
 {
         return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
                                     jffs2_nfs_get_inode);
 }
 
 static struct dentry *jffs2_fh_to_parent(struct super_block *sb, struct fid *fid,
-					 int fh_len, int fh_type)
+					 int fh_len, enum fid_type fh_type)
 {
         return generic_fh_to_parent(sb, fid, fh_len, fh_type,
                                     jffs2_nfs_get_inode);
diff --git a/fs/jfs/jfs_inode.h b/fs/jfs/jfs_inode.h
index adb2faf..aa7d573 100644
--- a/fs/jfs/jfs_inode.h
+++ b/fs/jfs/jfs_inode.h
@@ -19,6 +19,7 @@
 #define _H_JFS_INODE
 
 struct fid;
+enum fid_type;
 
 extern struct inode *ialloc(struct inode *, umode_t);
 extern int jfs_fsync(struct file *, struct dentry *, int);
@@ -35,9 +36,9 @@ extern void jfs_free_zero_link(struct inode *);
 extern struct dentry *jfs_get_parent(struct dentry *dentry);
 extern void jfs_get_inode_flags(struct jfs_inode_info *);
 extern struct dentry *jfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-	int fh_len, int fh_type);
+	int fh_len, enum fid_type fh_type);
 extern struct dentry *jfs_fh_to_parent(struct super_block *sb, struct fid *fid,
-	int fh_len, int fh_type);
+	int fh_len, enum fid_type fh_type);
 extern void jfs_set_inode_flags(struct inode *);
 extern int jfs_get_block(struct inode *, sector_t, struct buffer_head *, int);
 
diff --git a/fs/jfs/namei.c b/fs/jfs/namei.c
index cc3cedf..b824836 100644
--- a/fs/jfs/namei.c
+++ b/fs/jfs/namei.c
@@ -1496,14 +1496,14 @@ static struct inode *jfs_nfs_get_inode(struct super_block *sb,
 }
 
 struct dentry *jfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
 				    jfs_nfs_get_inode);
 }
 
 struct dentry *jfs_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
 				    jfs_nfs_get_inode);
diff --git a/fs/libfs.c b/fs/libfs.c
index e960a83..2a9eb1d 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -745,18 +745,19 @@ out:
  * inode for the object specified in the file handle.
  */
 struct dentry *generic_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type, struct inode *(*get_inode)
-			(struct super_block *sb, u64 ino, u32 gen))
+				    int fh_len, enum fid_type fh_type,
+				    exportfs_get_inode_t get_inode)
 {
 	struct inode *inode = NULL;
 
 	if (fh_len < 2)
-		return NULL;
+		return ERR_PTR(-ESTALE);
 
 	switch (fh_type) {
 	case FILEID_INO32_GEN:
 	case FILEID_INO32_GEN_PARENT:
 		inode = get_inode(sb, fid->i32.ino, fid->i32.gen);
+	default:
 		break;
 	}
 
@@ -778,18 +779,19 @@ EXPORT_SYMBOL_GPL(generic_fh_to_dentry);
  * is specified in the file handle, or NULL otherwise.
  */
 struct dentry *generic_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type, struct inode *(*get_inode)
-			(struct super_block *sb, u64 ino, u32 gen))
+				    int fh_len, enum fid_type fh_type,
+				    exportfs_get_inode_t get_inode)
 {
 	struct inode *inode = NULL;
 
 	if (fh_len <= 2)
-		return NULL;
+		return ERR_PTR(-ESTALE);
 
 	switch (fh_type) {
 	case FILEID_INO32_GEN_PARENT:
 		inode = get_inode(sb, fid->i32.parent_ino,
 				  (fh_len > 3 ? fid->i32.parent_gen : 0));
+	default:
 		break;
 	}
 
diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index 2ca0015..b3826c0 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -364,14 +364,14 @@ static struct inode *ntfs_nfs_get_inode(struct super_block *sb,
 }
 
 static struct dentry *ntfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
 				    ntfs_nfs_get_inode);
 }
 
 static struct dentry *ntfs_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
 				    ntfs_nfs_get_inode);
diff --git a/fs/ocfs2/export.c b/fs/ocfs2/export.c
index 2f27b33..d064bfa 100644
--- a/fs/ocfs2/export.c
+++ b/fs/ocfs2/export.c
@@ -116,12 +116,15 @@ bail:
 	return parent;
 }
 
-static int ocfs2_encode_fh(struct dentry *dentry, u32 *fh_in, int *max_len,
-			   int connectable)
+#define OCFS2_FILEID		((enum fid_type) 1)
+#define OCFS2_FILEID_PARENT	((enum fid_type) 2)
+
+static enum fid_type ocfs2_encode_fh(struct dentry *dentry, u32 *fh_in,
+					int *max_len, int connectable)
 {
 	struct inode *inode = dentry->d_inode;
 	int len = *max_len;
-	int type = 1;
+	int type = OCFS2_FILEID;
 	u64 blkno;
 	u32 generation;
 	__le32 *fh = (__force __le32 *) fh_in;
@@ -132,7 +135,7 @@ static int ocfs2_encode_fh(struct dentry *dentry, u32 *fh_in, int *max_len,
 
 	if (len < 3 || (connectable && len < 6)) {
 		mlog(ML_ERROR, "fh buffer is too small for encoding\n");
-		type = 255;
+		type = FILEID_ERROR;
 		goto bail;
 	}
 
@@ -163,7 +166,7 @@ static int ocfs2_encode_fh(struct dentry *dentry, u32 *fh_in, int *max_len,
 		spin_unlock(&dentry->d_lock);
 
 		len = 6;
-		type = 2;
+		type = OCFS2_FILEID_PARENT;
 
 		mlog(0, "Encoding parent: blkno: %llu, generation: %u\n",
 		     (unsigned long long)blkno, generation);
@@ -177,12 +180,12 @@ bail:
 }
 
 static struct dentry *ocfs2_fh_to_dentry(struct super_block *sb,
-		struct fid *fid, int fh_len, int fh_type)
+		struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct ocfs2_inode_handle handle;
 
-	if (fh_len < 3 || fh_type > 2)
-		return NULL;
+	if (fh_len < 3 || fh_type > OCFS2_FILEID_PARENT)
+		return ERR_PTR(-ESTALE);
 
 	handle.ih_blkno = (u64)le32_to_cpu(fid->raw[0]) << 32;
 	handle.ih_blkno |= (u64)le32_to_cpu(fid->raw[1]);
@@ -191,12 +194,12 @@ static struct dentry *ocfs2_fh_to_dentry(struct super_block *sb,
 }
 
 static struct dentry *ocfs2_fh_to_parent(struct super_block *sb,
-		struct fid *fid, int fh_len, int fh_type)
+		struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct ocfs2_inode_handle parent;
 
-	if (fh_type != 2 || fh_len < 6)
-		return NULL;
+	if (fh_type != OCFS2_FILEID_PARENT || fh_len < 6)
+		return ERR_PTR(-ESTALE);
 
 	parent.ih_blkno = (u64)le32_to_cpu(fid->raw[3]) << 32;
 	parent.ih_blkno |= (u64)le32_to_cpu(fid->raw[4]);
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index 6c4c2c6..3c548eb 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -1539,7 +1539,7 @@ static struct dentry *reiserfs_get_dentry(struct super_block *sb,
 }
 
 struct dentry *reiserfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	/* fhtype happens to reflect the number of u32s encoded.
 	 * due to a bug in earlier code, fhtype might indicate there
@@ -1566,7 +1566,7 @@ struct dentry *reiserfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
 }
 
 struct dentry *reiserfs_fh_to_parent(struct super_block *sb, struct fid *fid,
-		int fh_len, int fh_type)
+		int fh_len, enum fid_type fh_type)
 {
 	if (fh_type < 4)
 		return NULL;
@@ -1577,14 +1577,14 @@ struct dentry *reiserfs_fh_to_parent(struct super_block *sb, struct fid *fid,
 		(fh_type == 6) ? fid->raw[5] : 0);
 }
 
-int reiserfs_encode_fh(struct dentry *dentry, __u32 * data, int *lenp,
-		       int need_parent)
+enum fid_type reiserfs_encode_fh(struct dentry *dentry, __u32 * data,
+				    int *lenp, int need_parent)
 {
 	struct inode *inode = dentry->d_inode;
 	int maxlen = *lenp;
 
 	if (maxlen < 3)
-		return 255;
+		return FILEID_ERROR;
 
 	data[0] = inode->i_ino;
 	data[1] = le32_to_cpu(INODE_PKEY(inode)->k_dir_id);
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index f84bfaa..afaf45e 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -1292,38 +1292,40 @@ static struct dentry *udf_nfs_get_inode(struct super_block *sb, u32 block,
 }
 
 static struct dentry *udf_fh_to_dentry(struct super_block *sb,
-				       struct fid *fid, int fh_len, int fh_type)
+				       struct fid *fid, int fh_len,
+				       enum fid_type fh_type)
 {
 	if ((fh_len != 3 && fh_len != 5) ||
 	    (fh_type != FILEID_UDF_WITH_PARENT &&
 	     fh_type != FILEID_UDF_WITHOUT_PARENT))
-		return NULL;
+		return ERR_PTR(-ESTALE);
 
 	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)
+				       struct fid *fid, int fh_len,
+				       enum fid_type fh_type)
 {
 	if (fh_len != 5 || fh_type != FILEID_UDF_WITH_PARENT)
-		return NULL;
+		return ERR_PTR(-ESTALE);
 
 	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)
+static enum fid_type 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;
+	enum fid_type type = FILEID_UDF_WITHOUT_PARENT;
 
 	if (len < 3 || (connectable && len < 5))
-		return 255;
+		return FILEID_ERROR;
 
 	*lenp = 3;
 	fid->udf.block = location.logicalBlockNum;
diff --git a/fs/xfs/linux-2.6/xfs_export.c b/fs/xfs/linux-2.6/xfs_export.c
index 7f7abec..ce053aa 100644
--- a/fs/xfs/linux-2.6/xfs_export.c
+++ b/fs/xfs/linux-2.6/xfs_export.c
@@ -51,7 +51,7 @@ static int xfs_fileid_length(int fileid_type)
 	return 255; /* invalid */
 }
 
-STATIC int
+STATIC enum fid_type
 xfs_fs_encode_fh(
 	struct dentry		*dentry,
 	__u32			*fh,
@@ -61,18 +61,18 @@ xfs_fs_encode_fh(
 	struct fid		*fid = (struct fid *)fh;
 	struct xfs_fid64	*fid64 = (struct xfs_fid64 *)fh;
 	struct inode		*inode = dentry->d_inode;
-	int			fileid_type;
+	enum fid_type		fid_type;
 	int			len;
 
 	/* Directories don't need their parent encoded, they have ".." */
 	if (S_ISDIR(inode->i_mode) || !connectable)
-		fileid_type = FILEID_INO32_GEN;
+		fid_type = FILEID_INO32_GEN;
 	else
-		fileid_type = FILEID_INO32_GEN_PARENT;
+		fid_type = FILEID_INO32_GEN_PARENT;
 
 	/* filesystem may contain 64bit inode numbers */
 	if (!(XFS_M(inode->i_sb)->m_flags & XFS_MOUNT_SMALL_INUMS))
-		fileid_type |= XFS_FILEID_TYPE_64FLAG;
+		fid_type |= XFS_FILEID_TYPE_64FLAG;
 
 	/*
 	 * Only encode if there is enough space given.  In practice
@@ -80,12 +80,12 @@ xfs_fs_encode_fh(
 	 * over NFSv2 with the subtree_check export option; the other
 	 * seven combinations work.  The real answer is "don't use v2".
 	 */
-	len = xfs_fileid_length(fileid_type);
+	len = xfs_fileid_length(fid_type);
 	if (*max_len < len)
-		return 255;
+		return FILEID_ERROR;
 	*max_len = len;
 
-	switch (fileid_type) {
+	switch (fid_type) {
 	case FILEID_INO32_GEN_PARENT:
 		spin_lock(&dentry->d_lock);
 		fid->i32.parent_ino = dentry->d_parent->d_inode->i_ino;
@@ -106,9 +106,11 @@ xfs_fs_encode_fh(
 		fid64->ino = inode->i_ino;
 		fid64->gen = inode->i_generation;
 		break;
+	default:
+		break;
 	}
 
-	return fileid_type;
+	return fid_type;
 }
 
 STATIC struct inode *
@@ -144,15 +146,15 @@ xfs_nfs_get_inode(
 
 STATIC struct dentry *
 xfs_fs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-		 int fh_len, int fileid_type)
+		 int fh_len, enum fid_type fid_type)
 {
 	struct xfs_fid64	*fid64 = (struct xfs_fid64 *)fid;
 	struct inode		*inode = NULL;
 
-	if (fh_len < xfs_fileid_length(fileid_type))
-		return NULL;
+	if (fh_len < xfs_fileid_length(fid_type))
+		return ERR_PTR(-ESTALE);
 
-	switch (fileid_type) {
+	switch (fid_type) {
 	case FILEID_INO32_GEN_PARENT:
 	case FILEID_INO32_GEN:
 		inode = xfs_nfs_get_inode(sb, fid->i32.ino, fid->i32.gen);
@@ -161,6 +163,8 @@ xfs_fs_fh_to_dentry(struct super_block *sb, struct fid *fid,
 	case FILEID_INO32_GEN | XFS_FILEID_TYPE_64FLAG:
 		inode = xfs_nfs_get_inode(sb, fid64->ino, fid64->gen);
 		break;
+	default:
+		break;
 	}
 
 	return d_obtain_alias(inode);
@@ -168,12 +172,12 @@ xfs_fs_fh_to_dentry(struct super_block *sb, struct fid *fid,
 
 STATIC struct dentry *
 xfs_fs_fh_to_parent(struct super_block *sb, struct fid *fid,
-		 int fh_len, int fileid_type)
+		 int fh_len, enum fid_type fid_type)
 {
 	struct xfs_fid64	*fid64 = (struct xfs_fid64 *)fid;
 	struct inode		*inode = NULL;
 
-	switch (fileid_type) {
+	switch (fid_type) {
 	case FILEID_INO32_GEN_PARENT:
 		inode = xfs_nfs_get_inode(sb, fid->i32.parent_ino,
 					      fid->i32.parent_gen);
@@ -182,6 +186,8 @@ xfs_fs_fh_to_parent(struct super_block *sb, struct fid *fid,
 		inode = xfs_nfs_get_inode(sb, fid64->parent_ino,
 					      fid64->parent_gen);
 		break;
+	default:
+		break;
 	}
 
 	return d_obtain_alias(inode);
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 27e772c..d6995e8 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -1,3 +1,9 @@
+/* Persistent file handle encoding and decoding interface
+ *
+ * See Documentation/filesystems/Exporting for details on how to use this
+ * interface correctly.
+ */
+
 #ifndef LINUX_EXPORTFS_H
 #define LINUX_EXPORTFS_H 1
 
@@ -9,12 +15,15 @@ struct super_block;
 struct vfsmount;
 
 /*
- * The fileid_type identifies how the file within the filesystem is encoded.
- * In theory this is freely set and parsed by the filesystem, but we try to
- * stick to conventions so we can share some generic code and don't confuse
- * sniffers like ethereal/wireshark.
+ * The fid_type identifies how the parameters specifying a file within the
+ * filesystem are encoded.  In theory this is freely set and parsed by the
+ * filesystem, but we try to stick to conventions so we can share some generic
+ * code and don't confuse sniffers like ethereal/wireshark.
  *
- * The filesystem must not use the value '0' or '0xff'.
+ * The filesystem may use arbitrary values rather than picking the constants
+ * from this set, with the restriction that the values chosen must be between 1
+ * and 254.  0 and 255 are special purpose, and the value must fit within an
+ * unsigned byte.
  */
 enum fid_type {
 	/*
@@ -67,6 +76,10 @@ enum fid_type {
 	 * 32 bit parent block number, 32 bit parent generation number
 	 */
 	FILEID_UDF_WITH_PARENT = 0x52,
+
+	/* This is returned if the encode routine was unable to represent the
+	 * file */
+	FILEID_ERROR = 0xff
 };
 
 struct fid {
@@ -101,73 +114,97 @@ struct fid {
  * this interface correctly.
  *
  * encode_fh:
- *    @encode_fh should store in the file handle fragment @fh (using at most
- *    @max_len bytes) information that can be used by @decode_fh to recover the
- *    file refered to by the &struct dentry @de.  If the @connectable flag is
- *    set, the encode_fh() should store sufficient information so that a good
- *    attempt can be made to find not only the file but also it's place in the
- *    filesystem.   This typically means storing a reference to de->d_parent in
- *    the filehandle fragment.  encode_fh() should return the number of bytes
- *    stored or a negative error code such as %-ENOSPC
+ *    The @encode_fh operation should store into the file handle fragment
+ *    buffer @fh at most *@max_len 32-bit words of information that can be used
+ *    by fh_to_dentry() or fh_to_parent() to recover the file referred to by
+ *    the &struct dentry @de.
+ *
+ *    If the @connectable flag is set, the encode_fh() should store sufficient
+ *    information so that a good attempt can be made to find not only the file
+ *    but also it's place in the filesystem.  This typically means storing a
+ *    reference to de->d_parent in the filehandle fragment.
+ *
+ *    On success, encode_fh() should return the type of the file handle, which
+ *    the caller must retain in some manner so that it can be passed to
+ *    decode_fh().  See the comment on enum fid_type as to the permitted types
+ *    that may be returned.  Furthermore, *max_len should be updated upon
+ *    return to indicate the amount of buffer space actually filled.
+ *
+ *    On failure FILEID_ERROR should be returned.
  *
  * fh_to_dentry:
- *    @fh_to_dentry is given a &struct super_block (@sb) and a file handle
- *    fragment (@fh, @fh_len). It should return a &struct dentry which refers
- *    to the same file that the file handle fragment refers to.  If it cannot,
- *    it should return a %NULL pointer if the file was found but no acceptable
- *    &dentries were available, or an %ERR_PTR error code indicating why it
- *    couldn't be found (e.g. %ENOENT or %ENOMEM).  Any suitable dentry can be
- *    returned including, if necessary, a new dentry created with d_alloc_root.
- *    The caller can then find any other extant dentries by following the
- *    d_alias links.
+ *    The @fh_to_dentry operation is given a &struct super_block (@sb) and a
+ *    file handle fragment and type (@fh, @fh_len, @fh_type).  It should look
+ *    up a file by the data in the partial file handle.
+ *
+ *    On success, a pointer should be returned to a dentry that corresponds to
+ *    the file that the file handle fragment was generated from by encode_fh().
+ *    The dentry should come from the set of dentries available to the
+ *    specified superblock, whether they're in memory or in storage.
+ *
+ *    Any suitable dentry can be returned including, if necessary, a new dentry
+ *    created with d_alloc_root.  The caller can then find any other extant
+ *    dentries by following the d_alias links.
+ *
+ *    On failure, a negative error code should be returned indicating the
+ *    reason.  Note that a NULL pointer is not a valid return.
  *
  * fh_to_parent:
  *    Same as @fh_to_dentry, except that it returns a pointer to the parent
- *    dentry if it was encoded into the filehandle fragment by @encode_fh.
+ *    dentry if it was encoded into the filehandle fragment by encode_fh().
  *
  * get_name:
  *    @get_name should find a name for the given @child in the given @parent
  *    directory.  The name should be stored in the @name (with the
- *    understanding that it is already pointing to a a %NAME_MAX+1 sized
- *    buffer.   get_name() should return %0 on success, a negative error code
- *    or error.  @get_name will be called without @parent->i_mutex held.
+ *    understanding that it is already pointing to a %NAME_MAX+1 sized buffer.
+ *
+ *    get_name() should return zero on success and a negative error code on
+ *    failure.
+ *
+ *    get_name() will be called without @parent->i_mutex held.
  *
  * get_parent:
  *    @get_parent should find the parent directory for the given @child which
  *    is also a directory.  In the event that it cannot be found, or storage
- *    space cannot be allocated, a %ERR_PTR should be returned.
+ *    space cannot be allocated, a suitable negative error code should be
+ *    returned.
  *
  * Locking rules:
  *    get_parent is called with child->d_inode->i_mutex down
  *    get_name is not (which is possibly inconsistent)
  */
-
 struct export_operations {
-	int (*encode_fh)(struct dentry *de, __u32 *fh, int *max_len,
-			int connectable);
+	enum fid_type (*encode_fh)(struct dentry *de, __u32 *fh, int *max_len,
+				   int connectable);
 	struct dentry * (*fh_to_dentry)(struct super_block *sb, struct fid *fid,
-			int fh_len, int fh_type);
+			int fh_len, enum fid_type fh_type);
 	struct dentry * (*fh_to_parent)(struct super_block *sb, struct fid *fid,
-			int fh_len, int fh_type);
+			int fh_len, enum fid_type fh_type);
 	int (*get_name)(struct dentry *parent, char *name,
 			struct dentry *child);
 	struct dentry * (*get_parent)(struct dentry *child);
 };
 
-extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
-	int *max_len, int connectable);
+typedef int (*exportfs_acceptable_t)(void *context, struct dentry *dentry);
+
+extern enum fid_type exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
+					int *max_len, int connectable);
 extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
-	int fh_len, int fileid_type, int (*acceptable)(void *, struct dentry *),
-	void *context);
+					 int fh_len, enum fid_type fileid_type,
+					 exportfs_acceptable_t acceptable,
+					 void *context);
 
 /*
  * Generic helpers for filesystems.
  */
+typedef struct inode *(*exportfs_get_inode_t)(struct super_block *sb,
+					      u64 ino, u32 gen);
+
 extern struct dentry *generic_fh_to_dentry(struct super_block *sb,
-	struct fid *fid, int fh_len, int fh_type,
-	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));
+	struct fid *fid, int fh_len, enum fid_type fh_type,
+	exportfs_get_inode_t get_inode);
 extern struct dentry *generic_fh_to_parent(struct super_block *sb,
-	struct fid *fid, int fh_len, int fh_type,
-	struct inode *(*get_inode) (struct super_block *sb, u64 ino, u32 gen));
+	struct fid *fid, int fh_len, enum fid_type fh_type,
+	exportfs_get_inode_t get_inode);
 
 #endif /* LINUX_EXPORTFS_H */
diff --git a/include/linux/reiserfs_fs.h b/include/linux/reiserfs_fs.h
index bc5114d..0783dbe 100644
--- a/include/linux/reiserfs_fs.h
+++ b/include/linux/reiserfs_fs.h
@@ -24,6 +24,7 @@
 #include <linux/proc_fs.h>
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
+#include <linux/exportfs.h>
 #include <linux/reiserfs_fs_i.h>
 #include <linux/reiserfs_fs_sb.h>
 #endif
@@ -1880,11 +1881,11 @@ int reiserfs_write_inode(struct inode *inode, int);
 int reiserfs_get_block(struct inode *inode, sector_t block,
 		       struct buffer_head *bh_result, int create);
 struct dentry *reiserfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
-				     int fh_len, int fh_type);
+				     int fh_len, enum fid_type fh_type);
 struct dentry *reiserfs_fh_to_parent(struct super_block *sb, struct fid *fid,
-				     int fh_len, int fh_type);
-int reiserfs_encode_fh(struct dentry *dentry, __u32 * data, int *lenp,
-		       int connectable);
+				     int fh_len, enum fid_type fh_type);
+enum fid_type reiserfs_encode_fh(struct dentry *dentry, __u32 * data, int *lenp,
+				 int connectable);
 
 int reiserfs_truncate_file(struct inode *, int update_timestamps);
 void make_cpu_key(struct cpu_key *cpu_key, struct inode *inode, loff_t offset,
diff --git a/mm/shmem.c b/mm/shmem.c
index f1b0d48..d6e91dd 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2047,7 +2047,7 @@ static int shmem_match(struct inode *ino, void *vfh)
 }
 
 static struct dentry *shmem_fh_to_dentry(struct super_block *sb,
-		struct fid *fid, int fh_len, int fh_type)
+		struct fid *fid, int fh_len, enum fid_type fh_type)
 {
 	struct inode *inode;
 	struct dentry *dentry = NULL;
@@ -2067,8 +2067,8 @@ static struct dentry *shmem_fh_to_dentry(struct super_block *sb,
 	return dentry;
 }
 
-static int shmem_encode_fh(struct dentry *dentry, __u32 *fh, int *len,
-				int connectable)
+static enum fid_type shmem_encode_fh(struct dentry *dentry, __u32 *fh, int *len,
+				     int connectable)
 {
 	struct inode *inode = dentry->d_inode;
 

Re: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[Phillip Lougher]

David Howells wrote:

>  (*) FAT has had its fh type #defined (FAT_FILEID).
> 
>  (*) FUSE has had its fh types #defined (FUSE_FILEID, FUSE_FILEID_PARENT).
> 
>  (*) ISOFS has had its fh types #defined (ISOFS_FILEID, ISOFS_FILEID_PARENT).
> 
>  (*) OCFS2 has had its fh types #defined (OCFS2_FILEID, OCFS2_FILEID_PARENT).
> 

You define these in the filesystem c files themselves as #defines.  UDF 
and BTRFS place theirs in the enum fid_type definition itself (in 
linux/exportfs.h).  Is there any reason why you didn't do this?

> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c

> -static int export_encode_fh(struct dentry *dentry, struct fid *fid,
> -		int *max_len, int connectable)
> +static enum fid_type export_encode_fh(struct dentry *dentry, struct fid *fid,
> +				      int *max_len, int connectable)
>  {
>  	struct inode * inode = dentry->d_inode;
>  	int len = *max_len;
>  	int type = FILEID_INO32_GEN;

         ^^^^

         this should be changed to enum fid_type
> -	
> +
>  	if (len < 2 || (connectable && len < 4))
> -		return 255;
> +		return FILEID_ERROR;
>  
>  	len = 2;
>  	fid->i32.ino = inode->i_ino;
> @@ -341,24 +341,71 @@ static int export_encode_fh(struct dentry *dentry, struct fid *fid,
>  	return type;
>  }
>  

> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -610,8 +610,11 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
>  	return ERR_PTR(err);
>  }
>  
> -static int fuse_encode_fh(struct dentry *dentry, u32 *fh, int *max_len,
> -			   int connectable)
> +#define FUSE_FILEID		((enum fid_type) 0x81)
> +#define FUSE_FILEID_PARENT	((enum fid_type) 0x82)

Move to enum fid_type in linux/exportfs.h?


> diff --git a/fs/isofs/export.c b/fs/isofs/export.c
> index e81a305..b66c083 100644
> --- a/fs/isofs/export.c
> +++ b/fs/isofs/export.c
> @@ -106,7 +106,10 @@ static struct dentry *isofs_export_get_parent(struct dentry *child)
>  	return rv;
>  }
>  
> -static int
> +#define ISOFS_FILEID		((enum fid_type) 1)
> +#define ISOFS_FILEID_PARENT	((enum fid_type) 2)
> +

Move to enum fid_type in linux/exportfs.h?

Type  1 and 2 are also used by the generic fid_types FILEID_INO32_GEN 
and FILEID_INO32_GEN_PARENT, but (obviously) their formats are different.

Phillip

Re: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[David Howells]

Phillip Lougher <phillip@lougher.demon.co.uk> wrote:

> You define these in the filesystem c files themselves as #defines.  UDF and
> BTRFS place theirs in the enum fid_type definition itself (in
> linux/exportfs.h).  Is there any reason why you didn't do this?

I don't know that we want to stick loads of filesystem-specific stuff into a
general header file.  I believe we used to do that for struct inode (IIRC
there used to be a large union with filesystem-specific members).

> >  	int type = FILEID_INO32_GEN;
> 
>         ^^^^
> 
>         this should be changed to enum fid_type

I missed that.  Updated.

> > +#define FUSE_FILEID		((enum fid_type) 0x81)
> > +#define FUSE_FILEID_PARENT	((enum fid_type) 0x82)
> 
> Move to enum fid_type in linux/exportfs.h?

As mentioned above, do we really want to do that?

> > +#define ISOFS_FILEID		((enum fid_type) 1)
> > +#define ISOFS_FILEID_PARENT	((enum fid_type) 2)
> > +
> 
> Move to enum fid_type in linux/exportfs.h?
> 
> Type  1 and 2 are also used by the generic fid_types FILEID_INO32_GEN and
> FILEID_INO32_GEN_PARENT, but (obviously) their formats are different.

I know.  As far as I know, there's no restriction on them reusing types 1 and
2, just an advisory note where these collide with other export types.  Perhaps
Al or Christoph would care to comment on this.

Also, I don't know that changing these types would necessarily be a good
idea.  wireshark, for example, could multiplex the types by the length as the
standard types are lengths 2 and 4, and ISOFS types are 3 and 5.

David

Re: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[Christoph Hellwig]

On Tue, Dec 02, 2008 at 09:02:33PM +0000, David Howells wrote:
> Update the exportfs documentation and comments to fit the code, and add an
> extra FID type to be used for error indication (FILEID_ERROR).
> 
> The following other code changes have been made:
> 
>  (*) The encode_fh() op and exportfs_encode_fh() return enum fid_type rather
>      than int.

Once you change the prototype please also pass the struct fid * instead
of the current __u32 array, like the fh_to_dentry / fh_to_parent
methods.

>  (*) The exportfs_encode_fh() function has it's acceptable() function pointer
>      typedef'd.

I don't really see the point for that, but I don't care too much.

>  (*) The fh_to_dentry() and fh_to_parent() ops and exportfs_decode_fh() take
>      enum fid_type rather than int.

Ok.

>  (*) The generic_fh_to_dentry() and generic_fh_to_parent() functions now take
>      enum fid_type rather than int.  Their get_inode() function pointer has
>      been typedef'd.

Ok.  Again I don't really see the point of the typede, but hey..

>  (*) The generic_fh_to_dentry() and generic_fh_to_parent() functions no longer
>      return NULL, but return -ESTALE instead.  exportfs_decode_fh() does not
>      check for NULL, but will try to dereference it as IS_ERR() does not check
>      for it.

>  (*) The following filesystems have had their fh_to_dentry() and fh_to_parent()
>      implementations made to return -ESTALE rather than NULL: FUSE, GFS2,
>      ISOFS, OCFS2, UDF and XFS.

These two should probably be broken out into a smaller patch, before all
the cosmetic bits.

>  (*) Returns from encode_fh() implementations of 255 are changed to
>      FILEID_ERROR.

>  (*) FAT has had its fh type #defined (FAT_FILEID).
> 
>  (*) FUSE has had its fh types #defined (FUSE_FILEID, FUSE_FILEID_PARENT).
> 
>  (*) ISOFS has had its fh types #defined (ISOFS_FILEID, ISOFS_FILEID_PARENT).
> 
>  (*) OCFS2 has had its fh types #defined (OCFS2_FILEID, OCFS2_FILEID_PARENT).

Ok.


> +A filehandle fragment consists of an array of 1 or more 32-bit words.  The
> +contents and the layout of the data in the array are entirely at the whim of
> +the filesystem that generated it.

Not really true anymore, as we have a real struct for it now.  But all
the length calculation is still done in number of 4 byte words.  Messy..

>  struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
> -		int fh_len, int fileid_type,
> -		int (*acceptable)(void *, struct dentry *), void *context)
> +				  int fh_len, enum fid_type fileid_type,
> +				  exportfs_acceptable_t acceptable,
> +				  void *context)

Why the strange indentation?  Two tabs indent for spillover prototypes
is pretty much standard..

> +#define FAT_FILEID	((enum fid_type) 3)

That cast is completely pointless in C.

> +++ b/include/linux/exportfs.h
> @@ -1,3 +1,9 @@
> +/* Persistent file handle encoding and decoding interface

/*
 * Persistent file handle encoding and decoding interface

Re: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[Christoph Hellwig]

On Wed, Dec 03, 2008 at 08:27:48AM +0000, Phillip Lougher wrote:
> You define these in the filesystem c files themselves as #defines.  UDF  
> and BTRFS place theirs in the enum fid_type definition itself (in  
> linux/exportfs.h).  Is there any reason why you didn't do this?

The reason why the IDs for new filesystems go into exportfs.h to make
sure they don't clash for different filesystems.  While re-using the
same ID to mean a different format is technically fine, it's not very
nice for people trying to e.g. decipher NFS traffic in wireshark.

Somewhere on my TODO list is an item to walk through all the filesystems
and make them send out different IDs for new requests that don't overlap
while still accepting their legacy IDs for traffic from the time before
a server reboot.

Re: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[David Howells]

Christoph Hellwig <hch@infradead.org> wrote:

> Once you change the prototype please also pass the struct fid * instead
> of the current __u32 array, like the fh_to_dentry / fh_to_parent
> methods.

I can do that, but why not pass a u32* or void* instead in all cases?  The
operations are under no obligation to use struct fid.

> >  (*) The exportfs_encode_fh() function has it's acceptable() function pointer
> >      typedef'd.
> 
> I don't really see the point for that, but I don't care too much.

It's passed around between a number of functions and it makes the function
declarations more readable.

> These two should probably be broken out into a smaller patch, before all
> the cosmetic bits.

Okay.

> > +A filehandle fragment consists of an array of 1 or more 32-bit words.  The
> > +contents and the layout of the data in the array are entirely at the whim of
> > +the filesystem that generated it.
> 
> Not really true anymore, as we have a real struct for it now.

Which isn't used by all exportable filesystems...  Should those filesystems
have specific entries added to the union in struct fid?  Should they be
required to use the raw member of struct fid's union?

In fact, why struct fid at all: why not union fid?

It also seems odd to declare a struct that only has passing acquaintance with
reality.  struct fid is rarely used to its full extent, and may not even be
big enough - something that's decided on a per-use basis.

Furthermore, when the decode routines are called, less data may be presented
than is required to fill out a struct fid, so overlaying a struct fid on it
may include uninitialised data.

As I see it, struct fid doesn't gain anything - except perhaps documentary
purposes for people looking to build packet sniffers.  Even then it's not
really necessary, and nor is it complete.

The i32 branch of struct fid could be hidden inside fs/expfs.c, and the udf
branch inside fs/udf/.  The layouts could then be documented inside the
Documentation directory.

> But all the length calculation is still done in number of 4 byte words.
> Messy..

Yes.  The docs and comments either didn't specify the units, or stipulated
that they were in bytes:-(

This is one reason why I think passing a u32* might be better than struct fid
- that way the length is more obviously the size of the array in u32 units.

> Why the strange indentation?  Two tabs indent for spillover prototypes
> is pretty much standard..

So is aligning the spillover horizontally with the start of the scope (if
that's the right term).  It makes more sense too because relative indentation
gives a visual clue to relative significance.

> > +#define FAT_FILEID	((enum fid_type) 3)
> 
> That cast is completely pointless in C.

True, I suppose.

David

Re: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[Jörn Engel]

On Wed, 3 December 2008 13:14:03 +0000, David Howells wrote:
> Christoph Hellwig <hch@infradead.org> wrote:
> 
> > Why the strange indentation?  Two tabs indent for spillover prototypes
> > is pretty much standard..
> 
> So is aligning the spillover horizontally with the start of the scope (if
> that's the right term).  It makes more sense too because relative indentation
> gives a visual clue to relative significance.

It is also more susceptible to bitrot, though. :)

Jörn

-- 
--
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: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[David Howells]

Jörn Engel <joern@logfs.org> wrote:

> > So is aligning the spillover horizontally with the start of the scope (if
> > that's the right term).  It makes more sense too because relative indentation
> > gives a visual clue to relative significance.
> 
> It is also more susceptible to bitrot, though. :)

M-C-q is your friend:-)

David
--
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: [PATCH] Update the exportfs documentation and comments and fix some exportfs issues

[Phillip Lougher]

David Howells wrote:
> Phillip Lougher <phillip@lougher.demon.co.uk> wrote:
> 
>> You define these in the filesystem c files themselves as #defines.  UDF and
>> BTRFS place theirs in the enum fid_type definition itself (in
>> linux/exportfs.h).  Is there any reason why you didn't do this?
> 
> I don't know that we want to stick loads of filesystem-specific stuff into a
> general header file.  I believe we used to do that for struct inode (IIRC
> there used to be a large union with filesystem-specific members).

FYI yes we did, back in the 2.4.x days, not only for struct inode but 
also for struct super-block.

Personally I don't care where things are put, just as long as there's 
consensus.

> I know.  As far as I know, there's no restriction on them reusing types 1 and
> 2, just an advisory note where these collide with other export types.  Perhaps
> Al or Christoph would care to comment on this.
> 
> Also, I don't know that changing these types would necessarily be a good
> idea.  wireshark, for example, could multiplex the types by the length as the
> standard types are lengths 2 and 4, and ISOFS types are 3 and 5.
> 

Quite, except ocfs2 also uses type 1 with a length of 3 :-)  (it uses 
type 2 with a length of 6).

People on LKML often complain too few people review patches.  I get the 
feeling I wish I never bothered here.

Phillip