Re: [PATCH] ceph/005: verify correct statfs behaviour with quotas

["Luís Henriques" <lhenriques@suse.de>]

Zorro Lang <zlang@redhat.com> writes:

> On Wed, Apr 27, 2022 at 03:34:09PM +0100, Luís Henriques wrote:
>> When using a directory with 'max_bytes' quota as a base for a mount,
>> statfs shall use that 'max_bytes' value as the total disk size.  That
>> value shall be used even when using subdirectory as base for the mount.
>> 
>> A bug was found where, when this subdirectory also had a 'max_files'
>> quota, the real filesystem size would be returned instead of the parent
>> 'max_bytes' quota value.  This test case verifies this bug is fixed.
>> 
>> Signed-off-by: Luís Henriques <lhenriques@suse.de>
>> ---
>>  tests/ceph/005     | 40 ++++++++++++++++++++++++++++++++++++++++
>>  tests/ceph/005.out |  2 ++
>>  2 files changed, 42 insertions(+)
>>  create mode 100755 tests/ceph/005
>>  create mode 100644 tests/ceph/005.out
>> 
>> diff --git a/tests/ceph/005 b/tests/ceph/005
>> new file mode 100755
>> index 000000000000..0763a235a677
>> --- /dev/null
>> +++ b/tests/ceph/005
>> @@ -0,0 +1,40 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
>> +#
>> +# FS QA Test 005
>> +#
>> +# Make sure statfs reports correct total size when:
>> +# 1. using a directory with 'max_byte' quota as base for a mount
>> +# 2. using a subdirectory of the above directory with 'max_files' quota
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick quota
>> +
>> +_supported_fs generic
>
> As this case name is ceph/005, so I suppose you'd like to support 'ceph' only.

Yep, my mistake, sorry.  I'll fix it in next rev.

>> +_require_scratch
>> +
>> +_scratch_mount
>> +mkdir -p $SCRATCH_MNT/quota-dir/subdir
>> +
>> +# set quotas
>> +quota=$((1024*10000))
>> +$SETFATTR_PROG -n ceph.quota.max_bytes -v $quota $SCRATCH_MNT/quota-dir
>> +$SETFATTR_PROG -n ceph.quota.max_files -v $quota $SCRATCH_MNT/quota-dir/subdir
>> +_scratch_unmount
>> +
>> +SCRATCH_DEV=$SCRATCH_DEV/quota-dir _scratch_mount
>
> Try to not use SCRATCH_DEV like this.

I used this because I saw other tests doing something similar.  Basically,
I need to remount a filesystem with a different base directory.  Changing
the SCRATCH_DEV looked like a simple solution.

>> +total=`df -kP $SCRATCH_MNT | grep -v Filesystem | awk '{print $2}'`
>           ^^ $DF_PROG
>
> As we have _get_total_inode(), _get_used_inode(), _get_used_inode_percent(),
> _get_free_inode() and _get_available_space() in common/rc, I don't mind add
> one more:
>
> _get_total_space()
> {
> 	if [ -z "$1" ]; then
> 		echo "Usage: _get_total_space <mnt>"
> 		exit 1
> 	fi
> 	local total_kb;
> 	total_kb=`$DF_PROG $1 | tail -n1 | awk '{ print $2 }'`
> 	echo $((total_kb * 1024))
> }

Right, this makes sense.

>> +SCRATCH_DEV=$SCRATCH_DEV/quota-dir _scratch_unmount
>> +[ $total -eq 8192 ] || _fail "Incorrect statfs for quota-dir: $total"
>                 ^^^^
> I'm not familar with ceph, I just found "quota=$((1024*10000))" in this case,
> didn't find any place metioned 8192. So may you help to demystify why we expect
> "8192" at here?
>
> And if "8192" is a fixed expected number at here, then we can print it directly,
> as golden image, see below ...

Hmm... OK, I'm struggling to remember the details about this, and it was
only a month ago I wrote this test!  Which is a sign that I should have,
at least, added a comment explaining this value.

I'll need to dig into the statfs code again to explain why we're setting
quotas to 10M and 'df' shows 8M (which is the default size for 2 ceph
objects btw).  I'll revisit this test in the next few days and sort this
mystery.

Thanks a lot for your review.

Cheers
-- 
Luís

>> +
>> +SCRATCH_DEV=$SCRATCH_DEV/quota-dir/subdir _scratch_mount
>> +total=`df -kP $SCRATCH_MNT | grep -v Filesystem | awk '{print $2}'`
>> +SCRATCH_DEV=$SCRATCH_DEV/quota-dir/subdir _scratch_unmount
>> +[ $total -eq 8192 ] || _fail "Incorrect statfs for quota-dir/subdir: $total"
>
> May below code helps?
>
> _require_test
>
> localdir=$TEST_DIR/ceph-quota-dir-$seq
> rm -rf $localdir
> mkdir -p ${localdir}/subdir
> ...
> $SETFATTR_PROG -n ceph.quota.max_bytes -v $quota $localdir
> $SETFATTR_PROG -n ceph.quota.max_bytes -v $quota $localdir/subdir
> ...
>
> SCRATCH_DEV=$localdir _scratch_mount
> echo ceph quota size is $(_get_total_space $SCRATCH_MNT)
> SCRATCH_DEV=$localdir _scratch_unmount
>
> SCRATCH_DEV=$localdir/subdir _scratch_mount
> echo subdir ceph quota size is $(_get_total_space $SCRATCH_MNT)
> SCRATCH_DEV=$localdir/subdir _scratch_unmount
>
> Thanks,
> Zorro
>
>> +
>> +echo "Silence is golden"
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/ceph/005.out b/tests/ceph/005.out
>> new file mode 100644
>> index 000000000000..a5027f127cf0
>> --- /dev/null
>> +++ b/tests/ceph/005.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 005
>> +Silence is golden
>> 
>