Hi,
On Wed, 2009-09-23 at 14:41 +0200, Christoph Hellwig wrote:
> On Thu, Jun 25, 2009 at 07:35:43PM +0200, Christoph Hellwig wrote:
> > Add a flags argument to strcut xattr_handler and pass it to all xattr
> > handler methods. This allows using the same methods for multiple
> > handlers, e.g. for the ACL methods which perform exactly the same action
> > for the access and default ACLs, just using a different underlying
> > attribute. With a little more groundwork it'll also allow sharing the
> > methods for the regular user/trusted/secure handlers in extN, ocfs2 and
> > jffs2 like it's already done for xfs in this patch.
> >
> > Also change the inode argument to the handlers to a dentry to allow
> > using the handlers mechnism for filesystems that require it later,
> > e.g. cifs.
>
> Updated for current mainline which saw gfs2 converted to the xattr
> handlers infrastructure:
>
Wrt the GFS2 bits, they look good, but with one query:
> Index: linux-2.6/fs/gfs2/acl.c
> ===================================================================
> --- linux-2.6.orig/fs/gfs2/acl.c 2009-09-22 23:48:35.905254352 -0300
> +++ linux-2.6/fs/gfs2/acl.c 2009-09-22 23:48:40.801005886 -0300
> @@ -217,8 +217,8 @@ int gfs2_acl_create(struct gfs2_inode *d
> acl = clone;
>
> if (S_ISDIR(ip->i_inode.i_mode)) {
> - error = gfs2_xattr_set(&ip->i_inode, GFS2_EATYPE_SYS,
> - GFS2_POSIX_ACL_DEFAULT, data, len, 0);
> + error = __gfs2_xattr_set(ip, GFS2_POSIX_ACL_DEFAULT, data,
> + len, 0, GFS2_EATYPE_SYS);
> if (error)
> goto out;
> }
> @@ -230,8 +230,8 @@ int gfs2_acl_create(struct gfs2_inode *d
> goto munge;
>
> posix_acl_to_xattr(acl, data, len);
> - error = gfs2_xattr_set(&ip->i_inode, GFS2_EATYPE_SYS,
> - GFS2_POSIX_ACL_ACCESS, data, len, 0);
> + error = __gfs2_xattr_set(ip, GFS2_POSIX_ACL_ACCESS, data, len, 0,
> + GFS2_EATYPE_SYS);
> if (error)
> goto out;
> munge:
> Index: linux-2.6/fs/gfs2/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/gfs2/inode.c 2009-09-22 23:48:35.913254045 -0300
> +++ linux-2.6/fs/gfs2/inode.c 2009-09-22 23:48:40.804006338 -0300
> @@ -801,7 +801,7 @@ static int gfs2_security_init(struct gfs
> return err;
> }
>
> - err = gfs2_xattr_set(&ip->i_inode, GFS2_EATYPE_SECURITY, name, value, len, 0);
> + err = __gfs2_xattr_set(ip, name, value, len, 0, GFS2_EATYPE_SECURITY);
> kfree(value);
> kfree(name);
>
> Index: linux-2.6/fs/gfs2/xattr.c
> ===================================================================
> --- linux-2.6.orig/fs/gfs2/xattr.c 2009-09-22 23:48:35.918278918 -0300
> +++ linux-2.6/fs/gfs2/xattr.c 2009-09-22 23:48:40.809005649 -0300
> @@ -537,18 +537,17 @@ int gfs2_ea_get_copy(struct gfs2_inode *
> /**
> * gfs2_xattr_get - Get a GFS2 extended attribute
> * @inode: The inode
> - * @type: The type of extended attribute
> * @name: The name of the extended attribute
> * @buffer: The buffer to write the result into
> * @size: The size of the buffer
> + * @type: The type of extended attribute
> *
> * Returns: actual size of data on success, -errno on error
> */
> -
> -int gfs2_xattr_get(struct inode *inode, int type, const char *name,
> - void *buffer, size_t size)
> +static int gfs2_xattr_get(struct dentry *dentry, const char *name,
> + void *buffer, size_t size, int type)
> {
> - struct gfs2_inode *ip = GFS2_I(inode);
> + struct gfs2_inode *ip = GFS2_I(dentry->d_inode);
> struct gfs2_ea_location el;
> int error;
>
> @@ -1089,7 +1088,7 @@ static int ea_remove_stuffed(struct gfs2
>
> /**
> * gfs2_xattr_remove - Remove a GFS2 extended attribute
> - * @inode: The inode
> + * @ip: The inode
> * @type: The type of the extended attribute
> * @name: The name of the extended attribute
> *
> @@ -1100,9 +1099,8 @@ static int ea_remove_stuffed(struct gfs2
> * Returns: 0, or errno on failure
> */
>
> -static int gfs2_xattr_remove(struct inode *inode, int type, const char *name)
> +static int gfs2_xattr_remove(struct gfs2_inode *ip, int type, const char *name)
> {
> - struct gfs2_inode *ip = GFS2_I(inode);
> struct gfs2_ea_location el;
> int error;
>
> @@ -1126,35 +1124,32 @@ static int gfs2_xattr_remove(struct inod
> }
>
> /**
> - * gfs2_xattr_set - Set (or remove) a GFS2 extended attribute
> - * @inode: The inode
> - * @type: The type of the extended attribute
> + * __gfs2_xattr_set - Set (or remove) a GFS2 extended attribute
> + * @ip: The inode
> * @name: The name of the extended attribute
> * @value: The value of the extended attribute (NULL for remove)
> * @size: The size of the @value argument
> * @flags: Create or Replace
> + * @type: The type of the extended attribute
> *
> * See gfs2_xattr_remove() for details of the removal of xattrs.
> *
> * Returns: 0 or errno on failure
> */
>
> -int gfs2_xattr_set(struct inode *inode, int type, const char *name,
> - const void *value, size_t size, int flags)
> +int __gfs2_xattr_set(struct gfs2_inode *ip, const char *name,
> + const void *value, size_t size, int flags, int type)
> {
> - struct gfs2_sbd *sdp = GFS2_SB(inode);
> - struct gfs2_inode *ip = GFS2_I(inode);
> + struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct gfs2_ea_location el;
> unsigned int namel = strlen(name);
> int error;
>
> - if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> - return -EPERM;
We need to leave these checks here because the checks made in the VFS
are done before the glock is held, so they might yield the wrong
answers. I think we should check again here just to be on the safe side.
Otherwise:
Acked-by: Steven Whitehouse <swhiteho@xxxxxxxxxx>
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
