xfs
[Top] [All Lists]

Re: [RFC][PATCH] VFS interface for extended attributes

To: Alexander Viro <viro@xxxxxxxxxxxx>, Andi Kleen <ak@xxxxxxx>
Subject: Re: [RFC][PATCH] VFS interface for extended attributes
From: Nathan Scott <nathans@xxxxxxx>
Date: Thu, 15 Nov 2001 16:08:53 +1100
Cc: Andreas Gruenbacher <ag@xxxxxxxxxxx>, Linus Torvalds <torvalds@xxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-xfs@xxxxxxxxxxx
In-reply-to: <20011113062711.A1912@wotan.suse.de>; from ak@suse.de on Tue, Nov 13, 2001 at 06:27:11AM +0100
References: <Pine.LNX.4.21.0111121152410.14344-100000@moses.parsec.at> <Pine.GSO.4.21.0111121207530.21825-100000@weyl.math.psu.edu> <20011113062711.A1912@wotan.suse.de>
Sender: owner-linux-xfs@xxxxxxxxxxx
User-agent: Mutt/1.2.5i
On Tue, Nov 13, 2001 at 06:27:11AM +0100, Andi Kleen wrote:
> On Mon, Nov 12, 2001 at 07:32:18PM -0500, Alexander Viro wrote:
> > Which means that converting permission() to vfsmount/dentry should be
> > done first.  And that's not hard to do.
>
> It's just messy as it will require changes in all file systems.
>
> > Sorry, folks, but idea of private extendable syscall table (per-filesystem,
> > no less) doesn't look like a good thing.  That's _the_ reason why ioctl()
> > is bad.
>
> Unless I'm badly misreading the patch the op switch() is fixed in VFS mapping
> to clearly defined inode operations. It is not extensible per filesystem.
> Arguably they could be split into individual syscalls, but it looks not more
> like cosmetics at this point.

hi Al,

Below is an initial attempt at an interface which doesn't have a
command parameter at all, instead using separate syscalls as Andi
has suggested - is this closer to what you had in mind?

To prevent an exponential increase in the number of syscalls used,
it uses a flag parameter to distinguish similar operations and also
to combine the follow-symlink-or-not cases - it now uses six system
calls instead of the original three.

It also moves the individual VFS extended attribute operations out
into a separate ops vector for a little more interface clarity, not
sure if thats better/worse.  The patch ignores the problem of using
dentry vs. inode for now.

Thanks for the suggestions - I'd be interested in any thoughts you
have on this one as well.

cheers.

--
Nathan


diff -Naur 2.4.14-pristine/arch/i386/kernel/entry.S 
2.4.14-al/arch/i386/kernel/entry.S
--- 2.4.14-pristine/arch/i386/kernel/entry.S    Sat Nov  3 12:18:49 2001
+++ 2.4.14-al/arch/i386/kernel/entry.S  Tue Nov 13 15:54:55 2001
@@ -622,6 +622,12 @@
        .long SYMBOL_NAME(sys_ni_syscall)       /* Reserved for Security */
        .long SYMBOL_NAME(sys_gettid)
        .long SYMBOL_NAME(sys_readahead)        /* 225 */
+       .long SYMBOL_NAME(sys_setxattr)
+       .long SYMBOL_NAME(sys_fsetxattr)
+       .long SYMBOL_NAME(sys_getxattr)
+       .long SYMBOL_NAME(sys_fgetxattr)
+       .long SYMBOL_NAME(sys_listxattr)        /* 230 */
+       .long SYMBOL_NAME(sys_flistxattr)
 
        .rept NR_syscalls-(.-sys_call_table)/4
                .long SYMBOL_NAME(sys_ni_syscall)
diff -Naur 2.4.14-pristine/fs/Makefile 2.4.14-al/fs/Makefile
--- 2.4.14-pristine/fs/Makefile Tue Nov  6 08:40:59 2001
+++ 2.4.14-al/fs/Makefile       Fri Nov  9 15:27:42 2001
@@ -14,7 +14,7 @@
                super.o block_dev.o char_dev.o stat.o exec.o pipe.o namei.o \
                fcntl.o ioctl.o readdir.o select.o fifo.o locks.o \
                dcache.o inode.o attr.o bad_inode.o file.o iobuf.o dnotify.o \
-               filesystems.o namespace.o
+               filesystems.o namespace.o extattr.o
 
 ifeq ($(CONFIG_QUOTA),y)
 obj-y += dquot.o
