Re: The question of Q_XQUOTARM ioctl

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

On Mon, Jan 03, 2022 at 11:21:07PM -0800, Christoph Hellwig wrote:
> On Mon, Jan 03, 2022 at 06:34:56PM -0800, Darrick J. Wong wrote:
> > > 
> > > I don't know the right intention for Q_XQUOTARM now. Can you give me
> > > some advise? Or, we should remove Q_XQUOTARM ioctl and
> > > xfs_qm_scall_trunc_qfile code.
> > 
> > I think xfs_qm_scall_trunc_qfiles probably should be doing:
> > 
> > 	if (xfs_has_quota(mp) || flags == 0 ||
> > 	    (flags & ~XFS_QMOPT_QUOTALL)) {
> > 		xfs_debug(...);
> > 		return -EINVAL;
> > 	}
> > 
> > Note the inversion in the has_quota test.  That would make it so that
> > you can truncate the quota files if quota is not on.

NAK, that's wrong.  xfs_has_quota tells us if the superblock feature bit
is set.  The feature bit guards the sb_[ugp]uotino fields, so the above
code causes us to bail out with EINVAL if the filesystem doesn't have
quota inodes at all.  Thus, inverting the check (to make it so that we
only try to truncate if the fields are garbage) is not correct.

> Yes, that sounds reasonable.  Although I'd split the xfs_has_quota
> file into a separate check with a separate debug message.

So I think the fix here is to fix the testcases.  xfs/220 becomes:

# turn off quota accounting...
$XFS_QUOTA_PROG -x -c off $SCRATCH_DEV

# ...but if the kernel doesn't support turning off accounting, remount with
# noquota option to turn it off...
if $XFS_QUOTA_PROG -x -c 'state -u' $SCRATCH_DEV | grep -q 'Accounting: ON'; then
	_scratch_unmount
	_scratch_mount -o noquota
fi

# ...and remove space allocated to the quota files
# (this used to give wrong ENOSYS returns in 2.6.31)
$XFS_QUOTA_PROG -x -c remove $SCRATCH_DEV


--D