Re: [PATCH 1/2] common/rc: Enable _format_swapfile to return the swap size

From: Lukas Czerner <lczerner@redhat.com>
To: Carlos Maiolino <cmaiolino@redhat.com>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH 1/2] common/rc: Enable _format_swapfile to return the swap size
Date: Thu, 4 Nov 2021 13:52:26 +0100	[thread overview]
Message-ID: <20211104125226.7kozp45e2ifvmiyw@work> (raw)
In-Reply-To: <20211102152828.26895-2-cmaiolino@redhat.com>

On Tue, Nov 02, 2021 at 04:28:27PM +0100, Carlos Maiolino wrote:
> Once the kernel is free to not map the full swap file during a swapon call,
> it can be useful to know the exact size of the swap area created during
> _format_swapfile().
> 
> To achieve this, it is required to change _require_scratch_swapfile(), to drop
> the _format_swapfile() return value, otherwise, it will also have a return value
> that will end up in tests outputs causing tests to fail.
> 
> Tests using _format_swapfile() that do not require the swap file size do not
> need to be modified, as the return value will be simply ignored.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  common/rc | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 7f693d39..fb1f32e0 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2587,6 +2587,7 @@ _require_odirect()
>  _format_swapfile() {
>  	local fname="$1"
>  	local sz="$2"
> +	local swap_log=""
>  
>  	rm -f "$fname"
>  	touch "$fname"
> @@ -2595,8 +2596,11 @@ _format_swapfile() {
>  	$CHATTR_PROG +C "$fname" > /dev/null 2>&1
>  	_pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full
>  	# Ignore permission complaints on filesystems that don't support perms
> -	$MKSWAP_PROG "$fname" 2>&1 >> $seqres.full | \
> -		grep -v "insecure permission"
> +	swap_log=$($MKSWAP_PROG "$fname" 2>&1 | grep -v "insecure permission")
> +	echo $swap_log >> $seqres.full
> +
> +	# return created swap size
> +	echo $swap_log | grep -oP '(?<=size = )\w+'

I think you need take into account the fact that the units of the mkswap
output will change depedning on the actual size. It might be better to
grab the size in bytes which is in the output as well.

-Lukas

>  }
>  
>  _swapon_file() {
> @@ -2628,7 +2632,7 @@ _require_scratch_swapfile()
>  	_scratch_mount
>  
>  	# Minimum size for mkswap is 10 pages
> -	_format_swapfile "$SCRATCH_MNT/swap" $(($(get_page_size) * 10))
> +	_format_swapfile "$SCRATCH_MNT/swap" $(($(get_page_size) * 10)) > /dev/null
>  
>  	# ext* has supported all variants of swap files since their
>  	# introduction, so swapon should not fail.
> -- 
> 2.31.1
> 