Re: [PATCH -V2] Generic name to handle and open by handle syscalls

["Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>]

On Thu, 18 Mar 2010 13:31:45 -0400, Christoph Hellwig <hch@infradead.org> wrote:

Hi Christoph,

> I haven't looked at the code in detail yet, but the real value add for
> userspace nfs servers and similar would be globally unique handles.
> This of course requires filesystem support, but having a way to export
> the filesystem handle would also simplify nfsd a lot so that it doesn't
> have to rely on hacked support to pass this in from userspace which
> gets it from statfs, assuming f_fsid has parts of a uuid in it, or
> blkid.
> 
> This might be as simple as adding an s_uuid array to the superblock and
> having some simple routines to iterate it, or we might make that
> an export operation to allow a bit more flexibility.

As per the private email exchanges we had on this, do you agree that we
would need the vfsmount to successfully open a file by handle ? If yes
we would need to specify the mount point via mountfd. In that case do we
need the handle returned by kernel to be system wide unique ? 

We can build uniqueness in the userspace based on the mountfd and that
also enables us to use different field width for file system
identifier. So rather than forcing the usage of uuid, userspace can now
decided to use a fsid that is smaller and that uniquely identify only the
vfsmounts that the NFS server is exporting  ?

If you agree with the above do you see anything missing in the set of
patches posted. Are they merge ready ?


NOTE: In-lining below the patch that show why we would need a vfsmount
for open-by-handle.

-aneesh

I actually did a patch with not acceptable check. Where i am stuck is
the reconnect_path call for directories. That would need a vfsmount. I
can do a variant with struct super_block but then exportfs_get_name
needs a vfsmount for opening the parent directory using dentry_open
(get_name). We also need a vfsmount to do the final dentry_open after
we do the handle to dentry conversion in open_by_handle syscall.


diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index e9e1759..97f04fa 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -486,4 +486,74 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
 }
 EXPORT_SYMBOL_GPL(exportfs_decode_fh);

+static struct super_block *fs_get_sb(__kernel_fsid_t *fsid)
+{
+	struct super_block *sb, *found_sb = NULL;
+	struct file_system_type *fs_type;
+
+	read_lock(&file_systems_lock);
+retry:
+	fs_type = file_systems;
+	while (fs_type) {
+		list_for_each_entry(sb, &fs_type->fs_supers, s_instances) {
+			this_fsid = sb->get_fsid();
+			if (!memcmp(fsid, this_fsid, sizeof(__kernel_fsid_t))) {
+				/* found the matching super_block */
+				if (!grab_super(sb))
+					goto retry;
+				else
+					found_sb = sb;
+			}
+		}
+		fs = fs->next;
+	}
+	read_unlock(&file_systems_lock);
+	return found_sb;
+
+}
+
+struct dentry *exportfs_decode_unique_fh(__kernel_fsid_t *fsid, struct fid *fid,
+					int fh_len, int fileid_type)
+{
+	int err;
+	struct dentry *result;
+	char nbuf[NAME_MAX+1];
+	const struct export_operations *nop;
+	struct super_block  *sb = fs_get_sb(fsid);
+
+	if (!sb)
+		return ERR_PTR(-ESTALE);
+	nop = sb->s_export_op;
+	/*
+	 * Try to get any dentry for the given file handle from the filesystem.
+	 */
+	result = nop->fh_to_dentry(sb, fid, fh_len, fileid_type);
+	if (!result)
+		result = ERR_PTR(-ESTALE);
+	if (IS_ERR(result))
+		return result;
+
+	if (S_ISDIR(result->d_inode->i_mode)) {
+		/*
+		 * This request is for a directory.
+		 *
+		 * On the positive side there is only one dentry for each
+		 * directory inode.  On the negative side this implies that we
+		 * to ensure our dentry is connected all the way up to the
+		 * filesystem root.
+		 */
+		if (result->d_flags & DCACHE_DISCONNECTED) {
+                       /*FIXME!!!! We need vfsmount here */
+			err = reconnect_path(mnt, result, nbuf);
+			if (err)
+				goto err_result;
+		}
+		return result;
+	}
+	return result;
+err_result:
+	dput(result);
+	return ERR_PTR(err);
+}
+EXPORT_SYMBOL_GPL(exportfs_decode_unique_fh);
+
 MODULE_LICENSE("GPL");