Re: [PATCH -V8 2/9] vfs: Add name to file handle conversion support

[Aneesh Kumar K. V]

On Wed, 19 May 2010 14:22:22 +0530, "Aneesh Kumar K. V" <aneesh.kumar@xxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 19 May 2010 16:15:51 +0900, "J. R. Okajima" <hooanon05@xxxxxxxxxxx> wrote:
> > 
> > "Aneesh Kumar K. V":
> > > Now that we are not doing UUID based vfsmount lookup this make
> > > sense. Will update in the next iteration with UUID to be part of
> > > super_block.
> > 
> > Because this UUID is just for some FS's userspace helpers (in other
> > words, returning UUID is FS specific behaviour), I am afraid it is not a
> > good ideat to put the array into generic super_block.
> 
> UUID should be looked at as the file system identifier and IMHO struct
> super_block is the right place to hold it. For ex: ext*  put then in
> ext*_super_block. File system that doesn't support UUID can leave the
> superblock field zero filled.
> 
> 
> > 
> > About the design or approach, this might have been discussed earlier,
> > but I'd like to suggest to clarify some points here.
> > - While these new systemcalls provide generic features, the
> >   implementation depends upon s_export_op, ie. NFS-exporting.
> >   It means there are two requirements for these systemcalls, enabling
> >   CONFIG_EXPORTFS and FS has to set s_export_op.
> >   Is this acceptable?
> 
> 
> I think exportfs is the right interface we want to depend on for
> generating a handle. We should not be having another parallel interface for file
> handle generation. But agreed that we should return -EOPNOTSUPP in case
> EXPORTFS is disabled.
> 
> 
> > 
> > - exportfs_encode_fh() supports the default encoding
> >   export_encode_fh(), but exportfs_decode_fh() doesn't.
> >   The latest patch series modifes exportfs_decode_fh() to return ESTALE,
> >   but I'd suggest to make the caller of export_encode_fh() to check
> >   s_export_op->fh_to_dentry() and return ENOSYS.
> 
> I will fix to make the syscall return EOPNOTSUPP in case fh_to_dentry is
> not supported. But i guess we still need to keep the change in
> exportfs_decode_fh to return -ESTALE in case these operations are not definied.
> 
> 
> 
> >   Or implement the default decode routine as a contrast of
> >   export_encode_fh().
> > 
> > - Some FS (or its userspace helper) may want to put UUID into the
> >   handle. If those FS already have UUID in their fs private_data, then
> >   putting a pointer (instead of an array) is better.
> >   Or creating two new operations in s_op to encode/decode handle
> >   may be good too (regardless CONFIG_EXPORTFS). The generic
> >   implementations should be provided, and when these function pointers
> >   in s_op are not set, they should be called as default. These generic
> >   implementaions will be able to be used by expfs.c too. And UUID in
> >   super_block will be unnecessary.
> 
> 
> IMHO that would be over design. We can depend on exportfs
> interfaces and if not defined return EOPNOTSUPP.
> 

How about the below patch ?

commit 5f421ffbe9dd7bb84c5992b1725c06b511bc76d8
Author: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
Date:   Wed May 19 14:52:44 2010 +0530

    vfs: Return ENOSYS if CONFIG_EXPORTFS is not enabled
    
    Both the syscalls need exportfs support enabled. So if CONFIG_EXPORTFS
    is not enabled return ENOSYS. While converting name to handle check
    whether the filesystem support handle decode. If not return EOPNOTSUPP.
    We don't do a similar check in open by handle because exportfs_decode_fh
    will return ESTALE if file system doesn't support fh_to_dentry callback.
    
    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>

diff --git a/fs/open.c b/fs/open.c
index 14e6d3c..b5fc067 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1274,6 +1274,9 @@ SYSCALL_DEFINE4(name_to_handle_at, int, dfd, const char __user *, name,
 	long ret = -EINVAL;
 	struct path path;
 
+#ifndef CONFIG_EXPORTFS
+	return -ENOSYS;
+#endif
 	if ((flag & ~AT_SYMLINK_FOLLOW) != 0)
 		goto err_out;
 
@@ -1281,6 +1284,14 @@ SYSCALL_DEFINE4(name_to_handle_at, int, dfd, const char __user *, name,
 	ret = user_path_at(dfd, name, follow, &path);
 	if (ret)
 		goto err_out;
+	/*
+	 * We need t make sure wether the file system
+	 * support decoding of the file handle
+	 */
+	if (!path.mnt->mnt_sb->s_export_op ||
+		!path.mnt->mnt_sb->s_export_op->fh_to_dentry)
+		return -EOPNOTSUPP;
+
 	ret = do_sys_name_to_handle(&path, handle);
 	path_put(&path);
 err_out:
@@ -1450,6 +1461,9 @@ SYSCALL_DEFINE3(open_by_handle, int, mountdirfd,
 {
 	long ret;
 
+#ifndef CONFIG_EXPORTFS
+	return -ENOSYS;
+#endif
 	if (force_o_largefile())
 		flags |= O_LARGEFILE;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/