Re: [PATCH] generic: use _qmount_option and _qmount

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Dec 03, 2025 at 11:12:09AM +0800, Su Yue wrote:
> On Tue 02 Dec 2025 at 12:17, Zorro Lang <zlang@redhat.com> wrote:
> 
> > On Mon, Dec 01, 2025 at 02:59:24PM -0800, Darrick J. Wong wrote:
> > > On Mon, Nov 24, 2025 at 09:20:04PM +0800, Su Yue wrote:
> > > > Many generic tests call `_scratch_mount -o usrquota` then
> > > > chmod 777, quotacheck and quotaon.
> > > 
> > > What does the chmod 777 do, in relation to the quota{check,on}
> > > programs?
> > > Is it necessary for the root dir to be world writable (and
> > > executable!)
> > > for quota tools to work?
> > 
> > Hi Darrick,
> > 
> > The _qmount always does "chmod ugo+rwx $SCRATCH_MNT", I think it's for
> > regular
> > users (e.g. fsgqa) can write into $SCRATCH_MNT. If a test case doesn't
> > use
> > regular user, the "chmod 777" isn't necessary. The author of _qmount
> > might
> > think "chmod 777 $SCRATCH_MN" doesn't hurt anything quota test, so
> > always
> > does that no matter if it's needed.
> > 
> 
> I guess so. The code cames from 2001.
> 
> > If you think it hurts something, we can
> > only do that for the cases who really need that :)
> > 
> Yeah. We can remove the "chmod 777 $SCRATCH_MNT" from _qmount and adjust
> caeses influenced.
> 
> > I think this patch trys to help quota test cases to use unified common
> > helpers,
> > most of them don't need the "chmod 777" actually, right Su?
> > 
> 
> Correct. To be more accurate, 4 cases need "chmod 777".

Ok, yeah, let's only call chmod 0777 for the tests that need it.

--D

