diff mbox

[RFC,07/11] 9p: Stop using the generic xattr_handler infrastructure

Message ID 1440094798-1411-8-git-send-email-agruenba@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Grünbacher Aug. 20, 2015, 6:19 p.m. UTC 
Of all the file systems that use the generic xattr handler infrastructure, 9p
is the only one that needs dentries inside the xattr file system code.  Open
code the xattr handling code in 9p so that we can then convert the generic code
to pass down inodes instead of dentries; this actually takes only little
additional code.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/9p/acl.c       | 12 ++++++------
 fs/9p/vfs_super.c |  5 ++---
 fs/9p/xattr.c     | 46 ++++++++++++++++++++++++++++++++++++++--------
 fs/9p/xattr.h     | 15 ++++++++++++---
 4 files changed, 58 insertions(+), 20 deletions(-)

Comments

Christoph Hellwig Aug. 21, 2015, 6:46 a.m. UTC | #1 
On Thu, Aug 20, 2015 at 08:19:54PM +0200, Andreas Gruenbacher wrote:
> Of all the file systems that use the generic xattr handler infrastructure, 9p
> is the only one that needs dentries inside the xattr file system code.  Open
> code the xattr handling code in 9p so that we can then convert the generic code
> to pass down inodes instead of dentries; this actually takes only little
> additional code.

NAK.  The rest of the series looks somewhat sensible but this is wrong.

We need want to converge to one interface, probably more similar to the
handlers, so we need to decided if we want to pass dentries or inodes to
both of them.

Personally I'm in favor of inodes, and that's how it used to be, but
that makes life more complicated for path based file systems like cifs
or 9p.
--
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
Steven Whitehouse Aug. 21, 2015, 8:35 a.m. UTC | #2 
Hi,

On 21/08/15 07:46, Christoph Hellwig wrote:
> On Thu, Aug 20, 2015 at 08:19:54PM +0200, Andreas Gruenbacher wrote:
>> Of all the file systems that use the generic xattr handler infrastructure, 9p
>> is the only one that needs dentries inside the xattr file system code.  Open
>> code the xattr handling code in 9p so that we can then convert the generic code
>> to pass down inodes instead of dentries; this actually takes only little
>> additional code.
> NAK.  The rest of the series looks somewhat sensible but this is wrong.
>
> We need want to converge to one interface, probably more similar to the
> handlers, so we need to decided if we want to pass dentries or inodes to
> both of them.
>
> Personally I'm in favor of inodes, and that's how it used to be, but
> that makes life more complicated for path based file systems like cifs
> or 9p.
>

We did look at this in detail some time ago, and Al came to the 
conclusion that 9p and CIFS both required the dentries and that it would 
not be possible to pass just an inode. In the SELinux refresh case, the 
dentry is not always available, and I don't think we have any choice 
about that.

I know it is a bit of a mess, so if there is a better way to do it, then 
that would be good. At the moment though, it is not clear quite how we 
could get around this issue,

Steve.

--
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
diff mbox

Patch

diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index 5325304..d6a02e0 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -212,9 +212,9 @@  int v9fs_acl_mode(struct inode *dir, umode_t *modep,
 	return 0;
 }
 
-static int v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
-			      void *buffer, size_t size,
-			      const struct xattr_handler *handler)
+static ssize_t v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
+				  void *buffer, size_t size,
+				  const struct v9fs_xattr_handler *handler)
 {
 	struct v9fs_session_info *v9ses;
 	struct posix_acl *acl;
@@ -243,7 +243,7 @@  static int v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
 
 static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name,
 			      const void *value, size_t size,
-			      int flags, const struct xattr_handler *handler)
+			      int flags, const struct v9fs_xattr_handler *handler)
 {
 	int retval;
 	struct posix_acl *acl;
@@ -327,14 +327,14 @@  err_out:
 	return retval;
 }
 
