All of lore.kernel.org
 help / color / mirror / Atom feed
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 2/7] generic/{171,172,173,174,204}: check _scratch_mkfs_sized return code
Date: Tue, 8 Feb 2022 16:35:48 -0800	[thread overview]
Message-ID: <20220209003548.GC8288@magnolia> (raw)
In-Reply-To: <20220207065541.232685-3-shinichiro.kawasaki@wdc.com>

On Mon, Feb 07, 2022 at 03:55:36PM +0900, Shin'ichiro Kawasaki wrote:
> The test cases generic/{171,172,173,174,204} call _scratch_mkfs before
> _scratch_mkfs_sized, and they do not check return code of
> _scratch_mkfs_sized. Even if _scratch_mkfs_sized failed, _scratch_mount
> after it cannot detect the sized mkfs failure because _scratch_mkfs
> already created a file system on the device. This results in unexpected
> test condition of the test cases.
> 
> To avoid the unexpected test condition, check return code of
> _scratch_mkfs_sized in the test cases.
> 
> Suggested-by: Naohiro Aota <naohiro.aota@wdc.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Hm.  I wonder, are there other tests that employ this _scratch_mkfs ->
scratch_mkfs_sized sequence and need patching?

$ git grep -l _scratch_mkfs_sized | while read f; do grep -q
'_scratch_mkfs[[:space:]]' $f && echo $f; done
common/encrypt
common/rc
tests/ext4/021
tests/generic/171
tests/generic/172
tests/generic/173
tests/generic/174
tests/generic/204
tests/generic/520
tests/generic/525
tests/xfs/015

generic/520 is a false positive, and you patched the rest.  OK, good.

I wonder if the maintainer will ask for the _scratch_mkfs_sized in the
failure output, but as far as I'm concerned:

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

--D