> --
> Su
> 
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > > It can be simpilfied to _qmount_option and _qmount. The later
> > > > function already calls quotacheck, quota and chmod.
> > > >
> > > > Convertaions can save a few lines. tests/generic/380 is an >
> > > exception
> > > 
> > > "Conversions" ?
> > > 
> > > --D
> > > 
> > > > because it tests chown.
> > > >
> > > > Signed-off-by: Su Yue <glass.su@suse.com>
> > > > ---
> > > >  tests/generic/082 |  9 ++-------
> > > >  tests/generic/219 | 11 ++++-------
> > > >  tests/generic/230 | 11 ++++++-----
> > > >  tests/generic/231 |  6 ++----
> > > >  tests/generic/232 |  6 ++----
> > > >  tests/generic/233 |  6 ++----
> > > >  tests/generic/234 |  5 ++---
> > > >  tests/generic/235 |  5 ++---
> > > >  tests/generic/244 |  1 -
> > > >  tests/generic/270 |  6 ++----
> > > >  tests/generic/280 |  5 ++---
> > > >  tests/generic/400 |  2 +-
> > > >  12 files changed, 27 insertions(+), 46 deletions(-)
> > > >
> > > > diff --git a/tests/generic/082 b/tests/generic/082
> > > > index f078ef2ffff9..6bb9cf2a22ae 100755
> > > > --- a/tests/generic/082
> > > > +++ b/tests/generic/082
> > > > @@ -23,13 +23,8 @@ _require_scratch
> > > >  _require_quota
> > > >
> > > >  _scratch_mkfs >>$seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -
> > > > -# xfs doesn't need these setups and quotacheck even fails on >
> > > xfs, so just
> > > > -# redirect the output to $seqres.full for debug purpose and >
> > > ignore the results,
> > > > -# as we check the quota status later anyway.
> > > > -quotacheck -ug $SCRATCH_MNT >>$seqres.full 2>&1
> > > > -quotaon $SCRATCH_MNT >>$seqres.full 2>&1
> > > > +_qmount_option 'usrquota,grpquota'
> > > > +_qmount "usrquota,grpquota"
> > > >
> > > >  # first remount ro with a bad option, a failed remount ro >
> > > should not disable
> > > >  # quota, but currently xfs doesn't fail in this case, the >
> > > unknown option is
> > > > diff --git a/tests/generic/219 b/tests/generic/219
> > > > index 642823859886..a2eb0b20f408 100755
> > > > --- a/tests/generic/219
> > > > +++ b/tests/generic/219
> > > > @@ -91,25 +91,22 @@ test_accounting()
> > > >
> > > >  _scratch_unmount 2>/dev/null
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >  _force_vfs_quota_testing $SCRATCH_MNT
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > >  _scratch_unmount
> > > >
> > > >  echo; echo "### test user accounting"
> > > > -export MOUNT_OPTIONS="-o usrquota"
> > > > +_qmount_option "usrquota"
> > > >  _qmount
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > >  type=u
> > > >  test_files
> > > >  test_accounting
> > > >  _scratch_unmount 2>/dev/null
> > > >
> > > >  echo; echo "### test group accounting"
> > > > -export MOUNT_OPTIONS="-o grpquota"
> > > > +_qmount_option "grpquota"
> > > >  _qmount
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > >  type=g
> > > >  test_files
> > > >  test_accounting
> > > > diff --git a/tests/generic/230 b/tests/generic/230
> > > > index a8caf5a808c3..0a680dbc874b 100755
> > > > --- a/tests/generic/230
> > > > +++ b/tests/generic/230
> > > > @@ -99,7 +99,8 @@ grace=2
> > > >  _qmount_option 'defaults'
> > > >
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >  _force_vfs_quota_testing $SCRATCH_MNT
> > > >  BLOCK_SIZE=$(_get_file_block_size $SCRATCH_MNT)
> > > >  quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > @@ -113,8 +114,8 @@ setquota -g -t $grace $grace $SCRATCH_MNT
> > > >  _scratch_unmount
> > > >
> > > >  echo; echo "### test user limit enforcement"
> > > > -_scratch_mount "-o usrquota"
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota"
> > > > +_qmount
> > > >  type=u
> > > >  test_files
> > > >  test_enforcement
> > > > @@ -122,8 +123,8 @@ cleanup_files
> > > >  _scratch_unmount 2>/dev/null
> > > >
> > > >  echo; echo "### test group limit enforcement"
> > > > -_scratch_mount "-o grpquota"
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "grpquota"
> > > > +_qmount
> > > >  type=g
> > > >  test_files
> > > >  test_enforcement
> > > > diff --git a/tests/generic/231 b/tests/generic/231
> > > > index ce7e62ea1886..02910523d0b5 100755
> > > > --- a/tests/generic/231
> > > > +++ b/tests/generic/231
> > > > @@ -47,10 +47,8 @@ _require_quota
> > > >  _require_user
> > > >
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -chmod 777 $SCRATCH_MNT
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon -u -g $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >
> > > >  if ! _fsx 1; then
> > > >  	_scratch_unmount 2>/dev/null
> > > > diff --git a/tests/generic/232 b/tests/generic/232
> > > > index c903a5619045..21375809d299 100755
> > > > --- a/tests/generic/232
> > > > +++ b/tests/generic/232
> > > > @@ -44,10 +44,8 @@ _require_scratch
> > > >  _require_quota
> > > >
> > > >  _scratch_mkfs > $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -chmod 777 $SCRATCH_MNT
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon -u -g $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >
> > > >  _fsstress
> > > >  _check_quota_usage
> > > > diff --git a/tests/generic/233 b/tests/generic/233
> > > > index 3fc1b63abb24..4606f3bde2ab 100755
> > > > --- a/tests/generic/233
> > > > +++ b/tests/generic/233
> > > > @@ -59,10 +59,8 @@ _require_quota
> > > >  _require_user
> > > >
> > > >  _scratch_mkfs > $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -chmod 777 $SCRATCH_MNT
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon -u -g $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >  setquota -u $qa_user 32000 32000 1000 1000 $SCRATCH_MNT >
> > > 2>/dev/null
> > > >
> > > >  _fsstress
> > > > diff --git a/tests/generic/234 b/tests/generic/234
> > > > index 4b25fc6507cc..2c596492a3e0 100755
> > > > --- a/tests/generic/234
> > > > +++ b/tests/generic/234
> > > > @@ -66,9 +66,8 @@ _require_quota
> > > >
> > > >
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon -u -g $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >  test_setting
> > > >  _scratch_unmount
> > > >
> > > > diff --git a/tests/generic/235 b/tests/generic/235
> > > > index 037c29e806db..7a616650fc8f 100755
> > > > --- a/tests/generic/235
> > > > +++ b/tests/generic/235
> > > > @@ -25,9 +25,8 @@ do_repquota()
> > > >
> > > >
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >
> > > >  touch $SCRATCH_MNT/testfile
> > > >  chown $qa_user:$qa_user $SCRATCH_MNT/testfile
> > > > diff --git a/tests/generic/244 b/tests/generic/244
> > > > index b68035129c82..989bb4f5385e 100755
> > > > --- a/tests/generic/244
> > > > +++ b/tests/generic/244
> > > > @@ -66,7 +66,6 @@ done
> > > >  # remount just for kicks, make sure we get it off disk
> > > >  _scratch_unmount
> > > >  _qmount
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > >
> > > >  # Read them back by iterating based on quotas returned.
> > > >  # This should match what we set, even if we don't directly
> > > > diff --git a/tests/generic/270 b/tests/generic/270
> > > > index c3d5127a0b51..9ac829a7379f 100755
> > > > --- a/tests/generic/270
> > > > +++ b/tests/generic/270
> > > > @@ -62,10 +62,8 @@ _require_command "$SETCAP_PROG" setcap
> > > >  _require_attrs security
> > > >
> > > >  _scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full >
> > > 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -chmod 777 $SCRATCH_MNT
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon -u -g $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >
> > > >  if ! _workout; then
> > > >  	_scratch_unmount 2>/dev/null
> > > > diff --git a/tests/generic/280 b/tests/generic/280
> > > > index 3108fd23fb70..fae0a02145cf 100755
> > > > --- a/tests/generic/280
> > > > +++ b/tests/generic/280
> > > > @@ -34,9 +34,8 @@ _require_freeze
> > > >
> > > >  _scratch_unmount 2>/dev/null
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > > -_scratch_mount "-o usrquota,grpquota"
> > > > -quotacheck -u -g $SCRATCH_MNT 2>/dev/null
> > > > -quotaon $SCRATCH_MNT 2>/dev/null
> > > > +_qmount_option "usrquota,grpquota"
> > > > +_qmount
> > > >  xfs_freeze -f $SCRATCH_MNT
> > > >  setquota -u root 1 2 3 4 $SCRATCH_MNT &
> > > >  pid=$!
> > > > diff --git a/tests/generic/400 b/tests/generic/400
> > > > index 77970da69a41..ef27c254167c 100755
> > > > --- a/tests/generic/400
> > > > +++ b/tests/generic/400
> > > > @@ -22,7 +22,7 @@ _require_scratch
> > > >
> > > >  _scratch_mkfs >> $seqres.full 2>&1
> > > >
> > > > -MOUNT_OPTIONS="-o usrquota,grpquota"
> > > > +_qmount_option "usrquota,grpquota"
> > > >  _qmount
> > > >  _require_getnextquota
> > > >
> > > > --
> > > > 2.48.1
> > > >
> > > >
> > > 
>