Re: [PATCH V3 1/4] xfs/270: Fix ro mount failure when nrext64 option is enabled

From: "Darrick J. Wong" <djwong@kernel.org>
To: Chandan Babu R <chandan.babu@oracle.com>
Cc: fstests@vger.kernel.org, zlang@kernel.org, david@fromorbit.com,
	linux-xfs@vger.kernel.org
Subject: Re: [PATCH V3 1/4] xfs/270: Fix ro mount failure when nrext64 option is enabled
Date: Wed, 22 Jun 2022 10:03:21 -0700	[thread overview]
Message-ID: <YrNLWX0gVnl/EYTD@magnolia> (raw)
In-Reply-To: <20220611111037.433134-2-chandan.babu@oracle.com>

On Sat, Jun 11, 2022 at 04:40:34PM +0530, Chandan Babu R wrote:
> With nrext64 option enabled at run time, the read-only mount performed by the
> test fails because,
> 1. mkfs.xfs would have calculated log size based on reflink being enabled.
> 2. Clearing the reflink ro compat bit causes log size calculations to yield a
>    different value.
> 3. In the case where nrext64 is enabled, this causes attr reservation to be
>    the largest among all the transaction reservations.
> 4. This ends up causing XFS to require a larger ondisk log size than that
>    which is available.
> 
> This commit fixes the problem by setting features_ro_compat to the value
> obtained by the bitwise-OR of features_ro_compat field with 2^31.
> 
> This commit includes changes suggested by Dave Chinner to replace bashisms
> with invocations to inline awk scripts.
> 
> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> ---
>  tests/xfs/270     | 26 ++++++++++++++++++++++++--
>  tests/xfs/270.out |  1 -
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/xfs/270 b/tests/xfs/270
> index 0ab0c7d8..b740c379 100755
> --- a/tests/xfs/270
> +++ b/tests/xfs/270
> @@ -27,8 +27,30 @@ _scratch_mkfs_xfs >>$seqres.full 2>&1
>  # set the highest bit of features_ro_compat, use it as an unknown
>  # feature bit. If one day this bit become known feature, please
>  # change this case.
> -_scratch_xfs_set_metadata_field "features_ro_compat" "$((2**31))" "sb 0" | \
> -	grep 'features_ro_compat'
> +
> +ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0")
> +echo $ro_compat | grep -q -E '^0x[[:xdigit:]]$'
> +if [[ $? != 0  ]]; then
> +	echo "features_ro_compat has an invalid value."
> +fi
> +
> +ro_compat=$(echo $ro_compat | \
> +		    awk '/^0x[[:xdigit:]]+/ {

I think the double-braces around :xdigit: were the only changes between
v2 and v3, right?

If so,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> +				printf("0x%x\n", or(strtonum($1), 0x80000000))
> +			}')
> +
> +# write the new ro compat field to the superblock
> +_scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \
> +				> $seqres.full 2>&1
> +
> +# read the newly set ro compat filed for verification
> +new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \
> +						2>/dev/null)
> +
> +# verify the new ro_compat field is correct.
> +if [ $new_ro_compat != $ro_compat ]; then
> +	echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat"
> +fi
>  
>  # rw mount with unknown ro-compat feature should fail
>  echo "rw mount test"
> diff --git a/tests/xfs/270.out b/tests/xfs/270.out
> index 0a8b3851..edf4c254 100644
> --- a/tests/xfs/270.out
> +++ b/tests/xfs/270.out
> @@ -1,5 +1,4 @@
>  QA output created by 270
> -features_ro_compat = 0x80000000
>  rw mount test
>  ro mount test
>  rw remount test
> -- 
> 2.35.1
> 