[PATCH -V4] Generic name to handle and open by handle syscalls

[PATCH -V4] Generic name to handle and open by handle syscalls

[Aneesh Kumar K.V]

Hi,

The below set of patches implement open by handle support using exportfs
operations. This allows user space application to map a file name to file 
handle and later open the file using handle. This should be usable
for userspace NFS [1] and 9P server [2]. XFS already support this with the ioctls
XFS_IOC_PATH_TO_HANDLE and XFS_IOC_OPEN_BY_HANDLE.

[1] http://nfs-ganesha.sourceforge.net/
[2] http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01087.html

TODO:
I guess we would need to optimize how we get the vfsmount for the filesystem
uuid specified. Searching the file system list and task name space may be a big
overhead for each open by handle call.

Changes from V3:
a) Code cleanup suggested by Andreas
b) x86_64 syscall support
c) add compat syscall

Chages from V2:
a) Support system wide unique handle.

Changes from v1:
a) handle size is now specified in bytes
b) returns -EOVERFLOW if the handle size is small
c) dropped open_handle syscall and added open_by_handle_at syscall
   open_by_handle_at takes mount_fd as the directory fd of the mount point
   containing the file
e) handle will only be unique in a given file system. So for an NFS server
   exporting multiple file system, NFS server will have to internally track the
   mount point to which a file handle belongs to. We should be able to do it much
   easily than expecting kernel to give a system wide unique file handle. System
   wide unique file handle would need much larger changes to the exportfs or VFS
   interface and I was not sure whether we really need to do that in the kernel or
   in the user space
f) open_handle_at now only check for DAC_OVERRIDE capability


Example program: (x86_32). (x86_64 would need a different syscall number)
----------------
#include <stdio.h>
#include <stdlib.h>

#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <string.h>

struct uuid {
	char uuid[16];
};
struct file_handle {
        int handle_size;
        int handle_type;
	struct uuid fsid;
        void *handle;
};

static int name_to_handle(const char *name, struct file_handle  *fh)
{
	return syscall(338, name, fh);
}

static int open_by_handle(struct file_handle *fh,  int flags)
{
	return syscall(339, fh, flags);
}

int main(int argc, char *argv[])
{
        int ret;
        int fd, dirfd;
        char buf[100];
        struct file_handle fh;
        fh.handle_size = 0;
again:
	if (fh.handle_size)
        	fh.handle = malloc(fh.handle_size);
        fh.handle_type = 0;
        errno  = 0;
        ret = name_to_handle(argv[1], &fh);
        if (ret) {
                perror("Error:");
		printf("Found the handle size needed to be %d\n", fh.handle_size);
		printf("Trying again..\n");
		goto again;
                exit(1);
        }
        fd = open_by_handle(&fh, O_RDONLY);
        if (fd <= 0 ) {
                perror("Error:");
                exit(1);
        }
        memset(buf, 0 , 100);
        while (read(fd, buf, 100) > 0) {
                printf("%s", buf);
                memset(buf, 0 , 100);
        }
        return 0;
}

-aneesh

[PATCH -V4 1/6] exportfs: Return the minimum required handle size

[Aneesh Kumar K.V]

The exportfs encode handle function should return the minimum required
handle size. This helps user to find out the handle size by passing 0
handle size in the first step and then redoing to the call again with
the returned handle size value.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/btrfs/export.c             |    8 ++++++--
 fs/exportfs/expfs.c           |    9 +++++++--
 fs/fat/inode.c                |    4 +++-
 fs/fuse/inode.c               |    4 +++-
 fs/gfs2/export.c              |    8 ++++++--
 fs/isofs/export.c             |    8 ++++++--
 fs/ocfs2/export.c             |    8 +++++++-
 fs/reiserfs/inode.c           |    7 ++++++-
 fs/udf/namei.c                |    7 ++++++-
 fs/xfs/linux-2.6/xfs_export.c |    4 +++-
 mm/shmem.c                    |    4 +++-
 11 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
