diff mbox series

fstests: btrfs/301: handle auto-removed qgroups

Message ID 20240507070606.64126-1-wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series fstests: btrfs/301: handle auto-removed qgroups | expand

Commit Message

Qu Wenruo May 7, 2024, 7:06 a.m. UTC
There are always attempts to auto-remove empty qgroups after dropping a
subvolume.

For squota mode, not all qgroups can or should be dropped, as there are
common cases where the dropped subvolume are still referred by other
snapshots.
In that case, the numbers can only be freed when the last referencer
got dropped.

The latest kernel attempt would only try to drop empty qgroups for
squota mode.
But even with such safe change, the test case still needs to handle
auto-removed qgroups, by explicitly echoing "0", or later calculation
would break bash grammar.

This patch would add extra handling for such removed qgroups, to be
future proof for qgroup auto-removal behavior change.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/301 | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Boris Burkov May 21, 2024, 1:19 a.m. UTC | #1
On Tue, May 07, 2024 at 04:36:06PM +0930, Qu Wenruo wrote:
> There are always attempts to auto-remove empty qgroups after dropping a
> subvolume.
> 
> For squota mode, not all qgroups can or should be dropped, as there are
> common cases where the dropped subvolume are still referred by other
> snapshots.
> In that case, the numbers can only be freed when the last referencer
> got dropped.
> 
> The latest kernel attempt would only try to drop empty qgroups for
> squota mode.
> But even with such safe change, the test case still needs to handle
> auto-removed qgroups, by explicitly echoing "0", or later calculation
> would break bash grammar.
> 
> This patch would add extra handling for such removed qgroups, to be
> future proof for qgroup auto-removal behavior change.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Looks good, thanks!
Reviewed-by: Boris Burkov <boris@bur.io>

> ---
>  tests/btrfs/301 | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/301 b/tests/btrfs/301
> index db469724..bb18ab04 100755
> --- a/tests/btrfs/301
> +++ b/tests/btrfs/301
> @@ -51,9 +51,17 @@ _require_fio $fio_config
>  get_qgroup_usage()
>  {
>  	local qgroupid=$1
> +	local output
>  
> -	$BTRFS_UTIL_PROG qgroup show --sync --raw $SCRATCH_MNT | \
> -				grep "$qgroupid" | $AWK_PROG '{print $3}'
> +	output=$($BTRFS_UTIL_PROG qgroup show --sync --raw $SCRATCH_MNT | \
> +		 grep "$qgroupid" | $AWK_PROG '{print $3}')
> +	# The qgroup is auto-removed, this can only happen if its numbers are
> +	# already all zeros, so here we only need to explicitly echo "0".
> +	if [ -z "$output" ]; then
> +		echo "0"
> +	else
> +		echo "$output"
> +	fi
>  }
>  
>  get_subvol_usage()
> -- 
> 2.44.0
>
Anand Jain May 23, 2024, 3:43 p.m. UTC | #2
On 07/05/2024 15:06, Qu Wenruo wrote:
> There are always attempts to auto-remove empty qgroups after dropping a
> subvolume.
> 
> For squota mode, not all qgroups can or should be dropped, as there are
> common cases where the dropped subvolume are still referred by other
> snapshots.
> In that case, the numbers can only be freed when the last referencer
> got dropped.
> 
> The latest kernel attempt would only try to drop empty qgroups for
> squota mode.
> But even with such safe change, the test case still needs to handle
> auto-removed qgroups, by explicitly echoing "0", or later calculation
> would break bash grammar.
> 
> This patch would add extra handling for such removed qgroups, to be
> future proof for qgroup auto-removal behavior change.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Applied.

Thanks, Anand

> ---
>   tests/btrfs/301 | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/301 b/tests/btrfs/301
> index db469724..bb18ab04 100755
> --- a/tests/btrfs/301
> +++ b/tests/btrfs/301
> @@ -51,9 +51,17 @@ _require_fio $fio_config
>   get_qgroup_usage()
>   {
>   	local qgroupid=$1
> +	local output
>   
> -	$BTRFS_UTIL_PROG qgroup show --sync --raw $SCRATCH_MNT | \
> -				grep "$qgroupid" | $AWK_PROG '{print $3}'
> +	output=$($BTRFS_UTIL_PROG qgroup show --sync --raw $SCRATCH_MNT | \
> +		 grep "$qgroupid" | $AWK_PROG '{print $3}')
> +	# The qgroup is auto-removed, this can only happen if its numbers are
> +	# already all zeros, so here we only need to explicitly echo "0".
> +	if [ -z "$output" ]; then
> +		echo "0"
> +	else
> +		echo "$output"
> +	fi
>   }
>   
>   get_subvol_usage()
diff mbox series

Patch

diff --git a/tests/btrfs/301 b/tests/btrfs/301
index db469724..bb18ab04 100755
--- a/tests/btrfs/301
+++ b/tests/btrfs/301
@@ -51,9 +51,17 @@  _require_fio $fio_config
 get_qgroup_usage()
 {
 	local qgroupid=$1
+	local output
 
-	$BTRFS_UTIL_PROG qgroup show --sync --raw $SCRATCH_MNT | \
-				grep "$qgroupid" | $AWK_PROG '{print $3}'
+	output=$($BTRFS_UTIL_PROG qgroup show --sync --raw $SCRATCH_MNT | \
+		 grep "$qgroupid" | $AWK_PROG '{print $3}')
+	# The qgroup is auto-removed, this can only happen if its numbers are
+	# already all zeros, so here we only need to explicitly echo "0".
+	if [ -z "$output" ]; then
+		echo "0"
+	else
+		echo "$output"
+	fi
 }
 
 get_subvol_usage()