Re: [PATCH v3 1/2] generic: soft quota limits testing within grace time

From: "Darrick J. Wong" <djwong@kernel.org>
To: fstests@vger.kernel.org
Subject: Re: [PATCH v3 1/2] generic: soft quota limits testing within grace time
Date: Thu, 12 May 2022 17:58:14 -0700	[thread overview]
Message-ID: <20220513005814.GE27174@magnolia> (raw)
In-Reply-To: <20220512201651.yvi3hiwkln3dgojs@zlang-mailbox>

On Fri, May 13, 2022 at 04:16:51AM +0800, Zorro Lang wrote:
> On Thu, May 12, 2022 at 12:06:47PM -0700, Darrick J. Wong wrote:
> > On Fri, May 13, 2022 at 02:22:02AM +0800, Zorro Lang wrote:
> > > After soft limits are exceeded, within the grace time, fs quota
> > > should allow more space allocation before exceeding hard limits,
> > > even if allocating many small files.
> > > 
> > > This case can cover bc37e4fb5cac (xfs: revert "xfs: actually bump
> > > warning counts when we send warnings"). And will help to expose
> > > later behavior changes on this side.
> > > 
> > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > > ---
> > > 
> > > According to the review points of:
> > >   https://lore.kernel.org/fstests/20220511163806.uc4z7td2remhdru3@zlang-mailbox/T/#m23940b47261165a0442c7ae1eb4d96a1489a2935
> > > V3 did below changes:
> > > 1) fix some comments description
> > > 2) add a few hard limit test
> > > 3) And bring in [PATCH 2/2], to help g/603 to use the common functions of this
> > >    patch.
> > > 
> > > Thanks,
> > > Zorro
> > > 
> > >  common/quota          | 48 +++++++++++++++++++++
> > >  tests/generic/999     | 99 +++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/999.out |  2 +
> > >  3 files changed, 149 insertions(+)
> > >  create mode 100755 tests/generic/999
> > >  create mode 100644 tests/generic/999.out
> > > 
> 
> [snip]
> 
> > > +
> > > +	setquota -${type} $qa_user 1M 200M 0 0 $SCRATCH_MNT
> > > +	setquota -${type} -t 86400 86400 $SCRATCH_MNT
> > > +	repquota -v -${type} $SCRATCH_MNT | grep -v "^root" >>$seqres.full 2>&1
> > > +	# Exceed the soft quota limit a bit at first
> > > +	su $qa_user -c "$XFS_IO_PROG -f -t -c 'pwrite 0 2m' -c fsync ${file}.0" >>$seqres.full 2>&1
> > > +	# Write more data more times under soft quota limit exhausted condition,
> > > +	# but not reach hard limit. To make sure each write won't trigger EDQUOT.
> > > +	for ((i=1; i<=100; i++));do
> > > +		su "$qa_user" -c "$XFS_IO_PROG -f -c 'pwrite 0 1m' -c fsync ${file}.$i" >>$seqres.full 2>&1
> > 
> > Hmm -- if pwrite or fsync hit an error, shouldn't we let that end up in
> > 999.out so that the golden output diff fails?
> 
> Sure, will remove the "2>&1".
> 
> > 
> > > +		if [ $? -ne 0 ];then
> > > +			echo "Unexpected error (type=$type)!"
> > > +			break
> > > +		fi
> > > +	done
> > > +	repquota -v -${type} $SCRATCH_MNT | grep -v "^root" >>$seqres.full 2>&1
> > > +
> > > +	# As we've tested soft limit, now exceed the hard limit and give it a
> > > +	# test in passing.
> > > +	su $qa_user -c "$XFS_IO_PROG -f -t -c 'pwrite 0 100m' -c fsync ${file}.hard.0" >>$seqres.full 2>&1
> > > +	for ((i=1; i<=10; i++));do
> > > +		su "$qa_user" -c "$XFS_IO_PROG -f -c 'pwrite 0 1m' -c fsync ${file}.hard.$i" >>$seqres.full 2>&1
> > 
> > Don't we want to encode the actual error in the golden output?
> 
> Hmm... due to I have to filter the ENOSPC or EDQUOT output again for project
> quota, as I did in generic/603 :-P Or put the that function into common/quota
> too.

I think it'd be fine if the golden output contained:

pwrite: Insufficient quota
pwrite: Insufficient quota
pwrite: Not enough space

for each of the user/group/project quota tests.  Or did I misunderstand?

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > +		if [ $? -eq 0 ];then
> > > +			echo "Unexpected success (type=$type)!"
> > > +			break
> > > +		fi
> > > +	done
> > > +}
> > > +
> > > +_qmount_option "usrquota"
> > > +exercise u
> > > +_qmount_option "grpquota"
> > > +exercise g
> > > +_qmount_option "prjquota"
> > > +exercise P
> > > +
> > > +echo "Silence is golden"
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/999.out b/tests/generic/999.out
> > > new file mode 100644
> > > index 00000000..3b276ca8
> > > --- /dev/null
> > > +++ b/tests/generic/999.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 999
> > > +Silence is golden
> > > -- 
> > > 2.31.1
> > > 
> > 