index 951ef09..5f8ee5a 100644
--- a/fs/btrfs/export.c
+++ b/fs/btrfs/export.c
@@ -21,9 +21,13 @@ static int btrfs_encode_fh(struct dentry *dentry, u32 *fh, int *max_len,
 	int len = *max_len;
 	int type;
 
-	if ((len < BTRFS_FID_SIZE_NON_CONNECTABLE) ||
-	    (connectable && len < BTRFS_FID_SIZE_CONNECTABLE))
+	if (connectable && (len < BTRFS_FID_SIZE_CONNECTABLE)) {
+		*max_len = BTRFS_FID_SIZE_CONNECTABLE;
 		return 255;
+	} else if (len < BTRFS_FID_SIZE_NON_CONNECTABLE) {
+		*max_len = BTRFS_FID_SIZE_NON_CONNECTABLE;
+		return 255;
+	}
 
 	len  = BTRFS_FID_SIZE_NON_CONNECTABLE;
 	type = FILEID_BTRFS_WITHOUT_PARENT;
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index e9e1759..cfee0f0 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -319,9 +319,14 @@ static int export_encode_fh(struct dentry *dentry, struct fid *fid,
 	struct inode * inode = dentry->d_inode;
 	int len = *max_len;
 	int type = FILEID_INO32_GEN;
-	
-	if (len < 2 || (connectable && len < 4))
+
+	if (connectable && (len < 4)) {
+		*max_len = 4;
+		return 255;
+	} else if (len < 2) {
+		*max_len = 2;
 		return 255;
+	}
 
 	len = 2;
 	fid->i32.ino = inode->i_ino;
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 0ce143b..6f83bc7 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -738,8 +738,10 @@ fat_encode_fh(struct dentry *de, __u32 *fh, int *lenp, int connectable)
 	struct inode *inode =  de->d_inode;
 	u32 ipos_h, ipos_m, ipos_l;
 
-	if (len < 5)
+	if (len < 5) {
+		*lenp = 5;
 		return 255; /* no room */
+	}
 
 	ipos_h = MSDOS_I(inode)->i_pos >> 8;
 	ipos_m = (MSDOS_I(inode)->i_pos & 0xf0) << 24;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index ec14d19..beaea69 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -638,8 +638,10 @@ static int fuse_encode_fh(struct dentry *dentry, u32 *fh, int *max_len,
 	u64 nodeid;
 	u32 generation;
 
-	if (*max_len < len)
+	if (*max_len < len) {
+		*max_len = len;
 		return  255;
+	}
 
 	nodeid = get_fuse_inode(inode)->nodeid;
 	generation = inode->i_generation;
diff --git a/fs/gfs2/export.c b/fs/gfs2/export.c
index c22c211..d022236 100644
--- a/fs/gfs2/export.c
+++ b/fs/gfs2/export.c
@@ -36,9 +36,13 @@ static int gfs2_encode_fh(struct dentry *dentry, __u32 *p, int *len,
 	struct super_block *sb = inode->i_sb;
 	struct gfs2_inode *ip = GFS2_I(inode);
 
-	if (*len < GFS2_SMALL_FH_SIZE ||
-	    (connectable && *len < GFS2_LARGE_FH_SIZE))
+	if (connectable && (*len < GFS2_LARGE_FH_SIZE)) {
+		*len = GFS2_LARGE_FH_SIZE;
 		return 255;
+	} else if (*len < GFS2_SMALL_FH_SIZE) {
+		*len = GFS2_SMALL_FH_SIZE;
+		return 255;
+	}
 
 	fh[0] = cpu_to_be32(ip->i_no_formal_ino >> 32);
 	fh[1] = cpu_to_be32(ip->i_no_formal_ino & 0xFFFFFFFF);
diff --git a/fs/isofs/export.c b/fs/isofs/export.c
index ed752cb..dd4687f 100644
--- a/fs/isofs/export.c
+++ b/fs/isofs/export.c
@@ -124,9 +124,13 @@ isofs_export_encode_fh(struct dentry *dentry,
 	 * offset of the inode and the upper 16 bits of fh32[1] to
 	 * hold the offset of the parent.
 	 */
-
-	if (len < 3 || (connectable && len < 5))
+	if (connectable && (len < 5)) {
+		*max_len = 5;
+		return 255;
+	} else if (len < 3) {
+		*max_len = 3;
 		return 255;
+	}
 
 	len = 3;
 	fh32[0] = ei->i_iget5_block;
diff --git a/fs/ocfs2/export.c b/fs/ocfs2/export.c
index 19ad145..250a347 100644
--- a/fs/ocfs2/export.c
+++ b/fs/ocfs2/export.c
@@ -201,8 +201,14 @@ static int ocfs2_encode_fh(struct dentry *dentry, u32 *fh_in, int *max_len,
 		   dentry->d_name.len, dentry->d_name.name,
 		   fh, len, connectable);
 
-	if (len < 3 || (connectable && len < 6)) {
+	if (connectable && (len < 6)) {
 		mlog(ML_ERROR, "fh buffer is too small for encoding\n");
+		*max_len = 6;
+		type = 255;
+		goto bail;
+	} else if (len < 3) {
+		mlog(ML_ERROR, "fh buffer is too small for encoding\n");
+		*max_len = 3;
 		type = 255;
 		goto bail;
 	}
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index dc2c65e..5fff1e2 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -1588,8 +1588,13 @@ int reiserfs_encode_fh(struct dentry *dentry, __u32 * data, int *lenp,
 	struct inode *inode = dentry->d_inode;
 	int maxlen = *lenp;
 
-	if (maxlen < 3)
+	if (need_parent && (maxlen < 5)) {
+		*lenp = 5;
 		return 255;
+	} else if (maxlen < 3) {
+		*lenp = 3;
+		return 255;
+	}
 
 	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 7581602..37ce713 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -1360,8 +1360,13 @@ static int udf_encode_fh(struct dentry *de, __u32 *fh, int *lenp,
 	struct fid *fid = (struct fid *)fh;
 	int type = FILEID_UDF_WITHOUT_PARENT;
 
-	if (len < 3 || (connectable && len < 5))
+	if (connectable && (len < 5)) {
+		*lenp = 5;
+		return 255;
+	} else if (len < 3) {
+		*lenp = 3;
 		return 255;
+	}
 
 	*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 846b75a..82c0553 100644
--- a/fs/xfs/linux-2.6/xfs_export.c
+++ b/fs/xfs/linux-2.6/xfs_export.c
@@ -81,8 +81,10 @@ xfs_fs_encode_fh(
 	 * seven combinations work.  The real answer is "don't use v2".
 	 */
 	len = xfs_fileid_length(fileid_type);
-	if (*max_len < len)
+	if (*max_len < len) {
+		*max_len = len
 		return 255;
+	}
 	*max_len = len;
 
 	switch (fileid_type) {
diff --git a/mm/shmem.c b/mm/shmem.c
index eef4ebe..bbeda1c 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2125,8 +2125,10 @@ static int shmem_encode_fh(struct dentry *dentry, __u32 *fh, int *len,
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (*len < 3)
+	if (*len < 3) {
+		*len = 3;
 		return 255;
+	}
 
 	if (hlist_unhashed(&inode->i_hash)) {
 		/* Unfortunately insert_inode_hash is not idempotent,
-- 
1.7.0.4.360.g11766c

[PATCH -V4 2/6] vfs: Add name to file handle conversion support

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/open.c          |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |   19 ++++++++++
 2 files changed, 114 insertions(+), 0 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 74e5cd9..dd27cbd 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -30,6 +30,8 @@
 #include <linux/falloc.h>
 #include <linux/fs_struct.h>
 #include <linux/ima.h>
+#include <linux/exportfs.h>
+#include <linux/mnt_namespace.h>
 
 #include "internal.h"
 
@@ -1206,3 +1208,96 @@ int nonseekable_open(struct inode *inode, struct file *filp)
 }
 
 EXPORT_SYMBOL(nonseekable_open);
+
+/* limit the handle size to some value */
+#define MAX_HANDLE_SZ 4096
+long do_sys_name_to_handle(const char __user *name,
+				struct file_handle *fh)
+{
+	int retval;
+	int handle_size;
+	struct path path;
+	struct inode *inode;
+	void *handle = NULL;
+	struct super_block *sb;
+	struct uuid *this_fs_id;
+
+	if (fh->handle_size > MAX_HANDLE_SZ)
+		return -EINVAL;
+
+	retval = user_lpath(name, &path);
+	if (retval)
+		return retval;
+
+	inode = path.dentry->d_inode;
+	/*
+	 * name to handle conversion only done for regular files
+	 * directories and symbolic links
+	 */
+	if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode) &&
+		!S_ISLNK(inode->i_mode)) {
+		retval =  -EINVAL;
+		goto err_out;
+	}
+	if (fh->handle_size > 0) {
+		handle = kmalloc(fh->handle_size, GFP_KERNEL);
+		if (!handle) {
+			retval = -ENOMEM;
+			goto err_out;
+		}
+
+		handle_size = fh->handle_size;
+	} else
+		handle_size = 0;
+	/* we ask for a non connected handle */
+	retval = exportfs_encode_fh(path.dentry, (struct fid *)handle,
+				&handle_size,  0);
+	/* convert handle size to bytes */
+	handle_size *= sizeof(u32);
+	fh->handle_type = retval;
+	if (handle_size <= fh->handle_size) {
+		if (copy_to_user(fh->f_handle, handle,
+					handle_size))
+			retval = -EFAULT;
+		else
+			retval = 0;
+	} else
+		retval = -EOVERFLOW;
+	fh->handle_size = handle_size;
+	sb = path.mnt->mnt_sb;
+	if (!retval && sb->s_op->get_fsid) {
+		this_fs_id = sb->s_op->get_fsid(sb);
+		memcpy(fh->fsid.uuid, this_fs_id->uuid, sizeof(fh->fsid.uuid));
+	} else
+		memset(fh->fsid.uuid, 0, sizeof(fh->fsid.uuid));
+	kfree(handle);
+
+err_out:
+	path_put(&path);
+	return retval;
+}
+
+SYSCALL_DEFINE2(name_to_handle, const char __user *, name,
+		struct file_handle __user *, handle)
+{
+	long ret;
+	struct file_handle f_handle;
+	if (copy_from_user(&f_handle, handle, sizeof(struct file_handle))) {
+		ret = -EFAULT;
+		goto err_out;
+	}
+	ret = do_sys_name_to_handle(name, &f_handle);
+	if (copy_to_user(&handle->handle_type,
+			&f_handle.handle_type, sizeof(f_handle.handle_type)) ||
+		copy_to_user(&handle->handle_size,
+			&f_handle.handle_size, sizeof(f_handle.handle_size)) ||
+		copy_to_user(handle->fsid.uuid,
+			f_handle.fsid.uuid, sizeof(f_handle.fsid.uuid))) {
+
+		ret = -EFAULT;
+	}
+err_out:
+	/* avoid REGPARM breakage on x86: */
+	asmlinkage_protect(2, ret, name, handle);
+	return ret;
+}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 39d57bc..157ed57 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -948,6 +948,20 @@ struct file {
 	unsigned long f_mnt_write_state;
 #endif
 };
+
+struct uuid {
+	char uuid[16];
+};
+
+struct file_handle {
+	int handle_size;
+	int handle_type;
+	/* File system identifier */
+	struct uuid fsid;
+	/* file identifier */
+	void *f_handle;
+};
+
 extern spinlock_t files_lock;
 #define file_list_lock() spin_lock(&files_lock);
 #define file_list_unlock() spin_unlock(&files_lock);
@@ -1580,6 +1594,7 @@ struct super_operations {
 	ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
 #endif
 	int (*bdev_try_to_free_page)(struct super_block*, struct page*, gfp_t);
+	struct uuid *(*get_fsid)(struct super_block *);
 };
 
 /*
@@ -1918,6 +1933,8 @@ extern struct file * dentry_open(struct dentry *, struct vfsmount *, int,
 				 const struct cred *);
 extern int filp_close(struct file *, fl_owner_t id);
 extern char * getname(const char __user *);
+extern long do_sys_name_to_handle(const char __user *name,
+				struct file_handle *fh);
 
 /* fs/ioctl.c */
 
@@ -2328,6 +2345,8 @@ extern struct super_block *get_super(struct block_device *);
 extern struct super_block *get_active_super(struct block_device *bdev);
 extern struct super_block *user_get_super(dev_t);
 extern void drop_super(struct super_block *sb);
+extern struct super_block *fs_get_sb(struct uuid *fsid);
+
 
 extern int dcache_dir_open(struct inode *, struct file *);
 extern int dcache_dir_close(struct inode *, struct file *);
-- 
1.7.0.4.360.g11766c

[PATCH -V4 3/6] vfs: Add open by file handle support

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/filesystems.c              |   32 ++++++++-
 fs/namei.c                    |   24 ------
 fs/namespace.c                |   38 ++++++++++
 fs/open.c                     |  155 +++++++++++++++++++++++++++++++++++++++++
 fs/pnode.c                    |    2 +-
 include/linux/fs.h            |    1 +
 include/linux/mnt_namespace.h |    2 +
 include/linux/namei.h         |   24 ++++++
 8 files changed, 252 insertions(+), 26 deletions(-)

diff --git a/fs/filesystems.c b/fs/filesystems.c
index 68ba492..743d36e 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -281,5 +281,35 @@ struct file_system_type *get_fs_type(const char *name)
 	}
 	return fs;
 }
-
 EXPORT_SYMBOL(get_fs_type);
+
+struct super_block *fs_get_sb(struct uuid *fsid)
+{
+	struct uuid *this_fsid;
+	struct file_system_type *fs_type;
+	struct super_block *sb, *found_sb = NULL;
+
+	read_lock(&file_systems_lock);
+	fs_type = file_systems;
+	while (fs_type) {
+		spin_lock(&sb_lock);
+		list_for_each_entry(sb, &fs_type->fs_supers, s_instances) {
+			if (!sb->s_op->get_fsid)
+				continue;
+			this_fsid = sb->s_op->get_fsid(sb);
+			if (!memcmp(fsid->uuid, this_fsid->uuid,
+					sizeof(this_fsid->uuid))) {
+				/* found the matching super_block */
+				atomic_inc(&sb->s_active);
+				found_sb = sb;
+				spin_unlock(&sb_lock);
+				goto out;
+			}
+		}
+		spin_unlock(&sb_lock);
+		fs_type = fs_type->next;
+	}
+out:
+	read_unlock(&file_systems_lock);
+	return found_sb;
+}
diff --git a/fs/namei.c b/fs/namei.c
index a7dce91..a18711e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1521,30 +1521,6 @@ out_unlock:
 	return may_open(&nd->path, 0, open_flag & ~O_TRUNC);
 }
 
-/*
- * Note that while the flag value (low two bits) for sys_open means:
- *	00 - read-only
- *	01 - write-only
- *	10 - read-write
- *	11 - special
- * it is changed into
- *	00 - no permissions needed
- *	01 - read-permission
- *	10 - write-permission
- *	11 - read-write
- * for the internal routines (ie open_namei()/follow_link() etc)
- * This is more logical, and also allows the 00 "no perm needed"
- * to be used for symlinks (where the permissions are checked
- * later).
- *
-*/
-static inline int open_to_namei_flags(int flag)
-{
-	if ((flag+1) & O_ACCMODE)
-		flag++;
-	return flag;
-}
-
 static int open_will_truncate(int flag, struct inode *inode)
 {
 	/*
diff --git a/fs/namespace.c b/fs/namespace.c
index 8174c8a..6168526 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2364,3 +2364,41 @@ void put_mnt_ns(struct mnt_namespace *ns)
 	kfree(ns);
 }
 EXPORT_SYMBOL(put_mnt_ns);
+
+/*
+ * Get any vfsmount mapping the superblock in the
+ * task namespace
+ */
+struct vfsmount *fs_get_vfsmount(struct task_struct *task,
+				struct super_block *sb)
+{
+	struct nsproxy *nsp;
+	struct list_head *mount_list;
+	struct mnt_namespace *ns = NULL;
+	struct vfsmount *mnt, *sb_mnt = NULL;
+
+	rcu_read_lock();
+	nsp = task_nsproxy(task);
+	if (nsp) {
+		ns = nsp->mnt_ns;
+		if (ns)
+			get_mnt_ns(ns);
+	}
+	rcu_read_unlock();
+	if (!ns)
+		return NULL;
+	down_read(&namespace_sem);
+	list_for_each(mount_list, &ns->list) {
+		mnt = list_entry(mount_list, struct vfsmount, mnt_list);
+		if (mnt->mnt_sb == sb) {
+			/* found the matching super block */
+			sb_mnt = mnt;
+			mntget(sb_mnt);
+			break;
+		}
+	}
+	up_read(&namespace_sem);
+
+	put_mnt_ns(ns);
+	return sb_mnt;
+}
diff --git a/fs/open.c b/fs/open.c
index dd27cbd..ca09c2d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1301,3 +1301,158 @@ err_out:
 	asmlinkage_protect(2, ret, name, handle);
 	return ret;
 }
+
+static int vfs_dentry_acceptable(void *context, struct dentry *dentry)
+{
+	return 1;
+}
+
+static struct dentry *handle_to_dentry(struct vfsmount *mnt,
+				struct file_handle *fh)
+{
+	int retval = 0;
+	int handle_size;
+	void *handle = NULL;
+	struct dentry *dentry;
+
+	if ((fh->handle_size > MAX_HANDLE_SZ) ||
+		(fh->handle_size <= 0)) {
+		retval = -EINVAL;
+		goto err_out;
+	}
+	handle = kmalloc(fh->handle_size, GFP_KERNEL);
+	if (!handle) {
+		retval =  -ENOMEM;
+		goto err_out;
+	}
+	if (copy_from_user(handle, fh->f_handle, fh->handle_size)) {
+		retval = -EFAULT;
+		goto err_out;
+	}
+	/* change the handle size to multiple of sizeof(u32) */
+	handle_size = fh->handle_size >> 2;
+	dentry = exportfs_decode_fh(mnt, (struct fid *)handle,
+					handle_size, fh->handle_type,
+					vfs_dentry_acceptable, NULL);
+	kfree(handle);
+	return dentry;
+
+err_out:
+	kfree(handle);
+	return ERR_PTR(retval);
+}
+
+long do_sys_open_by_handle(struct file_handle *fh, int flags)
+{
+	int fd;
+	int retval = 0;
+	int d_flags  = flags;
+	struct file *filp;
+	struct vfsmount *mnt;
+	struct inode *inode;
+	struct dentry *dentry;
+	struct super_block *sb;
+
+	if (!capable(CAP_DAC_OVERRIDE))
+		return -EPERM;
+
+	sb = fs_get_sb(&fh->fsid);
+	if (!sb)
+		return -ESTALE;
+	/*
+	 * Find the vfsmount for this superblock in the
+	 * current namespace
+	 */
+	mnt = fs_get_vfsmount(current, sb);
+	if (!mnt) {
+		retval = -ESTALE;
+		goto out_sb;
+	}
+
+	dentry = handle_to_dentry(mnt, fh);
+	if (IS_ERR(dentry)) {
+		retval = PTR_ERR(dentry);
+		goto out_mnt;
+	}
+
+	inode = dentry->d_inode;
+	/* Restrict open_by_handle to directories & regular files. */
+	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))) {
+		retval = -EINVAL;
+		goto out_err;
+	}
+
+	flags  = open_to_namei_flags(flags);
+	/* O_TRUNC implies we need access checks for write permissions */
+	if (flags & O_TRUNC)
+		flags |= MAY_WRITE;
+
+	if ((!(flags & O_APPEND) || (flags & O_TRUNC)) &&
+		(flags & FMODE_WRITE) && IS_APPEND(inode)) {
+		retval = -EPERM;
+		goto out_err;
+	}
+
+	if ((flags & FMODE_WRITE) && IS_IMMUTABLE(inode)) {
+		retval = -EACCES;
+		goto out_err;
+	}
+
+	/* Can't write directories. */
+	if (S_ISDIR(inode->i_mode) && (flags & FMODE_WRITE)) {
+		retval = -EISDIR;
+		goto out_err;
+	}
+
+	fd = get_unused_fd();
+	if (fd < 0) {
+		retval = fd;
+		goto out_err;
+	}
+
+	filp = dentry_open(dentry, mntget(mnt),
+			d_flags, current_cred());
+	if (IS_ERR(filp)) {
+		put_unused_fd(fd);
+		retval =  PTR_ERR(filp);
+		goto out_err;
+	}
+
+	if (inode->i_mode & S_IFREG) {
+		filp->f_flags |= O_NOATIME;
+		filp->f_mode |= FMODE_NOCMTIME;
+	}
+	fsnotify_open(filp->f_path.dentry);
+	fd_install(fd, filp);
+	retval = fd;
+	goto out_mnt;
+
+out_err:
+	dput(dentry);
+out_mnt:
+	mntput(mnt);
+out_sb:
+	deactivate_super(sb);
+
+	return retval;
+}
+
+SYSCALL_DEFINE2(open_by_handle, struct file_handle __user *, handle,
+		int, flags)
+{
+	long ret;
+	struct file_handle f_handle;
+
+	if (force_o_largefile())
+		flags |= O_LARGEFILE;
+
+	if (copy_from_user(&f_handle, handle, sizeof(struct file_handle))) {
+		ret = -EFAULT;
+		goto err_out;
+	}
+	ret = do_sys_open_by_handle(&f_handle, flags);
+err_out:
+	/* avoid REGPARM breakage on x86: */
+	asmlinkage_protect(2, ret, handle, flags);
+	return ret;
+}
diff --git a/fs/pnode.c b/fs/pnode.c
index 5cc564a..9f6d12d 100644
--- a/fs/pnode.c
+++ b/fs/pnode.c
@@ -6,9 +6,9 @@
  *	Author : Ram Pai (linuxram@us.ibm.com)
  *
  */
+#include <linux/fs.h>
 #include <linux/mnt_namespace.h>
 #include <linux/mount.h>
-#include <linux/fs.h>
 #include "internal.h"
 #include "pnode.h"
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 157ed57..2116b00 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1935,6 +1935,7 @@ extern int filp_close(struct file *, fl_owner_t id);
 extern char * getname(const char __user *);
 extern long do_sys_name_to_handle(const char __user *name,
 				struct file_handle *fh);
+extern long do_sys_open_by_handle(struct file_handle *fh, int flags);
 
 /* fs/ioctl.c */
 
diff --git a/include/linux/mnt_namespace.h b/include/linux/mnt_namespace.h
index 0b89efc..d363ecc 100644
--- a/include/linux/mnt_namespace.h
+++ b/include/linux/mnt_namespace.h
@@ -36,6 +36,8 @@ extern const struct seq_operations mounts_op;
 extern const struct seq_operations mountinfo_op;
 extern const struct seq_operations mountstats_op;
 extern int mnt_had_events(struct proc_mounts *);
+extern struct vfsmount *fs_get_vfsmount(struct task_struct *task,
+					struct super_block *sb);
 
 #endif
 #endif
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 05b441d..a853aa0 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -4,6 +4,7 @@
 #include <linux/dcache.h>
 #include <linux/linkage.h>
 #include <linux/path.h>
+#include <asm-generic/fcntl.h>
 
 struct vfsmount;
 
@@ -96,4 +97,27 @@ static inline void nd_terminate_link(void *name, size_t len, size_t maxlen)
 	((char *) name)[min(len, maxlen)] = '\0';
 }
 
+/*
+ * Note that while the flag value (low two bits) for sys_open means:
+ *	00 - read-only
+ *	01 - write-only
+ *	10 - read-write
+ *	11 - special
+ * it is changed into
+ *	00 - no permissions needed
+ *	01 - read-permission
+ *	10 - write-permission
+ *	11 - read-write
+ * for the internal routines (ie open_namei()/follow_link() etc)
+ * This is more logical, and also allows the 00 "no perm needed"
+ * to be used for symlinks (where the permissions are checked
+ * later).
+ *
+*/
+static inline int open_to_namei_flags(int flag)
+{
+	if ((flag+1) & O_ACCMODE)
+		flag++;
+	return flag;
+}
 #endif /* _LINUX_NAMEI_H */
-- 
1.7.0.4.360.g11766c

[PATCH -V4 4/6] ext4: Add get_fsid callback

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/super.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e14d22c..b485f0a 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1049,6 +1049,14 @@ static int bdev_try_to_free_page(struct super_block *sb, struct page *page,
 	return try_to_free_buffers(page);
 }
 
+static struct uuid *ext4_get_fsid(struct super_block *sb)
+{
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
+	struct ext4_super_block *es = sbi->s_es;
+
+	return (struct uuid *)es->s_uuid;
+}
+
 #ifdef CONFIG_QUOTA
 #define QTYPE2NAME(t) ((t) == USRQUOTA ? "user" : "group")
 #define QTYPE2MOPT(on, t) ((t) == USRQUOTA?((on)##USRJQUOTA):((on)##GRPJQUOTA))
@@ -1109,6 +1117,7 @@ static const struct super_operations ext4_sops = {
 	.quota_write	= ext4_quota_write,
 #endif
 	.bdev_try_to_free_page = bdev_try_to_free_page,
+	.get_fsid	= ext4_get_fsid,
 };
 
 static const struct super_operations ext4_nojournal_sops = {
@@ -1128,6 +1137,7 @@ static const struct super_operations ext4_nojournal_sops = {
 	.quota_write	= ext4_quota_write,
 #endif
 	.bdev_try_to_free_page = bdev_try_to_free_page,
+	.get_fsid	= ext4_get_fsid,
 };
 
 static const struct export_operations ext4_export_ops = {
-- 
1.7.0.4.360.g11766c

[PATCH -V4 5/6] x86: Add new syscalls for x86_32

[Aneesh Kumar K.V]

This patch adds sys_name_to_handle and sys_open_by_handle
syscalls to x86_32

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/x86/include/asm/unistd_32.h   |    4 +++-
 arch/x86/kernel/syscall_table_32.S |    2 ++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/unistd_32.h b/arch/x86/include/asm/unistd_32.h
index beb9b5f..ce5b9c9 100644
--- a/arch/x86/include/asm/unistd_32.h
+++ b/arch/x86/include/asm/unistd_32.h
@@ -343,10 +343,12 @@
 #define __NR_rt_tgsigqueueinfo	335
 #define __NR_perf_event_open	336
 #define __NR_recvmmsg		337
+#define __NR_name_to_handle	338
+#define __NR_open_by_handle     339
 
 #ifdef __KERNEL__
 
-#define NR_syscalls 338
+#define NR_syscalls 340
 
 #define __ARCH_WANT_IPC_PARSE_VERSION
 #define __ARCH_WANT_OLD_READDIR
diff --git a/arch/x86/kernel/syscall_table_32.S b/arch/x86/kernel/syscall_table_32.S
index 8b37293..0340dba 100644
--- a/arch/x86/kernel/syscall_table_32.S
+++ b/arch/x86/kernel/syscall_table_32.S
@@ -337,3 +337,5 @@ ENTRY(sys_call_table)
 	.long sys_rt_tgsigqueueinfo	/* 335 */
 	.long sys_perf_event_open
 	.long sys_recvmmsg
+	.long sys_name_to_handle
+	.long sys_open_by_handle	/* 339 */
-- 
1.7.0.4.360.g11766c

[PATCH -V4 6/6] x86: Add new syscalls for x86_64

[Aneesh Kumar K.V]

Add sys_name_to_handle and sys_open_by_handle syscall for x86_64
Add necessary compat syscall support

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 arch/x86/ia32/ia32entry.S        |    2 +
 arch/x86/include/asm/unistd_64.h |    4 ++
 fs/compat.c                      |   72 ++++++++++++++++++++++++++++++++++++++
 include/linux/compat.h           |   14 +++++++
 4 files changed, 92 insertions(+), 0 deletions(-)

diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
index 59b4556..45cf8f0 100644
--- a/arch/x86/ia32/ia32entry.S
+++ b/arch/x86/ia32/ia32entry.S
@@ -842,4 +842,6 @@ ia32_sys_call_table:
 	.quad compat_sys_rt_tgsigqueueinfo	/* 335 */
 	.quad sys_perf_event_open
 	.quad compat_sys_recvmmsg
+	.quad compat_sys_name_to_handle
+	.quad compat_sys_open_by_handle		/* 339 */
 ia32_syscall_end:
diff --git a/arch/x86/include/asm/unistd_64.h b/arch/x86/include/asm/unistd_64.h
index ff4307b..99967d0 100644
--- a/arch/x86/include/asm/unistd_64.h
+++ b/arch/x86/include/asm/unistd_64.h
@@ -663,6 +663,10 @@ __SYSCALL(__NR_rt_tgsigqueueinfo, sys_rt_tgsigqueueinfo)
 __SYSCALL(__NR_perf_event_open, sys_perf_event_open)
 #define __NR_recvmmsg				299
 __SYSCALL(__NR_recvmmsg, sys_recvmmsg)
+#define __NR_name_to_handle			300
+__SYSCALL(__NR_name_to_handle, sys_name_to_handle)
+#define __NR_open_by_handle			301
+__SYSCALL(__NR_open_by_handle, sys_open_by_handle)
 
 #ifndef __NO_STUBS
 #define __ARCH_WANT_OLD_READDIR
diff --git a/fs/compat.c b/fs/compat.c
index 4b6ed03..5a03a5b 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -2310,3 +2310,75 @@ asmlinkage long compat_sys_timerfd_gettime(int ufd,
 }
 
 #endif /* CONFIG_TIMERFD */
+
+long get_compat_file_handle(struct file_handle *dst,
+			struct compat_file_handle __user *src)
+{
+	compat_uptr_t f_handle;
+
+	if (!access_ok(VERIFY_READ, src, sizeof(*src)) ||
+		__get_user(dst->handle_size, &src->handle_size) ||
+		__get_user(dst->handle_type, &src->handle_type) ||
+		copy_from_user(dst->fsid.uuid,
+			src->fsid.uuid, sizeof(dst->fsid.uuid)) ||
+		__get_user(f_handle, &src->f_handle)) {
+		return -EFAULT;
+	}
+	dst->f_handle = compat_ptr(f_handle);
+	return 0;
+}
+
+long put_compat_file_handle(struct compat_file_handle __user *dst,
+			struct file_handle *src)
+{
+	/*
+	 * dst->f_handle is copied from do_sys_name_to_handle
+	 */
+	return (!access_ok(VERIFY_WRITE, dst, sizeof(*dst)) ||
+		__put_user(src->handle_size, &dst->handle_size) ||
+		__put_user(src->handle_type, &dst->handle_type) ||
+		copy_to_user(dst->fsid.uuid,
+			src->fsid.uuid, sizeof(dst->fsid.uuid))) ? -EFAULT : 0;
+}
+
+
+asmlinkage long compat_sys_name_to_handle(const char __user *name,
+					struct compat_file_handle __user *uh)
+{
+	long ret1, ret2;
+	compat_uptr_t f_handle;
+	struct file_handle handle;
+
+	ret1 = (!access_ok(VERIFY_READ, uh, sizeof(*uh)) ||
+		__get_user(handle.handle_size, &uh->handle_size) ||
+		__get_user(f_handle, &uh->f_handle)) ? -EFAULT : 0;
+	if (ret1)
+		return ret1;
+
+	handle.f_handle = compat_ptr(f_handle);
+	ret1 = do_sys_name_to_handle(name, &handle);
+	if ((ret1 == -EOVERFLOW) || !ret1)
+		ret2 = put_compat_file_handle(uh, &handle) ? -EFAULT : 0;
+
+	if (ret1)
+		return ret1;
+	if (ret2)
+		return ret2;
+	return 0;
+}
+
+asmlinkage long compat_sys_open_by_handle(struct compat_file_handle __user *uh,
+					int flags)
+{
+	long error;
+	struct file_handle handle;
+
+	error = get_compat_file_handle(&handle, uh) ? -EFAULT : 0;
+	if (error)
+		return error;
+
+	/* we don't want to set the O_LARGEFILE flag */
+	error = do_sys_open_by_handle(&handle, flags);
+
+	return error;
+}
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 717c691..6d2fda1 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -12,6 +12,7 @@
 #include <linux/sem.h>
 #include <linux/socket.h>
 #include <linux/if.h>
+#include <linux/fs.h>
 
 #include <asm/compat.h>
 #include <asm/siginfo.h>
@@ -209,6 +210,15 @@ struct compat_robust_list_head {
 	compat_uptr_t			list_op_pending;
 };
 
+struct compat_file_handle {
+	int handle_size;
+	int handle_type;
+	/* File system identifier */
+	struct uuid fsid;
+	/* file identifier */
+	compat_uptr_t f_handle;
+};
+
 extern void compat_exit_robust_list(struct task_struct *curr);
 
 asmlinkage long
@@ -355,6 +365,10 @@ asmlinkage long compat_sys_newfstatat(unsigned int dfd, char __user * filename,
 				      int flag);
 asmlinkage long compat_sys_openat(unsigned int dfd, const char __user *filename,
 				  int flags, int mode);
+asmlinkage long compat_sys_name_to_handle(const char __user *name,
+					struct compat_file_handle __user *uh);
+asmlinkage long compat_sys_open_by_handle(struct compat_file_handle __user *uh,
+					int flags);
 
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */
-- 
1.7.0.4.360.g11766c

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

[Andreas Dilger]

On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> +long do_sys_name_to_handle(const char __user *name,
> +				struct file_handle *fh)
> +{
> +	/*
> +	 * name to handle conversion only done for regular files
> +	 * directories and symbolic links
> +	 */

Out of curiosity, why not do this for other types of files?  It makes sense from a consistency POV, so that the userspace code doesn't have to special-case each file by checking the type first (which would mean doing an extra path traversal and stat for each file first, adding overhead).

I'd always thought it would be cool to have a distributed filesystem where named pipes/sockets work across clients by using the IO transport of the filesystem, making single-system image environments a lot easier to implement.

> +	/* we ask for a non connected handle */
> +	retval = exportfs_encode_fh(path.dentry, (struct fid *)handle,
> +				&handle_size,  0);

Since there is virtually no overhead to do so, why not always return a connected handle?  This will allow the kernel to reconnect the looked-up dentries into the tree, instead of creating all disconnected dentries.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH -V4 6/6] x86: Add new syscalls for x86_64

[Andreas Dilger]

On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> Add necessary compat syscall support

Maybe I'm missing something, but why would we add a new syscall that immediately needs a compat interface?  I thought the kernel would simply return the packed file handle to userspace, instead of returning a pointer?

> +struct compat_file_handle {
> +	int handle_size;
> +	int handle_type;
> +	/* File system identifier */
> +	struct uuid fsid;
> +	/* file identifier */
> +	compat_uptr_t f_handle;
> +};

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH -V4 6/6] x86: Add new syscalls for x86_64

[Aneesh Kumar K. V]

On Fri, 23 Apr 2010 16:09:07 -0600, Andreas Dilger <adilger@sun.com> wrote:
> On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> > Add necessary compat syscall support
> 
> Maybe I'm missing something, but why would we add a new syscall that immediately needs a compat interface?  I thought the kernel would simply return the packed file handle to userspace, instead of returning a pointer?
> 
> > +struct compat_file_handle {
> > +	int handle_size;
> > +	int handle_type;
> > +	/* File system identifier */
> > +	struct uuid fsid;
> > +	/* file identifier */
> > +	compat_uptr_t f_handle;
> > +};
> 


I want to make sure we put uuid part of the fs as a separate member. I
guess we could avoid compat interface if we define the syscall as

open_by_handle(struct file_handle *handle, int size, int flags);

But the handle in the above case will not be usable if the file handle
returned is too big to be usable by a cluster file system because of the
UUID part of handle. Having uuid as a seperate member allows such userspace
to generate a smaller unique identifier for the filesystem and still use
the void *f_handle part of the struct file_handle.

With struct file_handle since we have void * usage i guess we would need
a compat interface.

-aneesh

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

[Christoph Hellwig]

On Fri, Apr 23, 2010 at 04:02:14PM -0600, Andreas Dilger wrote:
> Out of curiosity, why not do this for other types of files?  It makes sense from a consistency POV, so that the userspace code doesn't have to special-case each file by checking the type first (which would mean doing an extra path traversal and stat for each file first, adding overhead).

This limitation seems to be copied from the original XFS handle code,
but I don't see any good reason to stay with it.

Re: [PATCH -V4 6/6] x86: Add new syscalls for x86_64

[Christoph Hellwig]

On Sun, Apr 25, 2010 at 11:58:58PM +0530, Aneesh Kumar K. V wrote:
> On Fri, 23 Apr 2010 16:09:07 -0600, Andreas Dilger <adilger@sun.com> wrote:
> > On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> > > Add necessary compat syscall support
> > 
> > Maybe I'm missing something, but why would we add a new syscall that immediately needs a compat interface?  I thought the kernel would simply return the packed file handle to userspace, instead of returning a pointer?
> > 
> > > +struct compat_file_handle {
> > > +	int handle_size;
> > > +	int handle_type;
> > > +	/* File system identifier */
> > > +	struct uuid fsid;
> > > +	/* file identifier */
> > > +	compat_uptr_t f_handle;
> > > +};
> > 
> 
> 
> I want to make sure we put uuid part of the fs as a separate member. I
> guess we could avoid compat interface if we define the syscall as
> 
> open_by_handle(struct file_handle *handle, int size, int flags);
> 
> But the handle in the above case will not be usable if the file handle
> returned is too big to be usable by a cluster file system because of the
> UUID part of handle. Having uuid as a seperate member allows such userspace
> to generate a smaller unique identifier for the filesystem and still use
> the void *f_handle part of the struct file_handle.
> 
> With struct file_handle since we have void * usage i guess we would need
> a compat interface.

You can juse make the handle a

   char f_handle[0];

member and the structure variable size. 

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

[Aneesh Kumar K. V]

On Fri, 23 Apr 2010 16:02:14 -0600, Andreas Dilger <adilger@sun.com> wrote:
> On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> > +long do_sys_name_to_handle(const char __user *name,
> > +				struct file_handle *fh)
> > +{
> > +	/*
> > +	 * name to handle conversion only done for regular files
> > +	 * directories and symbolic links
> > +	 */
> 
> Out of curiosity, why not do this for other types of files?  It makes
> sense from a consistency POV, so that the userspace code doesn't have
> to special-case each file by checking the type first (which would mean
> doing an extra path traversal and stat for each file first, adding
> overhead).

The limitation directly came from the XFS ioctl. But I don't see a
reason why we should limit it for this syscall.  To better support
symlink and regular files i now have did sys_name_to_handle and
sys_lname_to_handle syscalls. I also did a freadlink syscall that will
return the symlink target name from the handle. 



> 
> I'd always thought it would be cool to have a distributed filesystem where named pipes/sockets work across clients by using the IO transport of the filesystem, making single-system image environments a lot easier to implement.
> 
> > +	/* we ask for a non connected handle */
> > +	retval = exportfs_encode_fh(path.dentry, (struct fid *)handle,
> > +				&handle_size,  0);
> 
> Since there is virtually no overhead to do so, why not always return a connected handle?  This will allow the kernel to reconnect the looked-up dentries into the tree, instead of creating all disconnected dentries.
> 

open by handle would return a connected dentry for directory. Since we
use a simple acceptable function that accept any dentry alias with
exportfs_decode_fh I was wondering whether we should return a connected
handle ? It would be simply increasing the size of the handle ?

-aneesh

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

[Aneesh Kumar K. V]

On Mon, 26 Apr 2010 05:52:08 -0400, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, Apr 23, 2010 at 04:02:14PM -0600, Andreas Dilger wrote:
> > Out of curiosity, why not do this for other types of files?  It makes sense from a consistency POV, so that the userspace code doesn't have to special-case each file by checking the type first (which would mean doing an extra path traversal and stat for each file first, adding overhead).
> 
> This limitation seems to be copied from the original XFS handle code,
> but I don't see any good reason to stay with it.
> 

The limitation came from the original XFS handle code. So to support
symlink and regular files better i added the below syscalls to the patch
set. sys_name_to_handle, sys_lname_to_handle and sys_freadlink. The last
to return the link target name from an fd. This enables to use handle
for symlink and then use the fd returned from open_by_handle to get the
target name. Let me know what you think

-aneesh

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

[J. Bruce Fields]

On Mon, Apr 26, 2010 at 03:42:45PM +0530, Aneesh Kumar K. V wrote:
> On Fri, 23 Apr 2010 16:02:14 -0600, Andreas Dilger <adilger@sun.com> wrote:
> > On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> > > +long do_sys_name_to_handle(const char __user *name,
> > > +				struct file_handle *fh)
> > > +{
> > > +	/*
> > > +	 * name to handle conversion only done for regular files
> > > +	 * directories and symbolic links
> > > +	 */
> > 
> > Out of curiosity, why not do this for other types of files?  It makes
> > sense from a consistency POV, so that the userspace code doesn't have
> > to special-case each file by checking the type first (which would mean
> > doing an extra path traversal and stat for each file first, adding
> > overhead).
> 
> The limitation directly came from the XFS ioctl. But I don't see a
> reason why we should limit it for this syscall.  To better support
> symlink and regular files i now have did sys_name_to_handle and
> sys_lname_to_handle syscalls. I also did a freadlink syscall that will
> return the symlink target name from the handle. 
> 
> 
> 
> > 
> > I'd always thought it would be cool to have a distributed filesystem where named pipes/sockets work across clients by using the IO transport of the filesystem, making single-system image environments a lot easier to implement.
> > 
> > > +	/* we ask for a non connected handle */
> > > +	retval = exportfs_encode_fh(path.dentry, (struct fid *)handle,
> > > +				&handle_size,  0);
> > 
> > Since there is virtually no overhead to do so, why not always return a connected handle?  This will allow the kernel to reconnect the looked-up dentries into the tree, instead of creating all disconnected dentries.
> > 
> 
> open by handle would return a connected dentry for directory. Since we
> use a simple acceptable function that accept any dentry alias with
> exportfs_decode_fh I was wondering whether we should return a connected
> handle ? It would be simply increasing the size of the handle ?

Well, and it means the filehandle changes when you rename the file to a
different directory.  Do you really want that?

--b.

Re: [PATCH -V4 6/6] x86: Add new syscalls for x86_64

[Andreas Dilger]

On 2010-04-26, at 03:53, Christoph Hellwig wrote:
> On Sun, Apr 25, 2010 at 11:58:58PM +0530, Aneesh Kumar K. V wrote:
>> On Fri, 23 Apr 2010 16:09:07 -0600, Andreas Dilger <adilger@sun.com> wrote:
>>> On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
>>>> Add necessary compat syscall support
>>> 
>>> Maybe I'm missing something, but why would we add a new syscall that immediately needs a compat interface?  I thought the kernel would simply return the packed file handle to userspace, instead of returning a pointer?
>>> 
>>>> +struct compat_file_handle {
>>>> +	int handle_size;
>>>> +	int handle_type;
>>>> +	/* File system identifier */
>>>> +	struct uuid fsid;
>>>> +	/* file identifier */
>>>> +	compat_uptr_t f_handle;
>>>> +};
>> 
>> 
>> I want to make sure we put uuid part of the fs as a separate member. I
>> guess we could avoid compat interface if we define the syscall as
>> 
>> open_by_handle(struct file_handle *handle, int size, int flags);
>> 
>> But the handle in the above case will not be usable if the file handle
>> returned is too big to be usable by a cluster file system because of the
>> UUID part of handle. Having uuid as a seperate member allows such userspace
>> to generate a smaller unique identifier for the filesystem and still use
>> the void *f_handle part of the struct file_handle.
>> 
>> With struct file_handle since we have void * usage i guess we would need
>> a compat interface.
> 
> You can juse make the handle a
> 
>   char f_handle[0];
> 
> member and the structure variable size. 

Exactly.  The filesystem knows what the total size is including the UUID (in fact it would be better to allow the UUIDs to be variable-sized also), and userspace should be able to treat this as an opaque blob, beyond the the size.

By having userspace "decode" the file handle in order to determine the handle size, copy the pointer data, etc. then you introduce much more complexity into the whole process.

It makes me wonder if we should have some sort of "handle version" in order to allow the kernel to verify what it is being passed, in case we want to change the interface in the future.


Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

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

[Aneesh Kumar K. V]

On Mon, 26 Apr 2010 13:57:52 -0400, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Mon, Apr 26, 2010 at 03:42:45PM +0530, Aneesh Kumar K. V wrote:
> > On Fri, 23 Apr 2010 16:02:14 -0600, Andreas Dilger <adilger@sun.com> wrote:
> > > On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> > > > +long do_sys_name_to_handle(const char __user *name,
> > > > +				struct file_handle *fh)
> > > > +{
> > > > +	/*
> > > > +	 * name to handle conversion only done for regular files
> > > > +	 * directories and symbolic links
> > > > +	 */
> > > 
> > > Out of curiosity, why not do this for other types of files?  It makes
> > > sense from a consistency POV, so that the userspace code doesn't have
> > > to special-case each file by checking the type first (which would mean
> > > doing an extra path traversal and stat for each file first, adding
> > > overhead).
> > 
> > The limitation directly came from the XFS ioctl. But I don't see a
> > reason why we should limit it for this syscall.  To better support
> > symlink and regular files i now have did sys_name_to_handle and
> > sys_lname_to_handle syscalls. I also did a freadlink syscall that will
> > return the symlink target name from the handle. 
> > 
> > 
> > 
> > > 
> > > I'd always thought it would be cool to have a distributed filesystem where named pipes/sockets work across clients by using the IO transport of the filesystem, making single-system image environments a lot easier to implement.
> > > 
> > > > +	/* we ask for a non connected handle */
> > > > +	retval = exportfs_encode_fh(path.dentry, (struct fid *)handle,
> > > > +				&handle_size,  0);
> > > 
> > > Since there is virtually no overhead to do so, why not always return a connected handle?  This will allow the kernel to reconnect the looked-up dentries into the tree, instead of creating all disconnected dentries.
> > > 
> > 
> > open by handle would return a connected dentry for directory. Since we
> > use a simple acceptable function that accept any dentry alias with
> > exportfs_decode_fh I was wondering whether we should return a connected
> > handle ? It would be simply increasing the size of the handle ?
> 
> Well, and it means the filehandle changes when you rename the file to a
> different directory.  Do you really want that?
> 

Atleast the usecase i am looking to we should retain the same handle
even after rename. So i guess getting a non connected handle is the
right thing

-aneesh

Re: [PATCH -V4 6/6] x86: Add new syscalls for x86_64

[Aneesh Kumar K. V]

On Mon, 26 Apr 2010 05:53:11 -0400, Christoph Hellwig <hch@infradead.org> wrote:
> On Sun, Apr 25, 2010 at 11:58:58PM +0530, Aneesh Kumar K. V wrote:
> > On Fri, 23 Apr 2010 16:09:07 -0600, Andreas Dilger <adilger@sun.com> wrote:
> > > On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> > > > Add necessary compat syscall support
> > > 
> > > Maybe I'm missing something, but why would we add a new syscall that immediately needs a compat interface?  I thought the kernel would simply return the packed file handle to userspace, instead of returning a pointer?
> > > 
> > > > +struct compat_file_handle {
> > > > +	int handle_size;
> > > > +	int handle_type;
> > > > +	/* File system identifier */
> > > > +	struct uuid fsid;
> > > > +	/* file identifier */
> > > > +	compat_uptr_t f_handle;
> > > > +};
> > > 
> > 
> > 
> > I want to make sure we put uuid part of the fs as a separate member. I
> > guess we could avoid compat interface if we define the syscall as
> > 
> > open_by_handle(struct file_handle *handle, int size, int flags);
> > 
> > But the handle in the above case will not be usable if the file handle
> > returned is too big to be usable by a cluster file system because of the
> > UUID part of handle. Having uuid as a seperate member allows such userspace
> > to generate a smaller unique identifier for the filesystem and still use
> > the void *f_handle part of the struct file_handle.
> > 
> > With struct file_handle since we have void * usage i guess we would need
> > a compat interface.
> 
> You can juse make the handle a
> 
>    char f_handle[0];
> 
> member and the structure variable size. 
> 

Even with the above change i guess we would need a
compat_sys_open_by_handle to make sure we don't pass O_LARGEFILE 
similar to what is done in compat_sys_open


-aneesh

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

[J. Bruce Fields]

On Tue, Apr 27, 2010 at 11:43:44AM +0530, Aneesh Kumar K. V wrote:
> On Mon, 26 Apr 2010 13:57:52 -0400, "J. Bruce Fields" <bfields@fieldses.org> wrote:
> > On Mon, Apr 26, 2010 at 03:42:45PM +0530, Aneesh Kumar K. V wrote:
> > > On Fri, 23 Apr 2010 16:02:14 -0600, Andreas Dilger <adilger@sun.com> wrote:
> > > > On 2010-04-23, at 05:38, Aneesh Kumar K.V wrote:
> > > > > +long do_sys_name_to_handle(const char __user *name,
> > > > > +				struct file_handle *fh)
> > > > > +{
> > > > > +	/*
> > > > > +	 * name to handle conversion only done for regular files
> > > > > +	 * directories and symbolic links
> > > > > +	 */
> > > > 
> > > > Out of curiosity, why not do this for other types of files?  It makes
> > > > sense from a consistency POV, so that the userspace code doesn't have
> > > > to special-case each file by checking the type first (which would mean
> > > > doing an extra path traversal and stat for each file first, adding
> > > > overhead).
> > > 
> > > The limitation directly came from the XFS ioctl. But I don't see a
> > > reason why we should limit it for this syscall.  To better support
> > > symlink and regular files i now have did sys_name_to_handle and
> > > sys_lname_to_handle syscalls. I also did a freadlink syscall that will
> > > return the symlink target name from the handle. 
> > > 
> > > 
> > > 
> > > > 
> > > > I'd always thought it would be cool to have a distributed filesystem where named pipes/sockets work across clients by using the IO transport of the filesystem, making single-system image environments a lot easier to implement.
> > > > 
> > > > > +	/* we ask for a non connected handle */
> > > > > +	retval = exportfs_encode_fh(path.dentry, (struct fid *)handle,
> > > > > +				&handle_size,  0);
> > > > 
> > > > Since there is virtually no overhead to do so, why not always return a connected handle?  This will allow the kernel to reconnect the looked-up dentries into the tree, instead of creating all disconnected dentries.
> > > > 
> > > 
> > > open by handle would return a connected dentry for directory. Since we
> > > use a simple acceptable function that accept any dentry alias with
> > > exportfs_decode_fh I was wondering whether we should return a connected
> > > handle ? It would be simply increasing the size of the handle ?
> > 
> > Well, and it means the filehandle changes when you rename the file to a
> > different directory.  Do you really want that?
> > 
> 
> Atleast the usecase i am looking to we should retain the same handle
> even after rename. So i guess getting a non connected handle is the
> right thing

I think so.  And if somebody really had to have the other behavior, I
think it could probably be implented in userspace.

--b.