xfs: pack icreate initialization parameters into a separate structure

Callers that want to create an inode currently pass all possible file
attribute values for the new inode into xfs_init_new_inode as ten
separate parameters.  This causes two code maintenance issues: first, we
have large multi-line call sites which programmers must read carefully
to make sure they did not accidentally invert a value.  Second, all
three file id parameters must be passed separately to the quota
functions; any discrepancy results in quota count errors.

Clean this up by creating a new icreate_args structure to hold all this
information, some helpers to initialize them properly, and make the
callers pass this structure through to the creation function, whose name
we shorten to xfs_icreate.  This eliminates the issues, enables us to
keep the inode init code in sync with userspace via libxfs, and is
needed for future metadata directory tree management.

(A subsequent cleanup will also fix the quota alloc calls.)

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

diff --git a/fs/xfs/libxfs/xfs_inode_util.h b/fs/xfs/libxfs/xfs_inode_util.h
index f7e4d5a8235dda8562621db20faf27248a6fe70f..9226482fdee8cea668a2730c89e172256b350ec4 100644 (file)
--- a/fs/xfs/libxfs/xfs_inode_util.h
+++ b/fs/xfs/libxfs/xfs_inode_util.h
@@ -13,4 +13,26 @@ uint32_t     xfs_ip2xflags(struct xfs_inode *ip);
 
 prid_t         xfs_get_initial_prid(struct xfs_inode *dp);
 
+/*
+ * File creation context.
+ *
+ * Due to our only partial reliance on the VFS to propagate uid and gid values
+ * according to accepted Unix behaviors, callers must initialize idmap to the
+ * correct idmapping structure to get the correct inheritance behaviors when
+ * XFS_MOUNT_GRPID is set.
+ *
+ * To create files detached from the directory tree (e.g. quota inodes), set
+ * idmap to NULL.  To create a tree root, set pip to NULL.
+ */
+struct xfs_icreate_args {
+       struct mnt_idmap        *idmap;
+       struct xfs_inode        *pip;   /* parent inode or null */
+       dev_t                   rdev;
+       umode_t                 mode;
+
+#define XFS_ICREATE_TMPFILE    (1U << 0)  /* create an unlinked file */
+#define XFS_ICREATE_INIT_XATTRS        (1U << 1)  /* will set xattrs immediately */
+       uint16_t                flags;
+};
+
 #endif /* __XFS_INODE_UTIL_H__ */

