On Thu, Feb 04, 2021 at 12:01:16AM +0800, Zorro Lang wrote:
> On Tue, Feb 02, 2021 at 11:41:01AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > This is a regression test for a bug in the XFS implementation of
> > FSSETXATTR. When we try to change a file's project id, the quota
> > reservation code will update the incore quota reservations for delayed
> > allocation blocks. Unfortunately, it does this before we finish
> > validating all the FSSETXATTR parameters, which means that if we decide
> > to bail out, we also fail to undo the incore changes.
> >
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > ---
>
> Looks like this patch comes from djwong-devel :) It can't be merged into
> xfstests-dev mainline directly.
Yeah, applying patches to fstests is annoying like that. :(
> Applying: xfs: test delalloc quota leak when chprojid fails
> error: patch failed: src/Makefile:29
> error: src/Makefile: patch does not apply
> error: patch failed: tests/xfs/group:544
> error: tests/xfs/group: patch does not apply
>
> Anyway, I think Eryu can solve it. I just have another question below.
>
> > .gitignore | 1 +
> > src/Makefile | 3 +-
> > src/chprojid_fail.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/xfs/765 | 63 +++++++++++++++++++++++++++++++++++++
> > tests/xfs/765.out | 3 ++
> > tests/xfs/group | 1 +
> > 6 files changed, 156 insertions(+), 1 deletion(-)
> > create mode 100644 src/chprojid_fail.c
> > create mode 100755 tests/xfs/765
> > create mode 100644 tests/xfs/765.out
> >
>
> [snip]
>
> > +# FS QA Test No. 765
> > +#
> > +# Regression test for failing to undo delalloc quota reservations when changing
> > +# project id but we fail some other part of FSSETXATTR validation. If we fail
> > +# the test, we trip debugging assertions in dmesg.
> > +#
> > +# The appropriate XFS patch is:
> > +# xfs: fix chown leaking delalloc quota blocks when fssetxattr fails
> > +
> > +seq=`basename $0`
> > +seqres=$RESULT_DIR/$seq
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> > +tmp=/tmp/$$
> > +status=1 # failure is the default!
> > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > +
> > +_cleanup()
> > +{
> > + cd /
> > + rm -f $tmp.*
> > +}
> > +
> > +# get standard environment, filters and checks
> > +. ./common/rc
> > +. ./common/quota
> > +
> > +# real QA test starts here
> > +_supported_fs xfs
> > +_require_xfs_debug
>
> Why CONFIG_XFS_DEBUG=y is necessary for this case? Maybe I miss something, but
> I didn't see any debug injection in this case. If a kernel is only built with
> CONFIG_XFS_WARN=y, I still can reproduce this bug [1].
Heh, I forgot that asserts can trigger even if debug is disabled.
> Even if the XFS_WARN and XFS_DEUBG are all unset, I think this case can be run
> without any failures. So why not just let it run? We could recommand using
> debug xfs kernel, but general kernel don't need to skip this test.
Ok. I'll get rid of it then.
--D
> Thanks,
> Zorro
>
> [1]
> # ./check xfs/765
> FSTYP -- xfs (non-debug)
> PLATFORM -- Linux/x86_64 hp-dl380pg8-01 4.18.0 #1 SMP Sat Jan 23 14:15:14 EST 2021
> MKFS_OPTIONS -- -f -m reflink=1,rmapbt=1 /dev/mapper/xfscratch
> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/mapper/xfscratch /mnt/scratch
>
> xfs/765 _check_dmesg: something found in dmesg (see /home/xfstests-dev/results//xfs/765.dmesg)
>
> Ran: xfs/765
> Failures: xfs/765
> Failed 1 of 1 tests
>
> [root@hp-dl380pg8-01 xfstests-dev]# less /home/xfstests-dev/results//xfs/765.dmesg
> [30020.559403] run fstests xfs/765 at 2021-02-03 10:36:02
> ...
> [30028.411797] XFS: Assertion failed: dqp->q_res_bcount >= be64_to_cpu(dqp->q_core.d_bcount), file: fs/xfs/xfs_trans_dquot.c, line: 471
>
> > +_require_command "$FILEFRAG_PROG" filefrag
> > +_require_test_program "chprojid_fail"
> > +_require_quota
> > +_require_scratch
> > +
> > +rm -f $seqres.full
> > +
> > +echo "Format filesystem" | tee -a $seqres.full
> > +_scratch_mkfs > $seqres.full
> > +_qmount_option 'prjquota'
> > +_qmount
> > +_require_prjquota $SCRATCH_DEV
> > +
> > +echo "Run test program"
> > +$XFS_QUOTA_PROG -f -x -c 'report -ap' $SCRATCH_MNT >> $seqres.full
> > +$here/src/chprojid_fail $SCRATCH_MNT/blah >> $seqres.full
> > +res=$?
> > +if [ $res -ne 0 ]; then
> > + echo "chprojid_fail returned $res, expected 0"
> > +fi
> > +$XFS_QUOTA_PROG -f -x -c 'report -ap' $SCRATCH_MNT >> $seqres.full
> > +$FILEFRAG_PROG -v $SCRATCH_MNT/blah >> $seqres.full
> > +$FILEFRAG_PROG -v $SCRATCH_MNT/blah 2>&1 | grep -q delalloc || \
> > + echo "file didn't get delalloc extents?"
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/xfs/765.out b/tests/xfs/765.out
> > new file mode 100644
> > index 00000000..f44ba43e
> > --- /dev/null
> > +++ b/tests/xfs/765.out
> > @@ -0,0 +1,3 @@
> > +QA output created by 765
> > +Format filesystem
> > +Run test program
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index f406a9b9..fb78b0d7 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -544,6 +544,7 @@
> > 762 auto quick rw scrub realtime
> > 763 auto quick rw realtime
> > 764 auto quick repair
> > +765 auto quick quota
> > 908 auto quick bigtime
> > 909 auto quick bigtime quota
> > 910 auto quick inobtcount
> >
>
