[patch 3/5] VFS: pass open file to -xattr()
From: Miklos Szeredi [EMAIL PROTECTED]
Pass the open file into the filesystem's *xattr() methods.
This is needed to be able to correctly implement open-unlink-f*xattr
semantics, without having to resort to silly-renaming.
Do this by adding a 'struct file *' parameter to i_op-*xattr(). For
f... variants pass the open file pointer, in other cases pass NULL.
This is safe from a compatibility standpoint, out-of-tree old stuff
will continue to work, but will get a warning at compile time.
Signed-off-by: Miklos Szeredi [EMAIL PROTECTED]
---
Index: linux/fs/bad_inode.c
===
--- linux.orig/fs/bad_inode.c 2007-09-21 13:45:07.0 +0200
+++ linux/fs/bad_inode.c2007-09-21 13:45:11.0 +0200
@@ -261,24 +261,25 @@ static int bad_inode_setattr(struct dent
}
static int bad_inode_setxattr(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags)
+ const void *value, size_t size, int flags, struct file *file)
{
return -EIO;
}
static ssize_t bad_inode_getxattr(struct dentry *dentry, const char *name,
- void *buffer, size_t size)
+ void *buffer, size_t size, struct file *file)
{
return -EIO;
}
static ssize_t bad_inode_listxattr(struct dentry *dentry, char *buffer,
- size_t buffer_size)
+ size_t buffer_size, struct file *file)
{
return -EIO;
}
-static int bad_inode_removexattr(struct dentry *dentry, const char *name)
+static int bad_inode_removexattr(struct dentry *dentry, const char *name,
+struct file *file)
{
return -EIO;
}
Index: linux/fs/ext2/xattr.c
===
--- linux.orig/fs/ext2/xattr.c 2007-09-21 13:44:54.0 +0200
+++ linux/fs/ext2/xattr.c 2007-09-21 13:45:11.0 +0200
@@ -328,7 +328,8 @@ cleanup:
* dentry-d_inode-i_mutex: don't care
*/
ssize_t
-ext2_listxattr(struct dentry *dentry, char *buffer, size_t size)
+ext2_listxattr(struct dentry *dentry, char *buffer, size_t size,
+ struct file *file)
{
return ext2_xattr_list(dentry-d_inode, buffer, size);
}
Index: linux/fs/ext2/xattr.h
===
--- linux.orig/fs/ext2/xattr.h 2007-09-21 13:44:54.0 +0200
+++ linux/fs/ext2/xattr.h 2007-09-21 13:45:11.0 +0200
@@ -61,7 +61,7 @@ extern struct xattr_handler ext2_xattr_a
extern struct xattr_handler ext2_xattr_acl_default_handler;
extern struct xattr_handler ext2_xattr_security_handler;
-extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
+extern ssize_t ext2_listxattr(struct dentry *, char *, size_t, struct file *);
extern int ext2_xattr_get(struct inode *, int, const char *, void *, size_t);
extern int ext2_xattr_set(struct inode *, int, const char *, const void *,
size_t, int);
Index: linux/fs/ext3/xattr.c
===
--- linux.orig/fs/ext3/xattr.c 2007-09-21 13:44:54.0 +0200
+++ linux/fs/ext3/xattr.c 2007-09-21 13:45:11.0 +0200
@@ -143,7 +143,8 @@ ext3_xattr_handler(int name_index)
* dentry-d_inode-i_mutex: don't care
*/
ssize_t
-ext3_listxattr(struct dentry *dentry, char *buffer, size_t size)
+ext3_listxattr(struct dentry *dentry, char *buffer, size_t size,
+ struct file *file)
{
return ext3_xattr_list(dentry-d_inode, buffer, size);
}
Index: linux/fs/ext3/xattr.h
===
--- linux.orig/fs/ext3/xattr.h 2007-09-21 13:44:54.0 +0200
+++ linux/fs/ext3/xattr.h 2007-09-21 13:45:11.0 +0200
@@ -64,7 +64,7 @@ extern struct xattr_handler ext3_xattr_a
extern struct xattr_handler ext3_xattr_acl_default_handler;
extern struct xattr_handler ext3_xattr_security_handler;
-extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
+extern ssize_t ext3_listxattr(struct dentry *, char *, size_t, struct file *);
extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
extern int ext3_xattr_list(struct inode *, char *, size_t);
Index: linux/fs/fuse/dir.c
===
--- linux.orig/fs/fuse/dir.c2007-09-21 13:45:07.0 +0200
+++ linux/fs/fuse/dir.c 2007-09-21 13:45:11.0 +0200
@@ -1118,7 +1118,8 @@ static int fuse_getattr(struct vfsmount
}
static int fuse_setxattr(struct dentry *entry, const char *name,
-const void *value, size_t size, int flags)
+const void *value, size_t size, int flags,
+struct file *file)
{
struct inode *inode = entry-d_inode;
struct fuse_conn *fc = get_fuse_conn(inode);
@@ -1156,7 +1157,7 @@ static int fuse_setxattr(struct
Re: [patch 3/5] VFS: pass open file to -xattr()
On Fri, Sep 21, 2007 at 02:23:46PM +0200, Miklos Szeredi wrote: From: Miklos Szeredi [EMAIL PROTECTED] Pass the open file into the filesystem's *xattr() methods. This is needed to be able to correctly implement open-unlink-f*xattr semantics, without having to resort to silly-renaming. Do this by adding a 'struct file *' parameter to i_op-*xattr(). For f... variants pass the open file pointer, in other cases pass NULL. This is safe from a compatibility standpoint, out-of-tree old stuff will continue to work, but will get a warning at compile time. NACK, no more optional arguments, and passing file structs to xattr stuff is silly. If your filesystem doesn't get open but unliked right you will have to resort to silly renaming, I'm sorry. Same argument applies to all pass file down patches in the series, I won't comment on the separately. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
I don't think it's silly. Read/write get passed the file descriptor, and it makes a lot of sense, if the filesystem has stateful opens. Similarly for any fs operation that gets a file descriptor, it makes sense to pass the relevant open file down into the filesystem. read/write fundamentally operate on file descriptors. None of these operations does, rather their normal forms get a path name and special forms operate on a file descriptor to avoid lookup races. Still the underlying operation has nothing to do with the file descriptor at all. I don't see why read/write are special, normal filesystems don't need the file descriptor in that case either. And other in BSD's, OS X, the read/write fs ops don't get the open file, and their fuse implementations have to hack around this. The exact same thing is true for the other operations on file descriptors, just in those cases Linux does the same as the other OS's. If you look carefully, the ftrunacate() already does this, becuse without that it's impossible to implement correct semantics in the filesystem in some cases. ftruncate is a special case due to O_TRUNC. No, it's special, because it does not do permission checking, while truncate() does. For other operations it's not impossible, but it would mean more hacks in the filesystem itself (such as sillyrenaming) that are entirely unneeded if the file info is available. It's not a problem at all for filesystem that implement normal unix semantics. If you want to shoer-horn strange semantics that barely fit, you'll need some more hacks. What about sshfs. Basically it uses the sftp protocol, which more or less implements the UNIX API in a remote protocol. So it has OPEN, READ, WRITE, CLOSE, STAT, FSTAT, etc. operation. Now why is FSTAT different than READ or WRITE? Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
On Fri, Sep 21, 2007 at 02:23:46PM +0200, Miklos Szeredi wrote: From: Miklos Szeredi [EMAIL PROTECTED] Pass the open file into the filesystem's *xattr() methods. This is needed to be able to correctly implement open-unlink-f*xattr semantics, without having to resort to silly-renaming. Do this by adding a 'struct file *' parameter to i_op-*xattr(). For f... variants pass the open file pointer, in other cases pass NULL. This is safe from a compatibility standpoint, out-of-tree old stuff will continue to work, but will get a warning at compile time. NACK, no more optional arguments, and passing file structs to xattr stuff is silly. If your filesystem doesn't get open but unliked right you will have to resort to silly renaming, I'm sorry. Same argument applies to all pass file down patches in the series, I won't comment on the separately. I don't think it's silly. Read/write get passed the file descriptor, and it makes a lot of sense, if the filesystem has stateful opens. Similarly for any fs operation that gets a file descriptor, it makes sense to pass the relevant open file down into the filesystem. If you look carefully, the ftrunacate() already does this, becuse without that it's impossible to implement correct semantics in the filesystem in some cases. For other operations it's not impossible, but it would mean more hacks in the filesystem itself (such as sillyrenaming) that are entirely unneeded if the file info is available. I agree, that the xattr API is quite ugly already, but adding one more argument won't make it all that much worse. Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
On Fri, Sep 21, 2007 at 03:00:06PM +0200, Miklos Szeredi wrote: I don't think it's silly. Read/write get passed the file descriptor, and it makes a lot of sense, if the filesystem has stateful opens. Similarly for any fs operation that gets a file descriptor, it makes sense to pass the relevant open file down into the filesystem. read/write fundamentally operate on file descriptors. None of these operations does, rather their normal forms get a path name and special forms operate on a file descriptor to avoid lookup races. Still the underlying operation has nothing to do with the file descriptor at all. If you look carefully, the ftrunacate() already does this, becuse without that it's impossible to implement correct semantics in the filesystem in some cases. ftruncate is a special case due to O_TRUNC. But I have plans to solve this whole issue more elegant than the current hack. For other operations it's not impossible, but it would mean more hacks in the filesystem itself (such as sillyrenaming) that are entirely unneeded if the file info is available. It's not a problem at all for filesystem that implement normal unix semantics. If you want to shoer-horn strange semantics that barely fit, you'll need some more hacks. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
On Fri, 2007-09-21 at 15:16 +0200, Miklos Szeredi wrote: ftruncate is a special case due to O_TRUNC. No, it's special, because it does not do permission checking, while truncate() does. So why not just add file-f_op-ftruncate() and file-f_op-fstat()? Most filesystems can trivially redirect these to do_truncate() and their existing getattr() method. Those, like FUSE, that care can use the hook. In fact, I think that NFSv4 could also benefit from an ftruncate(): currently we have to hunt around for an open file context for that particular case. Trond - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
On Fri, Sep 21, 2007 at 10:32:31AM -0400, Trond Myklebust wrote: On Fri, 2007-09-21 at 15:16 +0200, Miklos Szeredi wrote: ftruncate is a special case due to O_TRUNC. No, it's special, because it does not do permission checking, while truncate() does. So why not just add file-f_op-ftruncate() and file-f_op-fstat()? Most filesystems can trivially redirect these to do_truncate() and their existing getattr() method. Those, like FUSE, that care can use the hook. In fact, I think that NFSv4 could also benefit from an ftruncate(): currently we have to hunt around for an open file context for that particular case. Havin the file for fruncate is fine and I'm planning to do something along those lines. Having it for (f)stat is dumb because the operation is in no way related to the open file descriptor. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
On Fri, Sep 21, 2007 at 10:32:31AM -0400, Trond Myklebust wrote: On Fri, 2007-09-21 at 15:16 +0200, Miklos Szeredi wrote: ftruncate is a special case due to O_TRUNC. No, it's special, because it does not do permission checking, while truncate() does. So why not just add file-f_op-ftruncate() and file-f_op-fstat()? Most filesystems can trivially redirect these to do_truncate() and their existing getattr() method. Those, like FUSE, that care can use the hook. In fact, I think that NFSv4 could also benefit from an ftruncate(): currently we have to hunt around for an open file context for that particular case. Havin the file for fruncate is fine and I'm planning to do something along those lines. Having it for (f)stat is dumb because the operation is in no way related to the open file descriptor. What I'm saying is that read and write are _no_more_ related to the file than fstat. Read/write operate on inode data, fstat operates on inode metadata. OK, read/write have a position state in the open file, but that is something the filesystem should _never_ touch anyway, so it's irrelevant to the discussion. The fact is, if the filesystem uses a stateful open API, which defines an fstat() operation, it can be useful to use that instead of the plain stat(). But that is only possible if the VFS supplies the open file, otherwise there will be just hunting around for suitable open files, or sillyrenaming. None of which is desirable. Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
On Sep 21, 2007 16:59 +0200, Miklos Szeredi wrote: What I'm saying is that read and write are _no_more_ related to the file than fstat. Read/write operate on inode data, fstat operates on inode metadata. The read and write operations are DEFINITELY related to the file descriptor because of f_pos. Each process opening the same file can have a different f_pos so read/write will work in different locations of the file. In contrast getattr and getxattr operate on the single inode and you don't get e.g. a different i_size or i_uid or i_gid depending on who opened a file, nor is the xattr different. Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
On Sep 21, 2007 14:23 +0200, Miklos Szeredi wrote:
@@ -1214,10 +1214,12 @@ struct inode_operations {
+ int (*setxattr) (struct dentry *, const char *,const void *,size_t,int,
+ struct file *);
+ ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t,
+ struct file *);
+ ssize_t (*listxattr) (struct dentry *, char *, size_t, struct file *);
+ int (*removexattr) (struct dentry *, const char *, struct file *);
Likewise - these are no longer inode operations if you need a file.
Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/5] VFS: pass open file to -xattr()
What I'm saying is that read and write are _no_more_ related to the file than fstat. Read/write operate on inode data, fstat operates on inode metadata. The read and write operations are DEFINITELY related to the file descriptor because of f_pos. Each process opening the same file can have a different f_pos so read/write will work in different locations of the file. Ah yes, but f_pos is handled entirely within the VFS. The filesystem has nothing to do with f_pos, and the read/write methods are passed the offset explicitly. So with that the read/write calls (for regular files at least) really are not that different from fstat. In contrast getattr and getxattr operate on the single inode and you don't get e.g. a different i_size or i_uid or i_gid depending on who opened a file, nor is the xattr different. You won't get different data either (again for regular files). Yet passing file operations down to the filesystem implementation makes sense even for regular files, even if for most filesystems we could get away with a totally stateless read/write model (as some other OS's apparently do). What I'm arguing, is that if we pass the open file for read/write to the filesystem for regular files, it makes _equally_ as much sense to pass the open file for getattr/setattr/xattr operations. Miklos - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
