xfs: always return -ENOSPC on project quota reservation failure
authorEric Sandeen <sandeen@redhat.com>
Thu, 21 May 2020 20:06:59 +0000 (13:06 -0700)
committerDarrick J. Wong <darrick.wong@oracle.com>
Wed, 27 May 2020 15:49:25 +0000 (08:49 -0700)
XFS project quota treats project hierarchies as "mini filesysems" and
so rather than -EDQUOT, the intent is to return -ENOSPC when a quota
reservation fails, but this behavior is not consistent.

The only place we make a decision between -EDQUOT and -ENOSPC
returns based on quota type is in xfs_trans_dqresv().

This behavior is currently controlled by whether or not the
XFS_QMOPT_ENOSPC flag gets passed into the quota reservation.  However,
its use is not consistent; paths such as xfs_create() and xfs_symlink()
don't set the flag, so a reservation failure will return -EDQUOT for
project quota reservation failures rather than -ENOSPC for these sorts
of operations, even for project quota:

# mkdir mnt/project
# xfs_quota -x -c "project -s -p mnt/project 42" mnt
# xfs_quota -x -c 'limit -p isoft=2 ihard=3 42' mnt
# touch mnt/project/file{1,2,3}
touch: cannot touch ‘mnt/project/file3’: Disk quota exceeded

We can make this consistent by not requiring the flag to be set at the
top of the callchain; instead we can simply test whether we are
reserving a project quota with XFS_QM_ISPDQ in xfs_trans_dqresv and if
so, return -ENOSPC for that failure.  This removes the need for the
XFS_QMOPT_ENOSPC altogether and simplifies the code a fair bit.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
fs/xfs/libxfs/xfs_quota_defs.h
fs/xfs/xfs_qm.c
fs/xfs/xfs_trans_dquot.c

index b2113b1..56d9dd7 100644 (file)
@@ -100,7 +100,6 @@ typedef uint16_t    xfs_qwarncnt_t;
 #define XFS_QMOPT_FORCE_RES    0x0000010 /* ignore quota limits */
 #define XFS_QMOPT_SBVERSION    0x0000040 /* change superblock version num */
 #define XFS_QMOPT_GQUOTA       0x0002000 /* group dquot requested */
-#define XFS_QMOPT_ENOSPC       0x0004000 /* enospc instead of edquot (prj) */
 
 /*
  * flags to xfs_trans_mod_dquot to indicate which field needs to be
index fc93f88..27a9076 100644 (file)
@@ -1808,7 +1808,7 @@ xfs_qm_vop_chown_reserve(
 {
        struct xfs_mount        *mp = ip->i_mount;
        uint64_t                delblks;
-       unsigned int            blkflags, prjflags = 0;
+       unsigned int            blkflags;
        struct xfs_dquot        *udq_unres = NULL;
        struct xfs_dquot        *gdq_unres = NULL;
        struct xfs_dquot        *pdq_unres = NULL;
@@ -1849,7 +1849,6 @@ xfs_qm_vop_chown_reserve(
 
        if (XFS_IS_PQUOTA_ON(ip->i_mount) && pdqp &&
            ip->i_d.di_projid != be32_to_cpu(pdqp->q_core.d_id)) {
-               prjflags = XFS_QMOPT_ENOSPC;
                pdq_delblks = pdqp;
                if (delblks) {
                        ASSERT(ip->i_pdquot);
@@ -1859,8 +1858,7 @@ xfs_qm_vop_chown_reserve(
 
        error = xfs_trans_reserve_quota_bydquots(tp, ip->i_mount,
                                udq_delblks, gdq_delblks, pdq_delblks,
-                               ip->i_d.di_nblocks, 1,
-                               flags | blkflags | prjflags);
+                               ip->i_d.di_nblocks, 1, flags | blkflags);
        if (error)
                return error;
 
@@ -1878,8 +1876,7 @@ xfs_qm_vop_chown_reserve(
                ASSERT(udq_unres || gdq_unres || pdq_unres);
                error = xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
                            udq_delblks, gdq_delblks, pdq_delblks,
-                           (xfs_qcnt_t)delblks, 0,
-                           flags | blkflags | prjflags);
+                           (xfs_qcnt_t)delblks, 0, flags | blkflags);
                if (error)
                        return error;
                xfs_trans_reserve_quota_bydquots(NULL, ip->i_mount,
index 2c3557a..2c07897 100644 (file)
@@ -711,7 +711,7 @@ xfs_trans_dqresv(
 
 error_return:
        xfs_dqunlock(dqp);
-       if (flags & XFS_QMOPT_ENOSPC)
+       if (XFS_QM_ISPDQ(dqp))
                return -ENOSPC;
        return -EDQUOT;
 }
@@ -751,15 +751,13 @@ xfs_trans_reserve_quota_bydquots(
        ASSERT(flags & XFS_QMOPT_RESBLK_MASK);
 
        if (udqp) {
-               error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos,
-                                       (flags & ~XFS_QMOPT_ENOSPC));
+               error = xfs_trans_dqresv(tp, mp, udqp, nblks, ninos, flags);
                if (error)
                        return error;
        }
 
        if (gdqp) {
-               error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos,
-                                       (flags & ~XFS_QMOPT_ENOSPC));
+               error = xfs_trans_dqresv(tp, mp, gdqp, nblks, ninos, flags);
                if (error)
                        goto unwind_usr;
        }
@@ -804,16 +802,12 @@ xfs_trans_reserve_quota_nblks(
 
        if (!XFS_IS_QUOTA_RUNNING(mp) || !XFS_IS_QUOTA_ON(mp))
                return 0;
-       if (XFS_IS_PQUOTA_ON(mp))
-               flags |= XFS_QMOPT_ENOSPC;
 
        ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino));
 
        ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-       ASSERT((flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
-                               XFS_TRANS_DQ_RES_RTBLKS ||
-              (flags & ~(XFS_QMOPT_FORCE_RES | XFS_QMOPT_ENOSPC)) ==
-                               XFS_TRANS_DQ_RES_BLKS);
+       ASSERT((flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_RTBLKS ||
+              (flags & ~(XFS_QMOPT_FORCE_RES)) == XFS_TRANS_DQ_RES_BLKS);
 
        /*
         * Reserve nblks against these dquots, with trans as the mediator.