> ---
>  tests/generic/171 | 2 +-
>  tests/generic/172 | 2 +-
>  tests/generic/173 | 2 +-
>  tests/generic/174 | 2 +-
>  tests/generic/204 | 3 ++-
>  5 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/generic/171 b/tests/generic/171
> index fb2a6f14..f823a454 100755
> --- a/tests/generic/171
> +++ b/tests/generic/171
> @@ -42,7 +42,7 @@ sz_bytes=$((nr_blks * 8 * blksz))
>  if [ $sz_bytes -lt $((32 * 1048576)) ]; then
>  	sz_bytes=$((32 * 1048576))
>  fi
> -_scratch_mkfs_sized $sz_bytes >> $seqres.full 2>&1
> +_scratch_mkfs_sized $sz_bytes >> $seqres.full 2>&1 || _fail "mkfs failed"
>  _scratch_mount >> $seqres.full 2>&1
>  rm -rf $testdir
>  mkdir $testdir
> diff --git a/tests/generic/172 b/tests/generic/172
> index ab5122fa..383824b9 100755
> --- a/tests/generic/172
> +++ b/tests/generic/172
> @@ -40,7 +40,7 @@ umount $SCRATCH_MNT
>  
>  file_size=$((768 * 1024 * 1024))
>  fs_size=$((1024 * 1024 * 1024))
> -_scratch_mkfs_sized $fs_size >> $seqres.full 2>&1
> +_scratch_mkfs_sized $fs_size >> $seqres.full 2>&1 || _fail "mkfs failed"
>  _scratch_mount >> $seqres.full 2>&1
>  rm -rf $testdir
>  mkdir $testdir
> diff --git a/tests/generic/173 b/tests/generic/173
> index 0eb313e2..e1493278 100755
> --- a/tests/generic/173
> +++ b/tests/generic/173
> @@ -42,7 +42,7 @@ sz_bytes=$((nr_blks * 8 * blksz))
>  if [ $sz_bytes -lt $((32 * 1048576)) ]; then
>  	sz_bytes=$((32 * 1048576))
>  fi
> -_scratch_mkfs_sized $sz_bytes >> $seqres.full 2>&1
> +_scratch_mkfs_sized $sz_bytes >> $seqres.full 2>&1 || _fail "mkfs failed"
>  _scratch_mount >> $seqres.full 2>&1
>  rm -rf $testdir
>  mkdir $testdir
> diff --git a/tests/generic/174 b/tests/generic/174
> index 1505453e..c7a177b8 100755
> --- a/tests/generic/174
> +++ b/tests/generic/174
> @@ -43,7 +43,7 @@ sz_bytes=$((nr_blks * 8 * blksz))
>  if [ $sz_bytes -lt $((32 * 1048576)) ]; then
>  	sz_bytes=$((32 * 1048576))
>  fi
> -_scratch_mkfs_sized $sz_bytes >> $seqres.full 2>&1
> +_scratch_mkfs_sized $sz_bytes >> $seqres.full 2>&1 || _fail "mkfs failed"
>  _scratch_mount >> $seqres.full 2>&1
>  rm -rf $testdir
>  mkdir $testdir
> diff --git a/tests/generic/204 b/tests/generic/204
> index a3dabb71..b5deb443 100755
> --- a/tests/generic/204
> +++ b/tests/generic/204
> @@ -35,7 +35,8 @@ _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 $dbsize 2> /dev/null > $tmp.mkfs.raw \
> +	|| _fail "mkfs failed"
>  cat $tmp.mkfs.raw | _filter_mkfs 2> $tmp.mkfs > /dev/null
>  _scratch_mount
>  
> -- 
> 2.34.1
> 

  reply	other threads:[~2022-02-09  0:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07  6:55 [PATCH 0/7] fstests: fix _scratch_mkfs_sized failure handling Shin'ichiro Kawasaki
2022-02-07  6:55 ` [PATCH 1/7] common/rc: fix btrfs mixed mode usage in _scratch_mkfs_sized Shin'ichiro Kawasaki
2022-02-07  6:55 ` [PATCH 2/7] generic/{171,172,173,174,204}: check _scratch_mkfs_sized return code Shin'ichiro Kawasaki
2022-02-09  0:35   ` Darrick J. Wong [this message]
2022-02-09  8:09     ` Shinichiro Kawasaki
2022-02-07  6:55 ` [PATCH 3/7] ext4/021: " Shin'ichiro Kawasaki
2022-02-07  6:55 ` [PATCH 4/7] xfs/015: " Shin'ichiro Kawasaki
2022-02-09  0:36   ` Darrick J. Wong
2022-02-07  6:55 ` [PATCH 5/7] common: rename _filter_mkfs to _xfs_filter_mkfs Shin'ichiro Kawasaki
2022-02-09  0:43   ` Darrick J. Wong
2022-02-09  0:53     ` Darrick J. Wong
2022-02-09  8:18       ` Shinichiro Kawasaki
2022-02-07  6:55 ` [PATCH 6/7] common: move _xfs_filter_mkfs from common/filter to common/xfs Shin'ichiro Kawasaki
2022-02-07  6:55 ` [PATCH 7/7] generic/204: do xfs unique preparation only for xfs Shin'ichiro Kawasaki
  -- strict thread matches above, loose matches on Subject: below --
2022-02-07  3:09 [PATCH 0/7] fstests: fix _scratch_mkfs_sized failure handling Shin'ichiro Kawasaki
2022-02-07  3:09 ` [PATCH 2/7] generic/{171,172,173,174,204}: check _scratch_mkfs_sized return code Shin'ichiro Kawasaki
2022-01-24 11:10 [PATCH 0/7] fstests: fix _scratch_mkfs_sized failure handling Shin'ichiro Kawasaki
2022-01-24 11:10 ` [PATCH 2/7] generic/{171,172,173,174,204}: check _scratch_mkfs_sized return code Shin'ichiro Kawasaki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220209003548.GC8288@magnolia \
    --to=djwong@kernel.org \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=fstests@vger.kernel.org \
    --cc=johannes.thumshirn@wdc.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=naohiro.aota@wdc.com \
    --cc=shinichiro.kawasaki@wdc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.