diff -Naur 2.4.14-pristine/fs/extattr.c 2.4.14-al/fs/extattr.c
--- 2.4.14-pristine/fs/extattr.c        Thu Jan  1 10:00:00 1970
+++ 2.4.14-al/fs/extattr.c      Tue Nov 13 15:52:51 2001
@@ -0,0 +1,184 @@
+/*
+  File: fs/extattr.c
+
+  Extended attribute handling.
+ 
+  Copyright (C) 2001 by Andreas Gruenbacher <a.gruenbacher@xxxxxxxxxxxx>
+  Copyright (C) 2001 SGI - Silicon Graphics, Inc <linux-xfs@xxxxxxxxxxx>
+ */
+
+#include <linux/mm.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/smp_lock.h>
+#include <linux/extattr.h>
+
+
+/*
+ * Extended attribute SET operations
+ */
+static long
+setxattr(struct inode *i, char *name, void *value, size_t size, int flags)
+{
+       struct xattr_operations *ops;
+       int error = -EOPNOTSUPP;
+
+       lock_kernel();
+       ops = i->i_xop;
+       if (ops) {
+               if (flags & EA_CREATE) {
+                       if (ops->create)
+                               error = ops->create(i, name, value, size);
+               }
+               else if (flags & EA_REPLACE) {
+                       if (ops->replace)
+                               error = ops->replace(i, name, value, size);
+               }
+               else if (flags & EA_REMOVE) {
+                       if (ops->remove)
+                               error = ops->remove(i, name);
+               }
+               else if (ops->set)
+                       error = ops->set(i, name, value, size);
+       }
+       unlock_kernel();
+       return error;
+}
+
+asmlinkage long
+sys_setxattr(char *path, char *name, void *value, size_t size, int flags)
+{
+       struct nameidata nd;
+       int error;
+
+       error = (flags & EA_NOFOLLOW)?
+               user_path_walk_link(path, &nd):
+               user_path_walk(path, &nd);
+       if (error)
+               return error;
+       error = setxattr(nd.dentry->d_inode, name, value, size, flags);
+       path_release(&nd);
+       return error;
+}
+
+asmlinkage long
+sys_fsetxattr(int fd, char *name, void *value, size_t size, int flags)
+{
+       struct file *f;
+       int error = -EBADF;
+
+       f = fget(fd);
+       if (!f)
+               return error;
+       error = setxattr(f->f_dentry->d_inode, name, value, size, flags);
+       fput(f);
+       return error;
+}
+
+
+/*
+ * Extended attribute GET operations
+ */
+static long
+getxattr(struct inode *i, char *name, void *value, size_t size, int flags)
+{
+       struct xattr_operations *ops;
+       int error = -EOPNOTSUPP;
+
+       lock_kernel();
+       ops = i->i_xop;
+       if (ops) {
+               if (flags & EA_SIZEONLY) {
+                       if (ops->getsize)
+                               error = ops->getsize(i, name);
+               }
+               else if (ops->get)
+                       error = ops->get(i, name, value, size);
+       }
+       unlock_kernel();
+       return error;
+}
+
+asmlinkage long
+sys_getxattr(char *path, char *name, void *value, size_t size, int flags)
+{
+       struct nameidata nd;
+       int error;
+
+       error = (flags & EA_NOFOLLOW)?
+               user_path_walk_link(path, &nd):
+               user_path_walk(path, &nd);
+       if (error)
+               return error;
+       error = getxattr(nd.dentry->d_inode, name, value, size, flags);
+       path_release(&nd);
+       return error;
+}
+
+asmlinkage long
+sys_fgetxattr(int fd, char *name, void *value, size_t size, int flags)
+{
+       struct file *f;
+       int error = -EBADF;
+
+       f = fget(fd);
+       if (!f)
+               return error;
+       error = getxattr(f->f_dentry->d_inode, name, value, size, flags);
+       fput(f);
+       return error;
+}
+
+
+/*
+ * Extended attribute LIST operations
+ */
+static long
+listxattr(struct inode *i, char *name, void *value, size_t size, int flags)
+{
+       struct xattr_operations *ops;
+       int error = -EOPNOTSUPP;
+
+       lock_kernel();
+       ops = i->i_xop;
+       if (ops) {
+               if (flags & EA_SIZEONLY) {
+                       if (ops->listsize)
+                               error = ops->listsize(i, name);
+               }
+               else if (ops->list)
+                       error = ops->list(i, name, value, size);
+       }
+       unlock_kernel();
+       return error;
+}
+
+asmlinkage long
+sys_listxattr(char *path, char *name, void *value, size_t size, int flags)
+{
+       struct nameidata nd;
+       int error;
+
+       error = (flags & EA_NOFOLLOW)?
+               user_path_walk_link(path, &nd):
+               user_path_walk(path, &nd);
+       if (error)
+               return error;
+       error = listxattr(nd.dentry->d_inode, name, value, size, flags);
+       path_release(&nd);
+       return error;
+}
+
+asmlinkage long
+sys_flistxattr(int fd, char *name, void *value, size_t size, int flags)
+{
+       struct file *f;
+       int error = -EBADF;
+
+       f = fget(fd);
+       if (!f)
+               return error;
+       error = listxattr(f->f_dentry->d_inode, name, value, size, flags);
+       fput(f);
+       return error;
+}
diff -Naur 2.4.14-pristine/fs/inode.c 2.4.14-al/fs/inode.c
--- 2.4.14-pristine/fs/inode.c  Sat Sep 29 11:03:48 2001
+++ 2.4.14-al/fs/inode.c        Tue Nov 13 16:11:15 2001
@@ -768,10 +768,12 @@
 {
        static struct address_space_operations empty_aops;
        static struct inode_operations empty_iops;
+       static struct xattr_operations empty_xops;
        static struct file_operations empty_fops;
        memset(&inode->u, 0, sizeof(inode->u));
        inode->i_sock = 0;
        inode->i_op = &empty_iops;
+       inode->i_xop = &empty_xops;
        inode->i_fop = &empty_fops;
        inode->i_nlink = 1;
        atomic_set(&inode->i_writecount, 0);
diff -Naur 2.4.14-pristine/include/asm-i386/unistd.h 
2.4.14-al/include/asm-i386/unistd.h
--- 2.4.14-pristine/include/asm-i386/unistd.h   Thu Oct 18 03:03:03 2001
+++ 2.4.14-al/include/asm-i386/unistd.h Tue Nov 13 15:53:21 2001
@@ -230,6 +230,12 @@
 #define __NR_security          223     /* syscall for security modules */
 #define __NR_gettid            224
 #define __NR_readahead         225
+#define __NR_setxattr          226
+#define __NR_fsetxattr         227
+#define __NR_getxattr          228
+#define __NR_fgetxattr         229
+#define __NR_listxattr         230
+#define __NR_flistxattr                231
 
 /* user-visible error numbers are in the range -1 - -124: see 
<asm-i386/errno.h> */
 
diff -Naur 2.4.14-pristine/include/linux/extattr.h 
2.4.14-al/include/linux/extattr.h
--- 2.4.14-pristine/include/linux/extattr.h     Thu Jan  1 10:00:00 1970
+++ 2.4.14-al/include/linux/extattr.h   Tue Nov 13 15:43:08 2001
@@ -0,0 +1,18 @@
+/*
+  File: linux/extattr.h
+
+  Extended attributes handling.
+
+  Copyright (C) 2001 by Andreas Gruenbacher <a.gruenbacher@xxxxxxxxxxxx>
+  Copyright (C) 2001 SGI - Silicon Graphics, Inc <linux-xfs@xxxxxxxxxxx>
+*/
+#ifndef _LINUX_EXTATTR_H
+#define _LINUX_EXTATTR_H
+
+#define EA_CREATE      0x0001  /* Set the value: fail if attr already exists */
+#define EA_REPLACE     0x0002  /* Set the value: fail if attr does not exist */
+#define EA_REMOVE      0x0004  /* Remove the named attribute entirely */
+#define EA_SIZEONLY    0x0008  /* Retrieve a buffer size don't write into it */
+#define EA_NOFOLLOW    0x0010  /* Don't follow symlinks when traversing path */
+
+#endif /* _LINUX_EXTATTR_H */
diff -Naur 2.4.14-pristine/include/linux/fs.h 2.4.14-al/include/linux/fs.h
--- 2.4.14-pristine/include/linux/fs.h  Tue Nov  6 07:42:14 2001
+++ 2.4.14-al/include/linux/fs.h        Tue Nov 13 15:30:39 2001
@@ -444,6 +444,7 @@
        struct semaphore        i_zombie;
        struct inode_operations *i_op;
        struct file_operations  *i_fop; /* former ->i_op->default_file_ops */
+       struct xattr_operations *i_xop;
        struct super_block      *i_sb;
        wait_queue_head_t       i_wait;
        struct file_lock        *i_flock;
@@ -840,6 +841,17 @@
        int (*revalidate) (struct dentry *);
        int (*setattr) (struct dentry *, struct iattr *);
        int (*getattr) (struct dentry *, struct iattr *);
+};
+
+struct xattr_operations {
+       int (*create) (struct inode *, char *, void *, size_t);
+       int (*replace) (struct inode *, char *, void *, size_t);
+       int (*remove) (struct inode *, char *);
+       int (*set) (struct inode *, char *, void *, size_t);
+       int (*get) (struct inode *, char *, void *, size_t);
+       int (*getsize) (struct inode *, char *);
+       int (*list) (struct inode *, char *, void *, size_t);
+       int (*listsize) (struct inode *, char *);
 };
 
 /*


<Prev in Thread] Current Thread [Next in Thread>