xfs
[Top] [All Lists]

Re: [PATCH 2/9] xfs: separate xflags from xfs_ioctl_setattr

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/9] xfs: separate xflags from xfs_ioctl_setattr
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Thu, 29 Jan 2015 10:34:12 -0500
Cc: xfs@xxxxxxxxxxx, iustin@xxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1422328486-24661-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1422328486-24661-1-git-send-email-david@xxxxxxxxxxxxx> <1422328486-24661-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Tue, Jan 27, 2015 at 02:14:39PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> The setting of the extended flags is down through two separate
> interfaces, but they are munged together into xfs_ioctl_setattr
> and make that function far more complex than it needs to be.
> Separate it out into a helper function along with all the other
> common inode changes and transaction manipulations in
> xfs_ioctl_setattr().
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/xfs/xfs_ioctl.c | 89 
> +++++++++++++++++++++++++-----------------------------
>  1 file changed, 41 insertions(+), 48 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 09325ff..ba98412 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1013,6 +1013,44 @@ xfs_diflags_to_linux(
>               inode->i_flags &= ~S_NOATIME;
>  }
>  
> +static int
> +xfs_ioctl_setattr_xflags(
> +     struct xfs_trans        *tp,
> +     struct xfs_inode        *ip,
> +     struct fsxattr          *fa)
> +{
> +     struct xfs_mount        *mp = ip->i_mount;
> +
> +     /* Can't change realtime flag if any extents are allocated. */
> +     if ((ip->i_d.di_nextents || ip->i_delayed_blks) &&
> +         XFS_IS_REALTIME_INODE(ip) != (fa->fsx_xflags & XFS_XFLAG_REALTIME))
> +             return -EINVAL;
> +
> +     /* If realtime flag is set then must have realtime device */
> +     if (fa->fsx_xflags & XFS_XFLAG_REALTIME) {
> +             if (mp->m_sb.sb_rblocks == 0 || mp->m_sb.sb_rextsize == 0 ||
> +                 (ip->i_d.di_extsize % mp->m_sb.sb_rextsize))
> +                     return -EINVAL;
> +     }
> +
> +     /*
> +      * Can't modify an immutable/append-only file unless
> +      * we have appropriate permission.
> +      */
> +     if (((ip->i_d.di_flags & (XFS_DIFLAG_IMMUTABLE | XFS_DIFLAG_APPEND)) ||
> +          (fa->fsx_xflags & (XFS_XFLAG_IMMUTABLE | XFS_XFLAG_APPEND))) &&
> +         !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);
> +     xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> +     XFS_STATS_INC(xs_ig_attrchg);
> +     return 0;
> +}
> +
>  #define FSX_PROJID   1
>  #define FSX_EXTSIZE  2
>  #define FSX_XFLAGS   4
> @@ -1159,44 +1197,9 @@ xfs_ioctl_setattr(
>       }
>  
>  
> -     if (mask & FSX_XFLAGS) {
> -             /*
> -              * Can't change realtime flag if any extents are allocated.
> -              */
> -             if ((ip->i_d.di_nextents || ip->i_delayed_blks) &&
> -                 (XFS_IS_REALTIME_INODE(ip)) !=
> -                 (fa->fsx_xflags & XFS_XFLAG_REALTIME)) {
> -                     code = -EINVAL; /* EFBIG? */
> -                     goto error_return;
> -             }
> -
> -             /*
> -              * If realtime flag is set then must have realtime data.
> -              */
> -             if ((fa->fsx_xflags & XFS_XFLAG_REALTIME)) {
> -                     if ((mp->m_sb.sb_rblocks == 0) ||
> -                         (mp->m_sb.sb_rextsize == 0) ||
> -                         (ip->i_d.di_extsize % mp->m_sb.sb_rextsize)) {
> -                             code = -EINVAL;
> -                             goto error_return;
> -                     }
> -             }
> -
> -             /*
> -              * Can't modify an immutable/append-only file unless
> -              * we have appropriate permission.
> -              */
> -             if ((ip->i_d.di_flags &
> -                             (XFS_DIFLAG_IMMUTABLE|XFS_DIFLAG_APPEND) ||
> -                  (fa->fsx_xflags &
> -                             (XFS_XFLAG_IMMUTABLE | XFS_XFLAG_APPEND))) &&
> -                 !capable(CAP_LINUX_IMMUTABLE)) {
> -                     code = -EPERM;
> -                     goto error_return;
> -             }
> -     }
> -
> -     xfs_trans_ijoin(tp, ip, 0);
> +     code = xfs_ioctl_setattr_xflags(tp, ip, fa);
> +     if (code)
> +             goto error_return;
>  
>       /*
>        * Change file ownership.  Must be the owner or privileged.
> @@ -1227,11 +1230,6 @@ xfs_ioctl_setattr(
>  
>       }
>  
> -     if (mask & FSX_XFLAGS) {
> -             xfs_set_diflags(ip, fa->fsx_xflags);
> -             xfs_diflags_to_linux(ip);
> -     }
> -
>       /*
>        * Only set the extent size hint if we've already determined that the
>        * extent size hint should be set on the inode. If no extent size flags
> @@ -1246,11 +1244,6 @@ xfs_ioctl_setattr(
>               ip->i_d.di_extsize = extsize;
>       }
>  
> -     xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
> -     xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> -
> -     XFS_STATS_INC(xs_ig_attrchg);
> -
>       /*
>        * If this is a synchronous mount, make sure that the
>        * transaction goes to disk before returning to the user.
> -- 
> 2.0.0
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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