xfs
[Top] [All Lists]

[PATCH 3/9] xfs: factor out xfs_ioctl_setattr transaciton preamble

To: xfs@xxxxxxxxxxx
Subject: [PATCH 3/9] xfs: factor out xfs_ioctl_setattr transaciton preamble
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 27 Jan 2015 14:14:40 +1100
Cc: iustin@xxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1422328486-24661-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1422328486-24661-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

The setup of the transaction is done after a random smattering of
checks and before another bunch of ioperations specific
validity checks. Pull all the preamble out into a helper function
that returns a transaction or error.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 fs/xfs/xfs_ioctl.c | 96 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 45 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index ba98412..d06f596 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1042,7 +1042,6 @@ xfs_ioctl_setattr_xflags(
            !capable(CAP_LINUX_IMMUTABLE))
                return -EPERM;
 
-       xfs_trans_ijoin(tp, ip, 0);
        xfs_set_diflags(ip, fa->fsx_xflags);
        xfs_diflags_to_linux(ip);
        xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
@@ -1051,6 +1050,54 @@ xfs_ioctl_setattr_xflags(
        return 0;
 }
 
+/*
+ * Set up the transaction structure for the setattr operation, checking that we
+ * have permission to do so. On success, return a clean transaction and the
+ * inode locked exclusively ready for futher operation specific checks. On
+ * failure, return an error without modifying or locking the inode.
+ */
+static struct xfs_trans *
+xfs_ioctl_setattr_get_trans(
+       struct xfs_inode        *ip)
+{
+       struct xfs_mount        *mp = ip->i_mount;
+       struct xfs_trans        *tp;
+       int                     error;
+
+       if (mp->m_flags & XFS_MOUNT_RDONLY)
+               return ERR_PTR(-EROFS);
+       if (XFS_FORCED_SHUTDOWN(mp))
+               return ERR_PTR(-EIO);
+
+       tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
+       error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ichange, 0, 0);
+       if (error)
+               goto out_cancel;
+
+       xfs_ilock(ip, XFS_ILOCK_EXCL);
+       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
+
+       /*
+        * CAP_FOWNER overrides the following restrictions:
+        *
+        * The user ID of the calling process must be equal to the file owner
+        * ID, except in cases where the CAP_FSETID capability is applicable.
+        */
+       if (!inode_owner_or_capable(VFS_I(ip))) {
+               error = -EPERM;
+               goto out_cancel;
+       }
+
+       if (mp->m_flags & XFS_MOUNT_WSYNC)
+               xfs_trans_set_sync(tp);
+
+       return tp;
+
+out_cancel:
+       xfs_trans_cancel(tp, 0);
+       return ERR_PTR(error);
+}
+
 #define FSX_PROJID     1
 #define FSX_EXTSIZE    2
 #define FSX_XFLAGS     4
@@ -1063,7 +1110,6 @@ xfs_ioctl_setattr(
 {
        struct xfs_mount        *mp = ip->i_mount;
        struct xfs_trans        *tp;
-       unsigned int            lock_flags = 0;
        struct xfs_dquot        *udqp = NULL;
        struct xfs_dquot        *pdqp = NULL;
        struct xfs_dquot        *olddquot = NULL;
@@ -1071,11 +1117,6 @@ xfs_ioctl_setattr(
 
        trace_xfs_ioctl_setattr(ip);
 
-       if (mp->m_flags & XFS_MOUNT_RDONLY)
-               return -EROFS;
-       if (XFS_FORCED_SHUTDOWN(mp))
-               return -EIO;
-
        /*
         * Disallow 32bit project ids when projid32bit feature is not enabled.
         */
@@ -1099,29 +1140,9 @@ xfs_ioctl_setattr(
                        return code;
        }
 
-       /*
-        * For the other attributes, we acquire the inode lock and
-        * first do an error checking pass.
-        */
-       tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
-       code = xfs_trans_reserve(tp, &M_RES(mp)->tr_ichange, 0, 0);
-       if (code)
-               goto error_return;
-
-       lock_flags = XFS_ILOCK_EXCL;
-       xfs_ilock(ip, lock_flags);
-
-       /*
-        * CAP_FOWNER overrides the following restrictions:
-        *
-        * The user ID of the calling process must be equal
-        * to the file owner ID, except in cases where the
-        * CAP_FSETID capability is applicable.
-        */
-       if (!inode_owner_or_capable(VFS_I(ip))) {
-               code = -EPERM;
-               goto error_return;
-       }
+       tp = xfs_ioctl_setattr_get_trans(ip);
+       if (IS_ERR(tp))
+               return PTR_ERR(tp);
 
        /*
         * Do a quota reservation only if projid is actually going to change.
@@ -1244,20 +1265,7 @@ xfs_ioctl_setattr(
                ip->i_d.di_extsize = extsize;
        }
 
-       /*
-        * If this is a synchronous mount, make sure that the
-        * transaction goes to disk before returning to the user.
-        * This is slightly sub-optimal in that truncates require
-        * two sync transactions instead of one for wsync filesystems.
-        * One for the truncate and one for the timestamps since we
-        * don't want to change the timestamps unless we're sure the
-        * truncate worked.  Truncates are less than 1% of the laddis
-        * mix so this probably isn't worth the trouble to optimize.
-        */
-       if (mp->m_flags & XFS_MOUNT_WSYNC)
-               xfs_trans_set_sync(tp);
        code = xfs_trans_commit(tp, 0);
-       xfs_iunlock(ip, lock_flags);
 
        /*
         * Release any dquot(s) the inode had kept before chown.
@@ -1272,8 +1280,6 @@ xfs_ioctl_setattr(
        xfs_qm_dqrele(udqp);
        xfs_qm_dqrele(pdqp);
        xfs_trans_cancel(tp, 0);
-       if (lock_flags)
-               xfs_iunlock(ip, lock_flags);
        return code;
 }
 
-- 
2.0.0

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