xfs: make attr removal an explicit operation

Parent pointers match attrs on name+value, unlike everything else which
matches on only the name.  Therefore, we cannot keep using the heuristic
that !value means remove.  Make this an explicit operation code.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index b04e09143869dd1740b14172ea826a5eb1808a61..f8f7445b063c07c4168e42142273b7637b42e07c 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -916,10 +916,6 @@ xfs_attr_defer_add(
        trace_xfs_attr_defer_add(new->xattri_dela_state, args->dp);
 }
 
-/*
- * Note: If args->value is NULL the attribute will be removed, just like the
- * Linux ->setattr API.
- */
 int
 xfs_attr_set(
        struct xfs_da_args      *args,
@@ -955,7 +951,10 @@ xfs_attr_set(
        args->op_flags = XFS_DA_OP_OKNOENT |
                                        (args->op_flags & XFS_DA_OP_LOGGED);
 
-       if (args->value) {
+       switch (op) {
+       case XFS_ATTRUPDATE_UPSERT:
+       case XFS_ATTRUPDATE_CREATE:
+       case XFS_ATTRUPDATE_REPLACE:
                XFS_STATS_INC(mp, xs_attr_set);
                args->total = xfs_attr_calc_size(args, &local);
 
@@ -975,9 +974,11 @@ xfs_attr_set(
 
                if (!local)
                        rmt_blks = xfs_attr3_rmt_blocks(mp, args->valuelen);
-       } else {
+               break;
+       case XFS_ATTRUPDATE_REMOVE:
                XFS_STATS_INC(mp, xs_attr_remove);
                rmt_blks = xfs_attr3_rmt_blocks(mp, XFS_XATTR_SIZE_MAX);
+               break;
        }
 
        /*
@@ -989,7 +990,7 @@ xfs_attr_set(
        if (error)
                return error;
 
-       if (args->value || xfs_inode_hasattr(dp)) {
+       if (op != XFS_ATTRUPDATE_REMOVE || xfs_inode_hasattr(dp)) {
                error = xfs_iext_count_may_overflow(dp, XFS_ATTR_FORK,
                                XFS_IEXT_ATTR_MANIP_CNT(rmt_blks));
                if (error == -EFBIG)
@@ -1002,7 +1003,7 @@ xfs_attr_set(
        error = xfs_attr_lookup(args);
        switch (error) {
        case -EEXIST:
-               if (!args->value) {
+               if (op == XFS_ATTRUPDATE_REMOVE) {
                        /* if no value, we are performing a remove operation */
                        xfs_attr_defer_add(args, XFS_ATTRI_OP_FLAGS_REMOVE);
                        break;
@@ -1015,7 +1016,7 @@ xfs_attr_set(
                break;
        case -ENOATTR:
                /* Can't remove what isn't there. */
-               if (!args->value)
+               if (op == XFS_ATTRUPDATE_REMOVE)
                        goto out_trans_cancel;
 
                /* Pure replace fails if no existing attr to replace. */

diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 228360f7c85ce7276ef8de9a20a6bad4b9160406..c8005f52102adc530efbd7308bba9524cb76cb1e 100644 (file)
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -546,7 +546,8 @@ int xfs_attr_get_ilocked(struct xfs_da_args *args);
 int xfs_attr_get(struct xfs_da_args *args);
 
 enum xfs_attr_update {
-       XFS_ATTRUPDATE_UPSERTR, /* set/remove value, replace any existing attr */
+       XFS_ATTRUPDATE_REMOVE,  /* remove attr */
+       XFS_ATTRUPDATE_UPSERT,  /* set value, replace any existing attr */
        XFS_ATTRUPDATE_CREATE,  /* set value, fail if attr already exists */
        XFS_ATTRUPDATE_REPLACE, /* set value, fail if attr does not exist */
 };

diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
index 12f98af9e70921bfb17ad4eaa6492fe0415e51dc..c7c3dcfa2718f72de894e5425580a3ccde2fd882 100644 (file)
--- a/fs/xfs/xfs_acl.c
+++ b/fs/xfs/xfs_acl.c
@@ -201,16 +201,17 @@ __xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
                if (!args.value)
                        return -ENOMEM;
                xfs_acl_to_disk(args.value, acl);
+               error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT);
+               kvfree(args.value);
+       } else {
+               error = xfs_attr_change(&args, XFS_ATTRUPDATE_REMOVE);
+               /*
+                * If the attribute didn't exist to start with that's fine.
+                */
+               if (error == -ENOATTR)
+                       error = 0;
        }
 
-       error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERTR);
-       kvfree(args.value);
-
-       /*
-        * If the attribute didn't exist to start with that's fine.
-        */
-       if (!acl && error == -ENOATTR)
-               error = 0;
        if (!error)
                set_cached_acl(inode, type, acl);
        return error;

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 0eca7a9096e23fb988c90ab28d64857fe4d3a59b..e30f9f40f086d91a1fa091e69c10d04bfb48a283 100644 (file)
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -363,13 +363,16 @@ xfs_attr_filter(
 
 static inline enum xfs_attr_update
 xfs_xattr_flags(
-       u32                     ioc_flags)
+       u32                     ioc_flags,
+       void                    *value)
 {
+       if (!value)
+               return XFS_ATTRUPDATE_REMOVE;
        if (ioc_flags & XFS_IOC_ATTR_CREATE)
                return XFS_ATTRUPDATE_CREATE;
        if (ioc_flags & XFS_IOC_ATTR_REPLACE)
                return XFS_ATTRUPDATE_REPLACE;
-       return XFS_ATTRUPDATE_UPSERTR;
+       return XFS_ATTRUPDATE_UPSERT;
 }
 
 int
@@ -526,7 +529,7 @@ xfs_attrmulti_attr_set(
                args.valuelen = len;
        }
 
-       error = xfs_attr_change(&args, xfs_xattr_flags(flags));
+       error = xfs_attr_change(&args, xfs_xattr_flags(flags, args.value));
        if (!error && (flags & XFS_IOC_ATTR_ROOT))
                xfs_forget_acl(inode, name);
        kfree(args.value);

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index d02ca2248bbcfe9f3788cefba96c67d1c0e3068c..659fd10c0cda6259b597f546075306629bc44826 100644 (file)
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -63,7 +63,7 @@ xfs_initxattrs(
                        .value          = xattr->value,
                        .valuelen       = xattr->value_len,
                };
-               error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERTR);
+               error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT);
                if (error < 0)
                        break;
        }

diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 6ce1e06f650a583b8e2ec1391216898a696a0fec..0cbb93cf2869c48dc386b6fd37163bd8331d4173 100644 (file)
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -118,13 +118,16 @@ xfs_xattr_get(const struct xattr_handler *handler, struct dentry *unused,
 
 static inline enum xfs_attr_update
 xfs_xattr_flags_to_op(
-       int             flags)
+       int             flags,
+       const void      *value)
 {
+       if (!value)
+               return XFS_ATTRUPDATE_REMOVE;
        if (flags & XATTR_CREATE)
                return XFS_ATTRUPDATE_CREATE;
        if (flags & XATTR_REPLACE)
                return XFS_ATTRUPDATE_REPLACE;
-       return XFS_ATTRUPDATE_UPSERTR;
+       return XFS_ATTRUPDATE_UPSERT;
 }
 
 static int
@@ -143,7 +146,7 @@ xfs_xattr_set(const struct xattr_handler *handler,
        };
        int                     error;
 
-       error = xfs_attr_change(&args, xfs_xattr_flags_to_op(flags));
+       error = xfs_attr_change(&args, xfs_xattr_flags_to_op(flags, value));
        if (!error && (handler->flags & XFS_ATTR_ROOT))
                xfs_forget_acl(inode, name);
        return error;