From: Tim Shimmin <tes@sgi.com>
To: Christoph Hellwig <hch@lst.de>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 1/3] split out xfs value-add from xfs_setattr
Date: Mon, 07 Jul 2008 21:29:37 +1000 [thread overview]
Message-ID: <4871FE21.9020609@sgi.com> (raw)
In-Reply-To: <20080627154553.GA31476@lst.de>
Christoph,
I've gone thru this and it looks good to me.
However, there is enough stuff which was extracted and duplicated
from different places in xfs_setattr to easily miss something (IMHO) and so we
would need to test this carefully I reckon.
I'll have a look at the other 2 patches tomorrow.
Below are my notes:
* remove related code to quota, xflags, etc from xfs_setattr
* duplicate support code from xfs_setattr
* I need to compare old xfs_setattr() with xfs_ioctl_setattr() to check that
the code is still doing the same thing for the relevant cases...
* The "can't change extent size test" is duplicated as we split
up the predicate of EXTSIZE and XFLAGS into two
Okay
* xfs_trans_log_inode doesn't need to be called
in 2 spots now it can be done after the di_extsize and the set_diflags
spot and no need for just after the projid one.
Okay
* And we just call xfs_ichgtime directly instead of noting a timeflag
Okay
* commit_flags are zero as are non-zero for AT_SIZE changes
which we don't deal with here
Okay
* This code is different:
+ if (DM_EVENT_ENABLED(ip, DM_EVENT_ATTRIBUTE)) {
+ XFS_SEND_NAMESP(mp, DM_EVENT_ATTRIBUTE, ip, DM_RIGHT_NULL,
+ NULL, DM_RIGHT_NULL, NULL, NULL, 0, 0,
+ (mask & FSX_NONBLOCK) ? DM_FLAGS_NDELAY : 0);
+ }
+
+ vn_revalidate(XFS_ITOV(ip)); /* update flags */
Q: What happened to the ATTR_DMI flag testing?
A: We never set ATTR_DMI in the flags by the ioctls.
Okay cool.
Q: Why have you introduced the vn_revalidate?
A: Because you took it out of the callers to xfs_ioctl_setattr.
Okay
* So XFS_AT_ALL and XFS_AT_STAT are not used currently
Okay
--Tim
Christoph Hellwig wrote:
> xfs_setattr currently doesn't just handle the attributes set through
> ->setattr but also addition XFS-specific attributes: project id, inode
> flags and extent size hint. Having these in a single function makes it
> more complicated and forces to have us a bhv_vattr intermediate
> structure eating up stackspace.
>
> This patch adds a new xfs_ioctl_setattr helper for the XFS ioctls that
> set these attributes and remove the code to set them through
> xfs_setattr.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_ioctl.c 2008-06-15 16:41:09.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c 2008-06-15 16:42:18.000000000 +0200
> @@ -47,6 +47,8 @@
> #include "xfs_dfrag.h"
> #include "xfs_fsops.h"
> #include "xfs_vnodeops.h"
> +#include "xfs_quota.h"
> +#include "xfs_inode_item.h"
>
> #include <linux/capability.h>
> #include <linux/dcache.h>
> @@ -875,6 +877,297 @@ xfs_ioc_fsgetxattr(
> return 0;
> }
>
> +STATIC void
> +xfs_set_diflags(
> + struct xfs_inode *ip,
> + unsigned int xflags)
> +{
> + unsigned int di_flags;
> +
> + /* can't set PREALLOC this way, just preserve it */
> + di_flags = (ip->i_d.di_flags & XFS_DIFLAG_PREALLOC);
> + if (xflags & XFS_XFLAG_IMMUTABLE)
> + di_flags |= XFS_DIFLAG_IMMUTABLE;
> + if (xflags & XFS_XFLAG_APPEND)
> + di_flags |= XFS_DIFLAG_APPEND;
> + if (xflags & XFS_XFLAG_SYNC)
> + di_flags |= XFS_DIFLAG_SYNC;
> + if (xflags & XFS_XFLAG_NOATIME)
> + di_flags |= XFS_DIFLAG_NOATIME;
> + if (xflags & XFS_XFLAG_NODUMP)
> + di_flags |= XFS_DIFLAG_NODUMP;
> + if (xflags & XFS_XFLAG_PROJINHERIT)
> + di_flags |= XFS_DIFLAG_PROJINHERIT;
> + if (xflags & XFS_XFLAG_NODEFRAG)
> + di_flags |= XFS_DIFLAG_NODEFRAG;
> + if (xflags & XFS_XFLAG_FILESTREAM)
> + di_flags |= XFS_DIFLAG_FILESTREAM;
> + if ((ip->i_d.di_mode & S_IFMT) == S_IFDIR) {
> + if (xflags & XFS_XFLAG_RTINHERIT)
> + di_flags |= XFS_DIFLAG_RTINHERIT;
> + if (xflags & XFS_XFLAG_NOSYMLINKS)
> + di_flags |= XFS_DIFLAG_NOSYMLINKS;
> + if (xflags & XFS_XFLAG_EXTSZINHERIT)
> + di_flags |= XFS_DIFLAG_EXTSZINHERIT;
> + } else if ((ip->i_d.di_mode & S_IFMT) == S_IFREG) {
> + if (xflags & XFS_XFLAG_REALTIME)
> + di_flags |= XFS_DIFLAG_REALTIME;
> + if (xflags & XFS_XFLAG_EXTSIZE)
> + di_flags |= XFS_DIFLAG_EXTSIZE;
> + }
> +
> + ip->i_d.di_flags = di_flags;
> +}
> +
> +
> +#define FSX_PROJID 1
> +#define FSX_EXTSIZE 2
> +#define FSX_XFLAGS 4
> +#define FSX_NONBLOCK 8
> +
> +STATIC int
> +xfs_ioctl_setattr(
> + xfs_inode_t *ip,
> + struct fsxattr *fa,
> + int mask)
> +{
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_trans *tp;
> + unsigned int lock_flags = 0;
> + struct xfs_dquot *udqp = NULL, *gdqp = NULL;
> + struct xfs_dquot *olddquot = NULL;
> + int code;
> +
> + xfs_itrace_entry(ip);
> +
> + if (mp->m_flags & XFS_MOUNT_RDONLY)
> + return XFS_ERROR(EROFS);
> + if (XFS_FORCED_SHUTDOWN(mp))
> + return XFS_ERROR(EIO);
> +
> + /*
> + * If disk quotas is on, we make sure that the dquots do exist on disk,
> + * before we start any other transactions. Trying to do this later
> + * is messy. We don't care to take a readlock to look at the ids
> + * in inode here, because we can't hold it across the trans_reserve.
> + * If the IDs do change before we take the ilock, we're covered
> + * because the i_*dquot fields will get updated anyway.
> + */
> + if (XFS_IS_QUOTA_ON(mp) && (mask & FSX_PROJID)) {
> + code = XFS_QM_DQVOPALLOC(mp, ip, ip->i_d.di_uid,
> + ip->i_d.di_gid, fa->fsx_projid,
> + XFS_QMOPT_PQUOTA, &udqp, &gdqp);
> + if (code)
> + 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, 0, XFS_ICHANGE_LOG_RES(mp), 0, 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 (current->fsuid != ip->i_d.di_uid && !capable(CAP_FOWNER)) {
> + code = XFS_ERROR(EPERM);
> + goto error_return;
> + }
> +
> + /*
> + * Do a quota reservation only if projid is actually going to change.
> + */
> + if (mask & FSX_PROJID) {
> + if (XFS_IS_PQUOTA_ON(mp) &&
> + ip->i_d.di_projid != fa->fsx_projid) {
> + ASSERT(tp);
> + code = XFS_QM_DQVOPCHOWNRESV(mp, tp, ip, udqp, gdqp,
> + capable(CAP_FOWNER) ?
> + XFS_QMOPT_FORCE_RES : 0);
> + if (code) /* out of quota */
> + goto error_return;
> + }
> + }
> +
> + if (mask & FSX_EXTSIZE) {
> + /*
> + * Can't change extent size if any extents are allocated.
> + */
> + if (ip->i_d.di_nextents &&
> + ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) !=
> + fa->fsx_extsize)) {
> + code = XFS_ERROR(EINVAL); /* EFBIG? */
> + goto error_return;
> + }
> +
> + /*
> + * Extent size must be a multiple of the appropriate block
> + * size, if set at all.
> + */
> + if (fa->fsx_extsize != 0) {
> + xfs_extlen_t size;
> +
> + if (XFS_IS_REALTIME_INODE(ip) ||
> + ((mask & FSX_XFLAGS) &&
> + (fa->fsx_xflags & XFS_XFLAG_REALTIME))) {
> + size = mp->m_sb.sb_rextsize <<
> + mp->m_sb.sb_blocklog;
> + } else {
> + size = mp->m_sb.sb_blocksize;
> + }
> +
> + if (fa->fsx_extsize % size) {
> + code = XFS_ERROR(EINVAL);
> + goto error_return;
> + }
> + }
> + }
> +
> +
> + 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 = XFS_ERROR(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 = XFS_ERROR(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 = XFS_ERROR(EPERM);
> + goto error_return;
> + }
> + }
> +
> + xfs_trans_ijoin(tp, ip, lock_flags);
> + xfs_trans_ihold(tp, ip);
> +
> + /*
> + * Change file ownership. Must be the owner or privileged.
> + * If the system was configured with the "restricted_chown"
> + * option, the owner is not permitted to give away the file,
> + * and can change the group id only to a group of which he
> + * or she is a member.
> + */
> + if (mask & FSX_PROJID) {
> + /*
> + * CAP_FSETID overrides the following restrictions:
> + *
> + * The set-user-ID and set-group-ID bits of a file will be
> + * cleared upon successful return from chown()
> + */
> + if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
> + !capable(CAP_FSETID))
> + ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
> +
> + /*
> + * Change the ownerships and register quota modifications
> + * in the transaction.
> + */
> + if (ip->i_d.di_projid != fa->fsx_projid) {
> + if (XFS_IS_PQUOTA_ON(mp)) {
> + olddquot = XFS_QM_DQVOPCHOWN(mp, tp, ip,
> + &ip->i_gdquot, gdqp);
> + }
> + ip->i_d.di_projid = fa->fsx_projid;
> +
> + /*
> + * We may have to rev the inode as well as
> + * the superblock version number since projids didn't
> + * exist before DINODE_VERSION_2 and SB_VERSION_NLINK.
> + */
> + if (ip->i_d.di_version == XFS_DINODE_VERSION_1)
> + xfs_bump_ino_vers2(tp, ip);
> + }
> +
> + }
> +
> + if (mask & FSX_EXTSIZE)
> + ip->i_d.di_extsize = fa->fsx_extsize >> mp->m_sb.sb_blocklog;
> + if (mask & FSX_XFLAGS)
> + xfs_set_diflags(ip, fa->fsx_xflags);
> +
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> + xfs_ichgtime(ip, XFS_ICHGTIME_CHG);
> +
> + 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.
> + * 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.
> + */
> + XFS_QM_DQRELE(mp, olddquot);
> + XFS_QM_DQRELE(mp, udqp);
> + XFS_QM_DQRELE(mp, gdqp);
> +
> + if (code)
> + return code;
> +
> + if (DM_EVENT_ENABLED(ip, DM_EVENT_ATTRIBUTE)) {
> + XFS_SEND_NAMESP(mp, DM_EVENT_ATTRIBUTE, ip, DM_RIGHT_NULL,
> + NULL, DM_RIGHT_NULL, NULL, NULL, 0, 0,
> + (mask & FSX_NONBLOCK) ? DM_FLAGS_NDELAY : 0);
> + }
> +
> + vn_revalidate(XFS_ITOV(ip)); /* update flags */
> + return 0;
> +
> + error_return:
> + XFS_QM_DQRELE(mp, udqp);
> + XFS_QM_DQRELE(mp, gdqp);
> + xfs_trans_cancel(tp, 0);
> + if (lock_flags)
> + xfs_iunlock(ip, lock_flags);
> + return code;
> +}
> +
> STATIC int
> xfs_ioc_fssetxattr(
> xfs_inode_t *ip,
> @@ -882,31 +1175,16 @@ xfs_ioc_fssetxattr(
> void __user *arg)
> {
> struct fsxattr fa;
> - struct bhv_vattr *vattr;
> - int error;
> - int attr_flags;
> + unsigned int mask;
>
> if (copy_from_user(&fa, arg, sizeof(fa)))
> return -EFAULT;
>
> - vattr = kmalloc(sizeof(*vattr), GFP_KERNEL);
> - if (unlikely(!vattr))
> - return -ENOMEM;
> -
> - attr_flags = 0;
> + mask = FSX_XFLAGS | FSX_EXTSIZE | FSX_PROJID;
> if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
> - attr_flags |= ATTR_NONBLOCK;
> + mask |= FSX_NONBLOCK;
>
> - vattr->va_mask = XFS_AT_XFLAGS | XFS_AT_EXTSIZE | XFS_AT_PROJID;
> - vattr->va_xflags = fa.fsx_xflags;
> - vattr->va_extsize = fa.fsx_extsize;
> - vattr->va_projid = fa.fsx_projid;
> -
> - error = -xfs_setattr(ip, vattr, attr_flags, NULL);
> - if (!error)
> - vn_revalidate(XFS_ITOV(ip)); /* update flags */
> - kfree(vattr);
> - return 0;
> + return -xfs_ioctl_setattr(ip, &fa, mask);
> }
>
> STATIC int
> @@ -928,10 +1206,9 @@ xfs_ioc_setxflags(
> struct file *filp,
> void __user *arg)
> {
> - struct bhv_vattr *vattr;
> + struct fsxattr fa;
> unsigned int flags;
> - int attr_flags;
> - int error;
> + unsigned int mask;
>
> if (copy_from_user(&flags, arg, sizeof(flags)))
> return -EFAULT;
> @@ -941,22 +1218,12 @@ xfs_ioc_setxflags(
> FS_SYNC_FL))
> return -EOPNOTSUPP;
>
> - vattr = kmalloc(sizeof(*vattr), GFP_KERNEL);
> - if (unlikely(!vattr))
> - return -ENOMEM;
> -
> - attr_flags = 0;
> + mask = FSX_XFLAGS;
> if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
> - attr_flags |= ATTR_NONBLOCK;
> + mask |= FSX_NONBLOCK;
> + fa.fsx_xflags = xfs_merge_ioc_xflags(flags, xfs_ip2xflags(ip));
>
> - vattr->va_mask = XFS_AT_XFLAGS;
> - vattr->va_xflags = xfs_merge_ioc_xflags(flags, xfs_ip2xflags(ip));
> -
> - error = -xfs_setattr(ip, vattr, attr_flags, NULL);
> - if (likely(!error))
> - vn_revalidate(XFS_ITOV(ip)); /* update flags */
> - kfree(vattr);
> - return error;
> + return -xfs_ioctl_setattr(ip, &fa, mask);
> }
>
> STATIC int
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_vnode.h
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_vnode.h 2008-06-15 17:32:14.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_vnode.h 2008-06-15 17:35:44.000000000 +0200
> @@ -117,26 +117,11 @@ typedef struct bhv_vattr {
> #define XFS_AT_ACL 0x00080000
> #define XFS_AT_CAP 0x00100000
> #define XFS_AT_INF 0x00200000
> -#define XFS_AT_XFLAGS 0x00400000
> -#define XFS_AT_EXTSIZE 0x00800000
> #define XFS_AT_NEXTENTS 0x01000000
> #define XFS_AT_ANEXTENTS 0x02000000
> -#define XFS_AT_PROJID 0x04000000
> #define XFS_AT_SIZE_NOPERM 0x08000000
> #define XFS_AT_GENCOUNT 0x10000000
>
> -#define XFS_AT_ALL (XFS_AT_TYPE|XFS_AT_MODE|XFS_AT_UID|XFS_AT_GID|\
> - XFS_AT_FSID|XFS_AT_NODEID|XFS_AT_NLINK|XFS_AT_SIZE|\
> - XFS_AT_ATIME|XFS_AT_MTIME|XFS_AT_CTIME|XFS_AT_RDEV|\
> - XFS_AT_BLKSIZE|XFS_AT_NBLOCKS|XFS_AT_VCODE|XFS_AT_MAC|\
> - XFS_AT_ACL|XFS_AT_CAP|XFS_AT_INF|XFS_AT_XFLAGS|XFS_AT_EXTSIZE|\
> - XFS_AT_NEXTENTS|XFS_AT_ANEXTENTS|XFS_AT_PROJID|XFS_AT_GENCOUNT)
> -
> -#define XFS_AT_STAT (XFS_AT_TYPE|XFS_AT_MODE|XFS_AT_UID|XFS_AT_GID|\
> - XFS_AT_FSID|XFS_AT_NODEID|XFS_AT_NLINK|XFS_AT_SIZE|\
> - XFS_AT_ATIME|XFS_AT_MTIME|XFS_AT_CTIME|XFS_AT_RDEV|\
> - XFS_AT_BLKSIZE|XFS_AT_NBLOCKS|XFS_AT_PROJID)
> -
> #define XFS_AT_TIMES (XFS_AT_ATIME|XFS_AT_MTIME|XFS_AT_CTIME)
>
> #define XFS_AT_UPDTIMES (XFS_AT_UPDATIME|XFS_AT_UPDMTIME|XFS_AT_UPDCTIME)
> Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_vnodeops.c 2008-06-15 17:32:42.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_vnodeops.c 2008-06-15 17:39:09.000000000 +0200
> @@ -94,7 +94,6 @@ xfs_setattr(
> uid_t uid=0, iuid=0;
> gid_t gid=0, igid=0;
> int timeflags = 0;
> - xfs_prid_t projid=0, iprojid=0;
> struct xfs_dquot *udqp, *gdqp, *olddquot1, *olddquot2;
> int file_owner;
> int need_iolock = 1;
> @@ -139,8 +138,7 @@ xfs_setattr(
> * If the IDs do change before we take the ilock, we're covered
> * because the i_*dquot fields will get updated anyway.
> */
> - if (XFS_IS_QUOTA_ON(mp) &&
> - (mask & (XFS_AT_UID|XFS_AT_GID|XFS_AT_PROJID))) {
> + if (XFS_IS_QUOTA_ON(mp) && (mask & (XFS_AT_UID|XFS_AT_GID))) {
> uint qflags = 0;
>
> if ((mask & XFS_AT_UID) && XFS_IS_UQUOTA_ON(mp)) {
> @@ -155,12 +153,7 @@ xfs_setattr(
> } else {
> gid = ip->i_d.di_gid;
> }
> - if ((mask & XFS_AT_PROJID) && XFS_IS_PQUOTA_ON(mp)) {
> - projid = vap->va_projid;
> - qflags |= XFS_QMOPT_PQUOTA;
> - } else {
> - projid = ip->i_d.di_projid;
> - }
> +
> /*
> * We take a reference when we initialize udqp and gdqp,
> * so it is important that we never blindly double trip on
> @@ -168,8 +161,8 @@ xfs_setattr(
> */
> ASSERT(udqp == NULL);
> ASSERT(gdqp == NULL);
> - code = XFS_QM_DQVOPALLOC(mp, ip, uid, gid, projid, qflags,
> - &udqp, &gdqp);
> + code = XFS_QM_DQVOPALLOC(mp, ip, uid, gid, ip->i_d.di_projid,
> + qflags, &udqp, &gdqp);
> if (code)
> return code;
> }
> @@ -219,9 +212,7 @@ xfs_setattr(
> * Only the owner or users with CAP_FOWNER
> * capability may do these things.
> */
> - if (mask &
> - (XFS_AT_MODE|XFS_AT_XFLAGS|XFS_AT_EXTSIZE|XFS_AT_UID|
> - XFS_AT_GID|XFS_AT_PROJID)) {
> + if (mask & (XFS_AT_MODE|XFS_AT_UID|XFS_AT_GID)) {
> /*
> * CAP_FOWNER overrides the following restrictions:
> *
> @@ -270,7 +261,7 @@ xfs_setattr(
> * and can change the group id only to a group of which he
> * or she is a member.
> */
> - if (mask & (XFS_AT_UID|XFS_AT_GID|XFS_AT_PROJID)) {
> + if (mask & (XFS_AT_UID|XFS_AT_GID)) {
> /*
> * These IDs could have changed since we last looked at them.
> * But, we're assured that if the ownership did change
> @@ -278,12 +269,9 @@ xfs_setattr(
> * would have changed also.
> */
> iuid = ip->i_d.di_uid;
> - iprojid = ip->i_d.di_projid;
> igid = ip->i_d.di_gid;
> gid = (mask & XFS_AT_GID) ? vap->va_gid : igid;
> uid = (mask & XFS_AT_UID) ? vap->va_uid : iuid;
> - projid = (mask & XFS_AT_PROJID) ? (xfs_prid_t)vap->va_projid :
> - iprojid;
>
> /*
> * CAP_CHOWN overrides the following restrictions:
> @@ -303,11 +291,10 @@ xfs_setattr(
> goto error_return;
> }
> /*
> - * Do a quota reservation only if uid/projid/gid is actually
> + * Do a quota reservation only if uid/gid is actually
> * going to change.
> */
> if ((XFS_IS_UQUOTA_ON(mp) && iuid != uid) ||
> - (XFS_IS_PQUOTA_ON(mp) && iprojid != projid) ||
> (XFS_IS_GQUOTA_ON(mp) && igid != gid)) {
> ASSERT(tp);
> code = XFS_QM_DQVOPCHOWNRESV(mp, tp, ip, udqp, gdqp,
> @@ -361,78 +348,6 @@ xfs_setattr(
> }
>
> /*
> - * Change extent size or realtime flag.
> - */
> - if (mask & (XFS_AT_EXTSIZE|XFS_AT_XFLAGS)) {
> - /*
> - * Can't change extent size if any extents are allocated.
> - */
> - if (ip->i_d.di_nextents && (mask & XFS_AT_EXTSIZE) &&
> - ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) !=
> - vap->va_extsize) ) {
> - code = XFS_ERROR(EINVAL); /* EFBIG? */
> - goto error_return;
> - }
> -
> - /*
> - * Can't change realtime flag if any extents are allocated.
> - */
> - if ((ip->i_d.di_nextents || ip->i_delayed_blks) &&
> - (mask & XFS_AT_XFLAGS) &&
> - (XFS_IS_REALTIME_INODE(ip)) !=
> - (vap->va_xflags & XFS_XFLAG_REALTIME)) {
> - code = XFS_ERROR(EINVAL); /* EFBIG? */
> - goto error_return;
> - }
> - /*
> - * Extent size must be a multiple of the appropriate block
> - * size, if set at all.
> - */
> - if ((mask & XFS_AT_EXTSIZE) && vap->va_extsize != 0) {
> - xfs_extlen_t size;
> -
> - if (XFS_IS_REALTIME_INODE(ip) ||
> - ((mask & XFS_AT_XFLAGS) &&
> - (vap->va_xflags & XFS_XFLAG_REALTIME))) {
> - size = mp->m_sb.sb_rextsize <<
> - mp->m_sb.sb_blocklog;
> - } else {
> - size = mp->m_sb.sb_blocksize;
> - }
> - if (vap->va_extsize % size) {
> - code = XFS_ERROR(EINVAL);
> - goto error_return;
> - }
> - }
> - /*
> - * If realtime flag is set then must have realtime data.
> - */
> - if ((mask & XFS_AT_XFLAGS) &&
> - (vap->va_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 = XFS_ERROR(EINVAL);
> - goto error_return;
> - }
> - }
> -
> - /*
> - * Can't modify an immutable/append-only file unless
> - * we have appropriate permission.
> - */
> - if ((mask & XFS_AT_XFLAGS) &&
> - (ip->i_d.di_flags &
> - (XFS_DIFLAG_IMMUTABLE|XFS_DIFLAG_APPEND) ||
> - (vap->va_xflags &
> - (XFS_XFLAG_IMMUTABLE | XFS_XFLAG_APPEND))) &&
> - !capable(CAP_LINUX_IMMUTABLE)) {
> - code = XFS_ERROR(EPERM);
> - goto error_return;
> - }
> - }
> -
> - /*
> * Now we can make the changes. Before we join the inode
> * to the transaction, if XFS_AT_SIZE is set then take care of
> * the part of the truncation that must be done without the
> @@ -568,7 +483,7 @@ xfs_setattr(
> * and can change the group id only to a group of which he
> * or she is a member.
> */
> - if (mask & (XFS_AT_UID|XFS_AT_GID|XFS_AT_PROJID)) {
> + if (mask & (XFS_AT_UID|XFS_AT_GID)) {
> /*
> * CAP_FSETID overrides the following restrictions:
> *
> @@ -603,23 +518,6 @@ xfs_setattr(
> }
> ip->i_d.di_gid = gid;
> }
> - if (iprojid != projid) {
> - if (XFS_IS_PQUOTA_ON(mp)) {
> - ASSERT(!XFS_IS_GQUOTA_ON(mp));
> - ASSERT(mask & XFS_AT_PROJID);
> - ASSERT(gdqp);
> - olddquot2 = XFS_QM_DQVOPCHOWN(mp, tp, ip,
> - &ip->i_gdquot, gdqp);
> - }
> - ip->i_d.di_projid = projid;
> - /*
> - * We may have to rev the inode as well as
> - * the superblock version number since projids didn't
> - * exist before DINODE_VERSION_2 and SB_VERSION_NLINK.
> - */
> - if (ip->i_d.di_version == XFS_DINODE_VERSION_1)
> - xfs_bump_ino_vers2(tp, ip);
> - }
>
> xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
> timeflags |= XFS_ICHGTIME_CHG;
> @@ -647,57 +545,6 @@ xfs_setattr(
> }
>
> /*
> - * Change XFS-added attributes.
> - */
> - if (mask & (XFS_AT_EXTSIZE|XFS_AT_XFLAGS)) {
> - if (mask & XFS_AT_EXTSIZE) {
> - /*
> - * Converting bytes to fs blocks.
> - */
> - ip->i_d.di_extsize = vap->va_extsize >>
> - mp->m_sb.sb_blocklog;
> - }
> - if (mask & XFS_AT_XFLAGS) {
> - uint di_flags;
> -
> - /* can't set PREALLOC this way, just preserve it */
> - di_flags = (ip->i_d.di_flags & XFS_DIFLAG_PREALLOC);
> - if (vap->va_xflags & XFS_XFLAG_IMMUTABLE)
> - di_flags |= XFS_DIFLAG_IMMUTABLE;
> - if (vap->va_xflags & XFS_XFLAG_APPEND)
> - di_flags |= XFS_DIFLAG_APPEND;
> - if (vap->va_xflags & XFS_XFLAG_SYNC)
> - di_flags |= XFS_DIFLAG_SYNC;
> - if (vap->va_xflags & XFS_XFLAG_NOATIME)
> - di_flags |= XFS_DIFLAG_NOATIME;
> - if (vap->va_xflags & XFS_XFLAG_NODUMP)
> - di_flags |= XFS_DIFLAG_NODUMP;
> - if (vap->va_xflags & XFS_XFLAG_PROJINHERIT)
> - di_flags |= XFS_DIFLAG_PROJINHERIT;
> - if (vap->va_xflags & XFS_XFLAG_NODEFRAG)
> - di_flags |= XFS_DIFLAG_NODEFRAG;
> - if (vap->va_xflags & XFS_XFLAG_FILESTREAM)
> - di_flags |= XFS_DIFLAG_FILESTREAM;
> - if ((ip->i_d.di_mode & S_IFMT) == S_IFDIR) {
> - if (vap->va_xflags & XFS_XFLAG_RTINHERIT)
> - di_flags |= XFS_DIFLAG_RTINHERIT;
> - if (vap->va_xflags & XFS_XFLAG_NOSYMLINKS)
> - di_flags |= XFS_DIFLAG_NOSYMLINKS;
> - if (vap->va_xflags & XFS_XFLAG_EXTSZINHERIT)
> - di_flags |= XFS_DIFLAG_EXTSZINHERIT;
> - } else if ((ip->i_d.di_mode & S_IFMT) == S_IFREG) {
> - if (vap->va_xflags & XFS_XFLAG_REALTIME)
> - di_flags |= XFS_DIFLAG_REALTIME;
> - if (vap->va_xflags & XFS_XFLAG_EXTSIZE)
> - di_flags |= XFS_DIFLAG_EXTSIZE;
> - }
> - ip->i_d.di_flags = di_flags;
> - }
> - xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> - timeflags |= XFS_ICHGTIME_CHG;
> - }
> -
> - /*
> * Change file inode change time only if XFS_AT_CTIME set
> * AND we have been called by a DMI function.
> */
>
prev parent reply other threads:[~2008-07-07 11:28 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-27 15:45 [PATCH 1/3] split out xfs value-add from xfs_setattr Christoph Hellwig
2008-07-07 11:29 ` Tim Shimmin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4871FE21.9020609@sgi.com \
--to=tes@sgi.com \
--cc=hch@lst.de \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.