Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 9 Feb 2022 14:31:24 -0800
From:   "Darrick J. Wong" <djwong@kernel.org>
To:     Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Cc:     fstests@vger.kernel.org, linux-btrfs@vger.kernel.org,
        linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org,
        Naohiro Aota <naohiro.aota@wdc.com>,
        Johannes Thumshirn <johannes.thumshirn@wdc.com>,
        Damien Le Moal <damien.lemoal@opensource.wdc.com>
Subject: Re: [PATCH v2 2/6] generic/204: remove unnecessary _scratch_mkfs call
Message-ID: <20220209223124.GE8313@magnolia>
References: <20220209123305.253038-1-shinichiro.kawasaki@wdc.com>
 <20220209123305.253038-3-shinichiro.kawasaki@wdc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220209123305.253038-3-shinichiro.kawasaki@wdc.com>
Precedence: bulk

On Wed, Feb 09, 2022 at 09:33:01PM +0900, Shin'ichiro Kawasaki wrote:
> The test case generic/204 calls _scratch_mkfs to get data block size and
> i-node size of the filesystem and obtained data block size is passed to
> the following _scratch_mfks_sized call as an option. However, the
> _scratch_mkfs call is unnecessary since the sizes can be obtained by
> _scratch_mkfs_sized call without the data block size option.
> 
> Also the _scratch_mkfs call is harmful when the _scratch_mkfs succeeds
> and the _scratch_mkfs_sized fails. In this case, the _scratch_mkfs
> leaves valid working filesystem on scratch device then following mount
> and IO operations can not detect the failure of _scratch_mkfs_sized.
> This results in the test case run with unexpected test condition.
> 
> Hence, remove the _scratch_mkfs call and the data block size option for
> _scratch_mkfs_sized call.
> 
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Looks ok, assuming you've verified that fstests with FSTYP=xfs doesn't
regress...

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

--D

> ---
>  tests/generic/204 | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/tests/generic/204 b/tests/generic/204
> index a3dabb71..a33a090f 100755
> --- a/tests/generic/204
> +++ b/tests/generic/204
> @@ -24,10 +24,6 @@ _supported_fs generic
>  
>  _require_scratch
>  
> -# get the block size first
> -_scratch_mkfs 2> /dev/null | _filter_mkfs 2> $tmp.mkfs > /dev/null
> -. $tmp.mkfs
> -
>  # For xfs, we need to handle the different default log sizes that different
>  # versions of mkfs create. All should be valid with a 16MB log, so use that.
>  # And v4/512 v5/1k xfs don't have enough free inodes, set imaxpct=50 at mkfs
> @@ -35,7 +31,7 @@ _scratch_mkfs 2> /dev/null | _filter_mkfs 2> $tmp.mkfs > /dev/null
>  [ $FSTYP = "xfs" ] && MKFS_OPTIONS="$MKFS_OPTIONS -l size=16m -i maxpct=50"
>  
>  SIZE=`expr 115 \* 1024 \* 1024`
> -_scratch_mkfs_sized $SIZE $dbsize 2> /dev/null > $tmp.mkfs.raw
> +_scratch_mkfs_sized $SIZE 2> /dev/null > $tmp.mkfs.raw
>  cat $tmp.mkfs.raw | _filter_mkfs 2> $tmp.mkfs > /dev/null
>  _scratch_mount
>  
> -- 
> 2.34.1
>