xfs: clean up the attr flag confusion

Source kernel commit: d5f0f49a9bdd4206e941282dfd323c436331659b

The ATTR_* flags have a long IRIX history, where they a userspace
interface, the on-disk format and an internal interface.  We've split
out the on-disk interface to the XFS_ATTR_* values, but despite (or
because?) of that the flag have still been a mess.  Switch the
internal interface to pass the on-disk XFS_ATTR_* flags for the
namespace and the Linux XATTR_* flags for the actual flags instead.
The ATTR_* values that are actually used are move to xfs_fs.h with a
new XFS_IOC_* prefix to not conflict with the userspace version that
has the same name and must have the same value.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/db/attrset.c b/db/attrset.c
index 2d5e16485fc2aff0d7661b3a6743d13550813510..1ff2eb8584c7c41444854f7e294b37d3d3966985 100644 (file)
--- a/db/attrset.c
+++ b/db/attrset.c
@@ -16,6 +16,7 @@
 #include "field.h"
 #include "inode.h"
 #include "malloc.h"
+#include <sys/xattr.h>
 
 static int             attr_set_f(int argc, char **argv);
 static int             attr_remove_f(int argc, char **argv);
@@ -69,7 +70,7 @@ attr_set_f(
        struct xfs_inode        *ip = NULL;
        struct xfs_da_args      args = { NULL };
        char                    *name, *value, *sp;
-       int                     c, valuelen = 0, flags = 0;
+       int                     c, valuelen = 0;
 
        if (cur_typ == NULL) {
                dbprintf(_("no current type\n"));
@@ -84,23 +85,24 @@ attr_set_f(
                switch (c) {
                /* namespaces */
                case 'r':
-                       flags |= LIBXFS_ATTR_ROOT;
-                       flags &= ~LIBXFS_ATTR_SECURE;
+                       args.attr_filter |= LIBXFS_ATTR_ROOT;
+                       args.attr_filter &= ~LIBXFS_ATTR_SECURE;
                        break;
                case 'u':
-                       flags &= ~(LIBXFS_ATTR_ROOT | LIBXFS_ATTR_SECURE);
+                       args.attr_filter &= ~(LIBXFS_ATTR_ROOT |
+                                             LIBXFS_ATTR_SECURE);
                        break;
                case 's':
-                       flags |= LIBXFS_ATTR_SECURE;
-                       flags &= ~LIBXFS_ATTR_ROOT;
+                       args.attr_filter |= LIBXFS_ATTR_SECURE;
+                       args.attr_filter &= ~LIBXFS_ATTR_ROOT;
                        break;
 
                /* modifiers */
                case 'C':
-                       flags |= LIBXFS_ATTR_CREATE;
+                       args.attr_flags |= XATTR_CREATE;
                        break;
                case 'R':
-                       flags |= LIBXFS_ATTR_REPLACE;
+                       args.attr_flags |= XATTR_REPLACE;
                        break;
 
                case 'n':
@@ -152,7 +154,6 @@ attr_set_f(
        args.namelen = strlen(name);
        args.value = value;
        args.valuelen = valuelen;
-       args.flags = flags;
 
        if (libxfs_attr_set(&args)) {
                dbprintf(_("failed to set attr %s on inode %llu\n"),
@@ -180,7 +181,7 @@ attr_remove_f(
        struct xfs_inode        *ip = NULL;
        struct xfs_da_args      args = { NULL };
        char                    *name;
-       int                     c, flags = 0;
+       int                     c;
 
        if (cur_typ == NULL) {
                dbprintf(_("no current type\n"));
@@ -195,15 +196,16 @@ attr_remove_f(
                switch (c) {
                /* namespaces */
                case 'r':
-                       flags |= LIBXFS_ATTR_ROOT;
-                       flags &= ~LIBXFS_ATTR_SECURE;
+                       args.attr_filter |= LIBXFS_ATTR_ROOT;
+                       args.attr_filter &= ~LIBXFS_ATTR_SECURE;
                        break;
                case 'u':
-                       flags &= ~(LIBXFS_ATTR_ROOT | LIBXFS_ATTR_SECURE);
+                       args.attr_filter &= ~(LIBXFS_ATTR_ROOT |
+                                             LIBXFS_ATTR_SECURE);
                        break;
                case 's':
-                       flags |= LIBXFS_ATTR_SECURE;
-                       flags &= ~LIBXFS_ATTR_ROOT;
+                       args.attr_filter |= LIBXFS_ATTR_SECURE;
+                       args.attr_filter &= ~LIBXFS_ATTR_ROOT;
                        break;
 
                case 'n':
@@ -234,7 +236,6 @@ attr_remove_f(
        args.name = (unsigned char *)name;
        args.namelen = strlen(name);
        args.value = NULL;
-       args.flags = flags;
 
        if (libxfs_attr_set(&args)) {
                dbprintf(_("failed to remove attr %s from inode %llu\n"),

diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 0ffad205dd37ca0ae16d48617229991135f911b5..4462036b889005032a1c82243632c0079be16718 100644 (file)
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -13,10 +13,8 @@
  * it can be included in both the internal and external libxfs header files
  * without introducing any depenencies between the two.
  */
-#define LIBXFS_ATTR_CREATE             ATTR_CREATE
-#define LIBXFS_ATTR_REPLACE            ATTR_REPLACE
-#define LIBXFS_ATTR_ROOT               ATTR_ROOT
-#define LIBXFS_ATTR_SECURE             ATTR_SECURE
+#define LIBXFS_ATTR_ROOT               XFS_ATTR_ROOT
+#define LIBXFS_ATTR_SECURE             XFS_ATTR_SECURE
 
 #define xfs_agfl_size                  libxfs_agfl_size
 #define xfs_agfl_walk                  libxfs_agfl_walk

diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 68b6c0f0fcf7d5a87a526a2c47b971af4956eccb..6533ccb53af0814723fdc5ab2137984ce7615fab 100644 (file)
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -55,6 +55,8 @@
 #include "xfs_fs.h"
 #include "libfrog/crc32c.h"
 
+#include <sys/xattr.h>
+
 /* Zones used in libxfs allocations that aren't in shared header files */
 extern kmem_zone_t *xfs_buf_item_zone;
 extern kmem_zone_t *xfs_ili_zone;

diff --git a/libxfs/xfs_attr.c b/libxfs/xfs_attr.c
index f3176ac47e5c8e02c31f14f00c7fa60999f1ef47..36818814e6f3456c4281780e2c8ba080fa5a3f01 100644 (file)
--- a/libxfs/xfs_attr.c
+++ b/libxfs/xfs_attr.c
@@ -295,7 +295,7 @@ xfs_attr_set(
        struct xfs_inode        *dp = args->dp;
        struct xfs_mount        *mp = dp->i_mount;
        struct xfs_trans_res    tres;
-       int                     rsvd = (args->flags & ATTR_ROOT) != 0;
+       bool                    rsvd = (args->attr_filter & XFS_ATTR_ROOT);
        int                     error, local;
        unsigned int            total;
 
@@ -423,10 +423,10 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
        trace_xfs_attr_sf_addname(args);
 
        retval = xfs_attr_shortform_lookup(args);
-       if (retval == -ENOATTR && (args->flags & ATTR_REPLACE))
+       if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
                return retval;
        if (retval == -EEXIST) {
-               if (args->flags & ATTR_CREATE)
+               if (args->attr_flags & XATTR_CREATE)
                        return retval;
                retval = xfs_attr_shortform_remove(args);
                if (retval)
@@ -436,7 +436,7 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
                 * that the leaf format add routine won't trip over the attr
                 * not being around.
                 */
-               args->flags &= ~ATTR_REPLACE;
+               args->attr_flags &= ~XATTR_REPLACE;
        }
 
        if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
@@ -489,10 +489,10 @@ xfs_attr_leaf_addname(
         * the given flags produce an error or call for an atomic rename.
         */
        retval = xfs_attr3_leaf_lookup_int(bp, args);
-       if (retval == -ENOATTR && (args->flags & ATTR_REPLACE))
+       if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
                goto out_brelse;
        if (retval == -EEXIST) {
-               if (args->flags & ATTR_CREATE)  /* pure create op */
+               if (args->attr_flags & XATTR_CREATE)
                        goto out_brelse;
 
                trace_xfs_attr_leaf_replace(args);
@@ -763,10 +763,10 @@ restart:
                goto out;
        blk = &state->path.blk[ state->path.active-1 ];
        ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
-       if (retval == -ENOATTR && (args->flags & ATTR_REPLACE))
+       if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
                goto out;
        if (retval == -EEXIST) {
-               if (args->flags & ATTR_CREATE)
+               if (args->attr_flags & XATTR_CREATE)
                        goto out;
 
                trace_xfs_attr_node_replace(args);

diff --git a/libxfs/xfs_attr.h b/libxfs/xfs_attr.h
index 8d42f5782ff7a4d7dde536d63c2713ac8dad4dd5..a6bedb0eda2630e625f210225109ff4312d8d003 100644 (file)
--- a/libxfs/xfs_attr.h
+++ b/libxfs/xfs_attr.h
@@ -21,26 +21,6 @@ struct xfs_attr_list_context;
  * as possible so as to fit into the literal area of the inode.
  */
 
-/*========================================================================
- * External interfaces
- *========================================================================*/
-
-
-#define ATTR_DONTFOLLOW        0x0001  /* -- ignored, from IRIX -- */
-#define ATTR_ROOT      0x0002  /* use attrs in root (trusted) namespace */
-#define ATTR_TRUST     0x0004  /* -- unused, from IRIX -- */
-#define ATTR_SECURE    0x0008  /* use attrs in security namespace */
-#define ATTR_CREATE    0x0010  /* pure create: fail if attr already exists */
-#define ATTR_REPLACE   0x0020  /* pure set: fail if attr does not exist */
-
-#define XFS_ATTR_FLAGS \
-       { ATTR_DONTFOLLOW,      "DONTFOLLOW" }, \
-       { ATTR_ROOT,            "ROOT" }, \
-       { ATTR_TRUST,           "TRUST" }, \
-       { ATTR_SECURE,          "SECURE" }, \
-       { ATTR_CREATE,          "CREATE" }, \
-       { ATTR_REPLACE,         "REPLACE" }
-
 /*
  * The maximum size (into the kernel or returned from the kernel) of an
  * attribute value or the buffer used for an attr_list() call.  Larger
@@ -87,7 +67,7 @@ struct xfs_attr_list_context {
        int                     dupcnt;         /* count dup hashvals seen */
        int                     bufsize;        /* total buffer size */
        int                     firstu;         /* first used byte in buffer */
-       int                     flags;          /* from VOP call */
+       unsigned int            attr_filter;    /* XFS_ATTR_{ROOT,SECURE} */
        int                     resynch;        /* T/F: resynch with cursor */
        put_listent_func_t      put_listent;    /* list output fmt function */
        int                     index;          /* index into output buffer */

diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index 86e3135321f55da20120c1a5bfc5ad75e38e907a..d560f94eca354f5af34658ce81bae744aa038dbe 100644 (file)
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -453,8 +453,7 @@ xfs_attr_match(
                return false;
        if (memcmp(args->name, name, namelen) != 0)
                return false;
-       if (XFS_ATTR_NSP_ARGS_TO_ONDISK(args->flags) !=
-           XFS_ATTR_NSP_ONDISK(flags))
+       if (args->attr_filter != (flags & XFS_ATTR_NSP_ONDISK_MASK))
                return false;
        return true;
 }
@@ -694,7 +693,7 @@ xfs_attr_shortform_add(xfs_da_args_t *args, int forkoff)
 
        sfe->namelen = args->namelen;
        sfe->valuelen = args->valuelen;
-       sfe->flags = XFS_ATTR_NSP_ARGS_TO_ONDISK(args->flags);
+       sfe->flags = args->attr_filter;
        memcpy(sfe->nameval, args->name, args->namelen);
        memcpy(&sfe->nameval[args->namelen], args->value, args->valuelen);
        sf->hdr.count++;
@@ -903,7 +902,7 @@ xfs_attr_shortform_to_leaf(
                nargs.valuelen = sfe->valuelen;
                nargs.hashval = xfs_da_hashname(sfe->nameval,
                                                sfe->namelen);
-               nargs.flags = XFS_ATTR_NSP_ONDISK_TO_ARGS(sfe->flags);
+               nargs.attr_filter = sfe->flags & XFS_ATTR_NSP_ONDISK_MASK;
                error = xfs_attr3_leaf_lookup_int(bp, &nargs); /* set a->index */
                ASSERT(error == -ENOATTR);
                error = xfs_attr3_leaf_add(bp, &nargs);
@@ -1109,7 +1108,7 @@ xfs_attr3_leaf_to_shortform(
                nargs.value = &name_loc->nameval[nargs.namelen];
                nargs.valuelen = be16_to_cpu(name_loc->valuelen);
                nargs.hashval = be32_to_cpu(entry->hashval);
-               nargs.flags = XFS_ATTR_NSP_ONDISK_TO_ARGS(entry->flags);
+               nargs.attr_filter = entry->flags & XFS_ATTR_NSP_ONDISK_MASK;
                xfs_attr_shortform_add(&nargs, forkoff);
        }
        error = 0;
@@ -1434,8 +1433,9 @@ xfs_attr3_leaf_add_work(
        entry->nameidx = cpu_to_be16(ichdr->freemap[mapindex].base +
                                     ichdr->freemap[mapindex].size);
        entry->hashval = cpu_to_be32(args->hashval);
-       entry->flags = tmp ? XFS_ATTR_LOCAL : 0;
-       entry->flags |= XFS_ATTR_NSP_ARGS_TO_ONDISK(args->flags);
+       entry->flags = args->attr_filter;
+       if (tmp)
+               entry->flags |= XFS_ATTR_LOCAL;
        if (args->op_flags & XFS_DA_OP_RENAME) {
                entry->flags |= XFS_ATTR_INCOMPLETE;
                if ((args->blkno2 == args->blkno) &&

diff --git a/libxfs/xfs_da_btree.h b/libxfs/xfs_da_btree.h
index d93cb8385b52f3f2fd9d16df09711c583d0e0b45..f3660ae9eb3ea864f7f6f62b02c2af400120733f 100644 (file)
--- a/libxfs/xfs_da_btree.h
+++ b/libxfs/xfs_da_btree.h
@@ -59,7 +59,8 @@ typedef struct xfs_da_args {
        uint8_t         filetype;       /* filetype of inode for directories */
        void            *value;         /* set of bytes (maybe contain NULLs) */
        int             valuelen;       /* length of value */
-       int             flags;          /* argument flags (eg: ATTR_NOCREATE) */
+       unsigned int    attr_filter;    /* XFS_ATTR_{ROOT,SECURE} */
+       unsigned int    attr_flags;     /* XATTR_{CREATE,REPLACE} */
        xfs_dahash_t    hashval;        /* hash value of name */
        xfs_ino_t       inumber;        /* input/output inode number */
        struct xfs_inode *dp;           /* directory inode to manipulate */

diff --git a/libxfs/xfs_da_format.h b/libxfs/xfs_da_format.h
index 734837a9b51a739cb531d60d3f89520463fab097..08c0a4d98b89a0d2bd73363dffcd95d11fb93bff 100644 (file)
--- a/libxfs/xfs_da_format.h
+++ b/libxfs/xfs_da_format.h
@@ -692,19 +692,7 @@ struct xfs_attr3_leafblock {
 #define XFS_ATTR_ROOT          (1 << XFS_ATTR_ROOT_BIT)
 #define XFS_ATTR_SECURE                (1 << XFS_ATTR_SECURE_BIT)
 #define XFS_ATTR_INCOMPLETE    (1 << XFS_ATTR_INCOMPLETE_BIT)
-
-/*
- * Conversion macros for converting namespace bits from argument flags
- * to ondisk flags.
- */
-#define XFS_ATTR_NSP_ARGS_MASK         (ATTR_ROOT | ATTR_SECURE)
 #define XFS_ATTR_NSP_ONDISK_MASK       (XFS_ATTR_ROOT | XFS_ATTR_SECURE)
-#define XFS_ATTR_NSP_ONDISK(flags)     ((flags) & XFS_ATTR_NSP_ONDISK_MASK)
-#define XFS_ATTR_NSP_ARGS(flags)       ((flags) & XFS_ATTR_NSP_ARGS_MASK)
-#define XFS_ATTR_NSP_ARGS_TO_ONDISK(x) (((x) & ATTR_ROOT ? XFS_ATTR_ROOT : 0) |\
-                                        ((x) & ATTR_SECURE ? XFS_ATTR_SECURE : 0))
-#define XFS_ATTR_NSP_ONDISK_TO_ARGS(x) (((x) & XFS_ATTR_ROOT ? ATTR_ROOT : 0) |\
-                                        ((x) & XFS_ATTR_SECURE ? ATTR_SECURE : 0))
 
 /*
  * Alignment for namelist and valuelist entries (since they are mixed

diff --git a/libxfs/xfs_fs.h b/libxfs/xfs_fs.h
index 51a688f2d0b958f0b48a4331ac601e3f71b5ced3..38beb999c9b9474b6c44b7a1dddecc9967721f31 100644 (file)
--- a/libxfs/xfs_fs.h
+++ b/libxfs/xfs_fs.h
@@ -589,6 +589,16 @@ typedef struct xfs_fsop_setdm_handlereq {
        struct fsdmidata                __user *data;   /* DMAPI data   */
 } xfs_fsop_setdm_handlereq_t;
 
+/*
+ * Flags passed in xfs_attr_multiop.am_flags for the attr ioctl interface.
+ *
+ * NOTE: Must match the values declared in libattr without the XFS_IOC_ prefix.
+ */
+#define XFS_IOC_ATTR_ROOT      0x0002  /* use attrs in root namespace */
+#define XFS_IOC_ATTR_SECURE    0x0008  /* use attrs in security namespace */
+#define XFS_IOC_ATTR_CREATE    0x0010  /* fail if attr already exists */
+#define XFS_IOC_ATTR_REPLACE   0x0020  /* fail if attr does not exist */
+
 typedef struct xfs_attrlist_cursor {
        __u32           opaque[4];
 } xfs_attrlist_cursor_t;
@@ -630,7 +640,7 @@ typedef struct xfs_attr_multiop {
        void            __user *am_attrname;
        void            __user *am_attrvalue;
        __u32           am_length;
-       __u32           am_flags;
+       __u32           am_flags; /* XFS_IOC_ATTR_* */
 } xfs_attr_multiop_t;
 
 typedef struct xfs_fsop_attrmulti_handlereq {