diff --git a/fs/xfs/scrub/tempfile.c b/fs/xfs/scrub/tempfile.c
index b747b625c5ee4eb6408fedb96d854ef6a464dfc4..ee6f93e9f7cba78ceb812d86894a48e545fea21b 100644 (file)
--- a/fs/xfs/scrub/tempfile.c
+++ b/fs/xfs/scrub/tempfile.c
@@ -40,6 +40,11 @@ xrep_tempfile_create(
        struct xfs_scrub        *sc,
        uint16_t                mode)
 {
+       struct xfs_icreate_args args = {
+               .pip            = sc->mp->m_rootip,
+               .mode           = mode,
+               .flags          = XFS_ICREATE_TMPFILE,
+       };
        struct xfs_mount        *mp = sc->mp;
        struct xfs_trans        *tp = NULL;
        struct xfs_dquot        *udqp = NULL;
@@ -87,14 +92,11 @@ xrep_tempfile_create(
        error = xfs_dialloc(&tp, dp->i_ino, mode, &ino);
        if (error)
                goto out_trans_cancel;
-       error = xfs_init_new_inode(&nop_mnt_idmap, tp, dp, ino, mode, 0, 0,
-                       0, false, &sc->tempip);
+       error = xfs_icreate(tp, ino, &args, &sc->tempip);
        if (error)
                goto out_trans_cancel;
 
-       /* Change the ownership of the inode to root. */
-       VFS_I(sc->tempip)->i_uid = GLOBAL_ROOT_UID;
-       VFS_I(sc->tempip)->i_gid = GLOBAL_ROOT_GID;
+       /* We don't touch file data, so drop the realtime flags. */
        sc->tempip->i_diflags &= ~(XFS_DIFLAG_REALTIME | XFS_DIFLAG_RTINHERIT);
        xfs_trans_log_inode(tp, sc->tempip, XFS_ILOG_CORE);
 

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index afc798ffa164efd37a029cc0a1b63f9f8063f559..4f13ad0fa14ecc54617dc13ee82bf2542c6aa164 100644 (file)
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -662,18 +662,13 @@ xfs_inode_inherit_flags2(
  * Caller is responsible for unlocking the inode manually upon return
  */
 int
-xfs_init_new_inode(
-       struct mnt_idmap        *idmap,
+xfs_icreate(
        struct xfs_trans        *tp,
-       struct xfs_inode        *pip,
        xfs_ino_t               ino,
-       umode_t                 mode,
-       xfs_nlink_t             nlink,
-       dev_t                   rdev,
-       prid_t                  prid,
-       bool                    init_xattrs,
+       const struct xfs_icreate_args *args,
        struct xfs_inode        **ipp)
 {
+       struct xfs_inode        *pip = args->pip;
        struct inode            *dir = pip ? VFS_I(pip) : NULL;
        struct xfs_mount        *mp = tp->t_mountp;
        struct xfs_inode        *ip;
@@ -706,26 +701,43 @@ xfs_init_new_inode(
 
        ASSERT(ip != NULL);
        inode = VFS_I(ip);
-       set_nlink(inode, nlink);
-       inode->i_rdev = rdev;
-       ip->i_projid = prid;
-
-       if (dir && !(dir->i_mode & S_ISGID) && xfs_has_grpid(mp)) {
-               inode_fsuid_set(inode, idmap);
-               inode->i_gid = dir->i_gid;
-               inode->i_mode = mode;
+
+       if (args->flags & XFS_ICREATE_TMPFILE)
+               set_nlink(inode, 0);
+       else if (S_ISDIR(args->mode))
+               set_nlink(inode, 2);
+       else
+               set_nlink(inode, 1);
+       inode->i_rdev = args->rdev;
+
+       if (!args->idmap || pip == NULL) {
+               /* creating a tree root, sb rooted, or detached file */
+               inode->i_uid = GLOBAL_ROOT_UID;
+               inode->i_gid = GLOBAL_ROOT_GID;
+               ip->i_projid = 0;
+               inode->i_mode = args->mode;
        } else {
-               inode_init_owner(idmap, inode, dir, mode);
-       }
+               /* creating a child in the directory tree */
+               if (dir && !(dir->i_mode & S_ISGID) && xfs_has_grpid(mp)) {
+                       inode_fsuid_set(inode, args->idmap);
+                       inode->i_gid = dir->i_gid;
+                       inode->i_mode = args->mode;
+               } else {
+                       inode_init_owner(args->idmap, inode, dir, args->mode);
+               }
 
-       /*
-        * If the group ID of the new file does not match the effective group
-        * ID or one of the supplementary group IDs, the S_ISGID bit is cleared
-        * (and only if the irix_sgid_inherit compatibility variable is set).
-        */
-       if (irix_sgid_inherit && (inode->i_mode & S_ISGID) &&
-           !vfsgid_in_group_p(i_gid_into_vfsgid(idmap, inode)))
-               inode->i_mode &= ~S_ISGID;
+               /*
+                * If the group ID of the new file does not match the effective
+                * group ID or one of the supplementary group IDs, the S_ISGID
+                * bit is cleared (and only if the irix_sgid_inherit
+                * compatibility variable is set).
+                */
+               if (irix_sgid_inherit && (inode->i_mode & S_ISGID) &&
+                   !vfsgid_in_group_p(i_gid_into_vfsgid(args->idmap, inode)))
+                       inode->i_mode &= ~S_ISGID;
+
+               ip->i_projid = pip ? xfs_get_initial_prid(pip) : 0;
+       }
 
        ip->i_disk_size = 0;
        ip->i_df.if_nextents = 0;
@@ -745,7 +757,7 @@ xfs_init_new_inode(
        }
 
        flags = XFS_ILOG_CORE;
-       switch (mode & S_IFMT) {
+       switch (args->mode & S_IFMT) {
        case S_IFIFO:
        case S_IFCHR:
        case S_IFBLK:
@@ -778,7 +790,7 @@ xfs_init_new_inode(
         * this saves us from needing to run a separate transaction to set the
         * fork offset in the immediate future.
         */
-       if (init_xattrs) {
+       if (args->flags & XFS_ICREATE_INIT_XATTRS) {
                ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
                xfs_ifork_init_attr(ip, XFS_DINODE_FMT_EXTENTS, 0);
 
@@ -941,6 +953,13 @@ xfs_create(
        bool                    init_xattrs,
        xfs_inode_t             **ipp)
 {
+       struct xfs_icreate_args args = {
+               .idmap          = idmap,
+               .pip            = dp,
+               .rdev           = rdev,
+               .mode           = mode,
+               .flags          = init_xattrs ? XFS_ICREATE_INIT_XATTRS : 0,
+       };
        int                     is_dir = S_ISDIR(mode);
        struct xfs_mount        *mp = dp->i_mount;
        struct xfs_inode        *ip = NULL;
@@ -1016,8 +1035,7 @@ xfs_create(
         */
        error = xfs_dialloc(&tp, dp->i_ino, mode, &ino);
        if (!error)
-               error = xfs_init_new_inode(idmap, tp, dp, ino, mode,
-                               is_dir ? 2 : 1, rdev, prid, init_xattrs, &ip);
+               error = xfs_icreate(tp, ino, &args, &ip);
        if (error)
                goto out_trans_cancel;
 
@@ -1125,11 +1143,17 @@ xfs_create_tmpfile(
        bool                    init_xattrs,
        struct xfs_inode        **ipp)
 {
+       struct xfs_icreate_args args = {
+               .idmap          = idmap,
+               .pip            = dp,
+               .mode           = mode,
+               .flags          = XFS_ICREATE_TMPFILE,
+       };
        struct xfs_mount        *mp = dp->i_mount;
        struct xfs_inode        *ip = NULL;
        struct xfs_trans        *tp = NULL;
        int                     error;
-       prid_t                  prid;
+       prid_t                  prid;
        struct xfs_dquot        *udqp = NULL;
        struct xfs_dquot        *gdqp = NULL;
        struct xfs_dquot        *pdqp = NULL;
@@ -1141,6 +1165,8 @@ xfs_create_tmpfile(
                return -EIO;
 
        prid = xfs_get_initial_prid(dp);
+       if (init_xattrs)
+               args.flags |= XFS_ICREATE_INIT_XATTRS;
 
        /*
         * Make sure that we have allocated dquot(s) on disk.  The uid/gid
@@ -1164,8 +1190,7 @@ xfs_create_tmpfile(
 
        error = xfs_dialloc(&tp, dp->i_ino, mode, &ino);
        if (!error)
-               error = xfs_init_new_inode(idmap, tp, dp, ino, mode,
-                               0, 0, prid, init_xattrs, &ip);
+               error = xfs_icreate(tp, ino, &args, &ip);
        if (error)
                goto out_trans_cancel;
 

diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 15ab7a1c79a608690a5d24f6a6d91e7b5d72f8fc..7d3fea66e069e3c2ffa3c9fde733ddd0896530d1 100644 (file)
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -554,10 +554,8 @@ int                xfs_iflush_cluster(struct xfs_buf *);
 void           xfs_lock_two_inodes(struct xfs_inode *ip0, uint ip0_mode,
                                struct xfs_inode *ip1, uint ip1_mode);
 
-int xfs_init_new_inode(struct mnt_idmap *idmap, struct xfs_trans *tp,
-               struct xfs_inode *pip, xfs_ino_t ino, umode_t mode,
-               xfs_nlink_t nlink, dev_t rdev, prid_t prid, bool init_xattrs,
-               struct xfs_inode **ipp);
+int xfs_icreate(struct xfs_trans *tp, xfs_ino_t ino,
+               const struct xfs_icreate_args *args, struct xfs_inode **ipp);
 
 static inline int
 xfs_itruncate_extents(

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 47120b745c47f7f0a6b7e22e7993905262691d71..78f839630c624b85aac4ca9b7dd50291ad0d5c1f 100644 (file)
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -793,12 +793,14 @@ xfs_qm_qino_alloc(
                return error;
 
        if (need_alloc) {
+               struct xfs_icreate_args args = {
+                       .mode           = S_IFREG,
+               };
                xfs_ino_t       ino;
 
                error = xfs_dialloc(&tp, 0, S_IFREG, &ino);
                if (!error)
-                       error = xfs_init_new_inode(&nop_mnt_idmap, tp, NULL, ino,
-                                       S_IFREG, 1, 0, 0, false, ipp);
+                       error = xfs_icreate(tp, ino, &args, ipp);
                if (error) {
                        xfs_trans_cancel(tp);
                        return error;

diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 53ed512c6f21115c530e115d4446c07681484bfc..3b797a39950d55f36515d2fc8e133e3ec997f18b 100644 (file)
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -90,6 +90,11 @@ xfs_symlink(
        struct xfs_inode        **ipp)
 {
        struct xfs_mount        *mp = dp->i_mount;
+       struct xfs_icreate_args args = {
+               .idmap          = idmap,
+               .pip            = dp,
+               .mode           = S_IFLNK | (mode & ~S_IFMT),
+       };
        struct xfs_trans        *tp = NULL;
        struct xfs_inode        *ip = NULL;
        int                     error = 0;
@@ -111,6 +116,9 @@ xfs_symlink(
        if (xfs_is_shutdown(mp))
                return -EIO;
 
+       if (xfs_has_parent(mp))
+               args.flags |= XFS_ICREATE_INIT_XATTRS;
+
        /*
         * Check component lengths of the target path name.
         */
@@ -170,9 +178,7 @@ xfs_symlink(
         */
        error = xfs_dialloc(&tp, dp->i_ino, S_IFLNK, &ino);
        if (!error)
-               error = xfs_init_new_inode(idmap, tp, dp, ino,
-                               S_IFLNK | (mode & ~S_IFMT), 1, 0, prid,
-                               xfs_has_parent(mp), &ip);
+               error = xfs_icreate(tp, ino, &args, &ip);
        if (error)
                goto out_trans_cancel;