-const struct xattr_handler v9fs_xattr_acl_access_handler = {
+const struct v9fs_xattr_handler v9fs_xattr_acl_access_handler = {
 	.prefix	= POSIX_ACL_XATTR_ACCESS,
 	.flags	= ACL_TYPE_ACCESS,
 	.get	= v9fs_xattr_get_acl,
 	.set	= v9fs_xattr_set_acl,
 };
 
-const struct xattr_handler v9fs_xattr_acl_default_handler = {
+const struct v9fs_xattr_handler v9fs_xattr_acl_default_handler = {
 	.prefix	= POSIX_ACL_XATTR_DEFAULT,
 	.flags	= ACL_TYPE_DEFAULT,
 	.get	= v9fs_xattr_get_acl,
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index bf495ce..7914544 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -80,10 +80,9 @@  v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
 	sb->s_blocksize_bits = fls(v9ses->maxdata - 1);
 	sb->s_blocksize = 1 << sb->s_blocksize_bits;
 	sb->s_magic = V9FS_MAGIC;
-	if (v9fs_proto_dotl(v9ses)) {
+	if (v9fs_proto_dotl(v9ses))
 		sb->s_op = &v9fs_super_ops_dotl;
-		sb->s_xattr = v9fs_xattr_handlers;
-	} else
+	else
 		sb->s_op = &v9fs_super_ops;
 	sb->s_bdi = &v9ses->bdi;
 	if (v9ses->cache)
diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index 00b28f4..64ffd00 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -132,14 +132,44 @@  int v9fs_fid_xattr_set(struct p9_fid *fid, const char *name,
 	return retval;
 }
 
+ssize_t v9fs_getxattr(struct dentry *dentry, const char *name, void *buffer, size_t size)
+{
+	const struct v9fs_xattr_handler **handlers = v9fs_xattr_handlers;
+
+	for (; *handlers != NULL; handlers++) {
+		const struct v9fs_xattr_handler *handler = *handlers;
+		const char *suffix = strcmp_prefix(name, handler->prefix);
+
+		if (suffix)
+			return handler->get(dentry, suffix, buffer, size, handler);
+	}
+	return -EOPNOTSUPP;
+}
+
+int v9fs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		  size_t size, int flags)
+{
+	const struct v9fs_xattr_handler **handlers = v9fs_xattr_handlers;
+
+	for (; *handlers != NULL; handlers++) {
+		const struct v9fs_xattr_handler *handler = *handlers;
+		const char *suffix = strcmp_prefix(name, handler->prefix);
+
+		if (suffix)
+			return handler->set(dentry, name, value, size, flags,
+					    handler);
+	}
+	return -EOPNOTSUPP;
+}
+
 ssize_t v9fs_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 {
 	return v9fs_xattr_get(dentry, NULL, buffer, buffer_size);
 }
 
-static int v9fs_xattr_handler_get(struct dentry *dentry, const char *name,
-				  void *buffer, size_t size,
-				  const struct xattr_handler *handler)
+static ssize_t v9fs_xattr_handler_get(struct dentry *dentry, const char *name,
+				      void *buffer, size_t size,
+				      const struct v9fs_xattr_handler *handler)
 {
 	int prefix_len = strlen(handler->prefix);
 
@@ -150,7 +180,7 @@  static int v9fs_xattr_handler_get(struct dentry *dentry, const char *name,
 
 static int v9fs_xattr_handler_set(struct dentry *dentry, const char *name,
 				  const void *value, size_t size, int flags,
-				  const struct xattr_handler *handler)
+				  const struct v9fs_xattr_handler *handler)
 {
 	int prefix_len = strlen(handler->prefix);
 
@@ -159,27 +189,27 @@  static int v9fs_xattr_handler_set(struct dentry *dentry, const char *name,
 	return v9fs_xattr_set(dentry, name - prefix_len, value, size, flags);
 }
 
-static struct xattr_handler v9fs_xattr_user_handler = {
+static struct v9fs_xattr_handler v9fs_xattr_user_handler = {
 	.prefix	= XATTR_USER_PREFIX,
 	.get	= v9fs_xattr_handler_get,
 	.set	= v9fs_xattr_handler_set,
 };
 
-static struct xattr_handler v9fs_xattr_trusted_handler = {
+static struct v9fs_xattr_handler v9fs_xattr_trusted_handler = {
 	.prefix	= XATTR_TRUSTED_PREFIX,
 	.get	= v9fs_xattr_handler_get,
 	.set	= v9fs_xattr_handler_set,
 };
 
 #ifdef CONFIG_9P_FS_SECURITY
-static struct xattr_handler v9fs_xattr_security_handler = {
+static struct v9fs_xattr_handler v9fs_xattr_security_handler = {
 	.prefix	= XATTR_SECURITY_PREFIX,
 	.get	= v9fs_xattr_handler_get,
 	.set	= v9fs_xattr_handler_set,
 };
 #endif
 
-const struct xattr_handler *v9fs_xattr_handlers[] = {
+const struct v9fs_xattr_handler *v9fs_xattr_handlers[] = {
 	&v9fs_xattr_user_handler,
 	&v9fs_xattr_trusted_handler,
 #ifdef CONFIG_9P_FS_POSIX_ACL
diff --git a/fs/9p/xattr.h b/fs/9p/xattr.h
index c63c3be..488ad6d 100644
--- a/fs/9p/xattr.h
+++ b/fs/9p/xattr.h
@@ -18,9 +18,18 @@ 
 #include <net/9p/9p.h>
 #include <net/9p/client.h>
 
-extern const struct xattr_handler *v9fs_xattr_handlers[];
-extern const struct xattr_handler v9fs_xattr_acl_access_handler;
-extern const struct xattr_handler v9fs_xattr_acl_default_handler;
+struct v9fs_xattr_handler {
+	const char *prefix;
+	int flags;
+	ssize_t (*get)(struct dentry *, const char *, void *, size_t,
+		       const struct v9fs_xattr_handler *);
+	int (*set)(struct dentry *, const char *, const void *, size_t, int,
+		   const struct v9fs_xattr_handler *);
+};
+
+extern const struct v9fs_xattr_handler *v9fs_xattr_handlers[];
+extern const struct v9fs_xattr_handler v9fs_xattr_acl_access_handler;
+extern const struct v9fs_xattr_handler v9fs_xattr_acl_default_handler;
 
 extern ssize_t v9fs_fid_xattr_get(struct p9_fid *, const char *,
 				  void *, size_t);