fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] common/rc: make _mkfs_dev return mkfs_status
@ 2021-05-15 11:28 Sun Ke
  2021-05-15 15:49 ` Darrick J. Wong
  2021-05-16 14:00 ` Eryu Guan
  0 siblings, 2 replies; 3+ messages in thread
From: Sun Ke @ 2021-05-15 11:28 UTC (permalink / raw)
  To: fstests, guan; +Cc: sunke32

If _mkfs_dev fialed,the test should notrun. _mkfs_dev may need to
return mkfs_status as _scratch_mkfs_ext4 and _scratch_mkfs_xfs do.

Signed-off-by: Sun Ke <sunke32@huawei.com>
---
I run generic/test/042 on f2fs, it failed, because the device size is too small:

F2FS-tools: mkfs.f2fs Ver: 1.9.0 (2017-09-21)

Info: Disable heap-based policy
Info: Debug level = 0 
Info: Label = 
Info: Trim is enabled
Info: Segments per section = 1 
Info: Sections per zone = 1 
Info: sector size = 512 
Info: total sectors = 51200 (25 MB) 
Info: zone aligned segment0 blkaddr: 512 
    Error: Device size is not sufficient for F2FS volume
    Error: Failed to prepare a super block!!!
    Error: Could not format the device!!!

Change the device size to 40M, it can pass. But I am not sure if it will change 
the test's intention. 

[root@localhost xfstests]# ./check tests/generic/042
FSTYP         -- f2fs
PLATFORM      -- Linux/x86_64 localhost 5.12.0-rc5-next-20210330 #2 SMP Thu Apr 15 00:58:54 EDT 2021
MKFS_OPTIONS  -- /dev/sdb
MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch

generic/042 3s ...  4s
Ran: generic/042
Passed all 1 tests

Other tests also use _mkfs_dev(), if making _mkfs_dev return 
mkfs_status is ok, I will modify them in V2.

[root@localhost xfstests]# ./check tests/generic/042
FSTYP         -- f2fs
PLATFORM      -- Linux/x86_64 localhost 5.12.0-rc5-next-20210330 #3 SMP
Wed Apr 21 22:29:25 EDT 2021
MKFS_OPTIONS  -- /dev/sdb
MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch

generic/042 2s ... [not run] mkfs failed!
Ran: generic/042
Not run: generic/042
Passed all 1 tests


 common/rc         | 14 ++++++++------
 tests/generic/042 |  2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/common/rc b/common/rc
index 0ce3cb0d..4254806a 100644
--- a/common/rc
+++ b/common/rc
@@ -642,6 +642,8 @@ _test_mkfs()
 _mkfs_dev()
 {
     local tmp=`mktemp -u`
+    local mkfs_status
+
     case $FSTYP in
     nfs*)
 	# do nothing for nfs
@@ -677,15 +679,15 @@ _mkfs_dev()
 		2>$tmp.mkfserr 1>$tmp.mkfsstd
 	;;
     esac
+    mkfs_status=$?
 
-    if [ $? -ne 0 ]; then
-	# output stored mkfs output
-	cat $tmp.mkfserr >&2
-	cat $tmp.mkfsstd
-	status=1
-	exit 1
+    if [ $mkfs_status -ne 0 ]; then
+        # output stored mkfs output
+        cat $tmp.mkfserr >&2
+        cat $tmp.mkfsstd
     fi
     rm -f $tmp.mkfserr $tmp.mkfsstd
+    return $mkfs_status
 }
 
 # remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
diff --git a/tests/generic/042 b/tests/generic/042
index 35727bcb..68bc15ca 100755
--- a/tests/generic/042
+++ b/tests/generic/042
@@ -46,7 +46,7 @@ _crashtest()
 	# the image to detect stale data exposure.
 	$XFS_IO_PROG -f -c "truncate 0" -c "pwrite -S 0xCD 0 25M" $img \
 		>> $seqres.full 2>&1
-	_mkfs_dev $img >> $seqres.full 2>&1
+	_mkfs_dev $img >> $seqres.full 2>&1 || _notrun "mkfs failed!"
 
 	mkdir -p $mnt
 	_mount $img $mnt
-- 
2.13.6


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] common/rc: make _mkfs_dev return mkfs_status
  2021-05-15 11:28 [PATCH] common/rc: make _mkfs_dev return mkfs_status Sun Ke
@ 2021-05-15 15:49 ` Darrick J. Wong
  2021-05-16 14:00 ` Eryu Guan
  1 sibling, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2021-05-15 15:49 UTC (permalink / raw)
  To: Sun Ke; +Cc: fstests, guan

On Sat, May 15, 2021 at 07:28:20AM -0400, Sun Ke wrote:
> If _mkfs_dev fialed,the test should notrun. _mkfs_dev may need to
> return mkfs_status as _scratch_mkfs_ext4 and _scratch_mkfs_xfs do.
> 
> Signed-off-by: Sun Ke <sunke32@huawei.com>
> ---
> I run generic/test/042 on f2fs, it failed, because the device size is too small:
> 
> F2FS-tools: mkfs.f2fs Ver: 1.9.0 (2017-09-21)
> 
> Info: Disable heap-based policy
> Info: Debug level = 0 
> Info: Label = 
> Info: Trim is enabled
> Info: Segments per section = 1 
> Info: Sections per zone = 1 
> Info: sector size = 512 
> Info: total sectors = 51200 (25 MB) 
> Info: zone aligned segment0 blkaddr: 512 
>     Error: Device size is not sufficient for F2FS volume
>     Error: Failed to prepare a super block!!!
>     Error: Could not format the device!!!
> 
> Change the device size to 40M, it can pass. But I am not sure if it will change 
> the test's intention. 
> 
> [root@localhost xfstests]# ./check tests/generic/042
> FSTYP         -- f2fs
> PLATFORM      -- Linux/x86_64 localhost 5.12.0-rc5-next-20210330 #2 SMP Thu Apr 15 00:58:54 EDT 2021
> MKFS_OPTIONS  -- /dev/sdb
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch
> 
> generic/042 3s ...  4s
> Ran: generic/042
> Passed all 1 tests
> 
> Other tests also use _mkfs_dev(), if making _mkfs_dev return 
> mkfs_status is ok, I will modify them in V2.
> 
> [root@localhost xfstests]# ./check tests/generic/042
> FSTYP         -- f2fs
> PLATFORM      -- Linux/x86_64 localhost 5.12.0-rc5-next-20210330 #3 SMP
> Wed Apr 21 22:29:25 EDT 2021
> MKFS_OPTIONS  -- /dev/sdb
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch
> 
> generic/042 2s ... [not run] mkfs failed!
> Ran: generic/042
> Not run: generic/042
> Passed all 1 tests
> 
> 
>  common/rc         | 14 ++++++++------
>  tests/generic/042 |  2 +-
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 0ce3cb0d..4254806a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -642,6 +642,8 @@ _test_mkfs()
>  _mkfs_dev()
>  {
>      local tmp=`mktemp -u`
> +    local mkfs_status
> +
>      case $FSTYP in
>      nfs*)
>  	# do nothing for nfs
> @@ -677,15 +679,15 @@ _mkfs_dev()
>  		2>$tmp.mkfserr 1>$tmp.mkfsstd
>  	;;
>      esac
> +    mkfs_status=$?
>  
> -    if [ $? -ne 0 ]; then
> -	# output stored mkfs output
> -	cat $tmp.mkfserr >&2
> -	cat $tmp.mkfsstd
> -	status=1
> -	exit 1
> +    if [ $mkfs_status -ne 0 ]; then
> +        # output stored mkfs output
> +        cat $tmp.mkfserr >&2
> +        cat $tmp.mkfsstd
>      fi
>      rm -f $tmp.mkfserr $tmp.mkfsstd
> +    return $mkfs_status

Hmm, this changes _mkfs_dev behavior such that it no longer exits the
test, which implies that the callers could be broken.  How about
renaming the above to _try_mkfs_dev and then add a _mkfs_dev to:

_mkfs_dev() {
	_try_mkfs_dev "$@"
	if [ $? -ne 0 ]; then
		status=1
		exit 1
	fi
}

I think then the only difference is that _mkfs_dev now cleans up the
$tmp.mkfs* files.

--D

>  }
>  
>  # remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
> diff --git a/tests/generic/042 b/tests/generic/042
> index 35727bcb..68bc15ca 100755
> --- a/tests/generic/042
> +++ b/tests/generic/042
> @@ -46,7 +46,7 @@ _crashtest()
>  	# the image to detect stale data exposure.
>  	$XFS_IO_PROG -f -c "truncate 0" -c "pwrite -S 0xCD 0 25M" $img \
>  		>> $seqres.full 2>&1
> -	_mkfs_dev $img >> $seqres.full 2>&1
> +	_mkfs_dev $img >> $seqres.full 2>&1 || _notrun "mkfs failed!"
>  
>  	mkdir -p $mnt
>  	_mount $img $mnt
> -- 
> 2.13.6
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] common/rc: make _mkfs_dev return mkfs_status
  2021-05-15 11:28 [PATCH] common/rc: make _mkfs_dev return mkfs_status Sun Ke
  2021-05-15 15:49 ` Darrick J. Wong
@ 2021-05-16 14:00 ` Eryu Guan
  1 sibling, 0 replies; 3+ messages in thread
From: Eryu Guan @ 2021-05-16 14:00 UTC (permalink / raw)
  To: Sun Ke; +Cc: fstests

On Sat, May 15, 2021 at 07:28:20AM -0400, Sun Ke wrote:
> If _mkfs_dev fialed,the test should notrun. _mkfs_dev may need to
> return mkfs_status as _scratch_mkfs_ext4 and _scratch_mkfs_xfs do.
> 
> Signed-off-by: Sun Ke <sunke32@huawei.com>
> ---
> I run generic/test/042 on f2fs, it failed, because the device size is too small:

I think we should require a minimal device size in generic/042 for f2fs,
or write a larger fs image file if possible so that f2fs could run.

Thanks,
Eryu

> 
> F2FS-tools: mkfs.f2fs Ver: 1.9.0 (2017-09-21)
> 
> Info: Disable heap-based policy
> Info: Debug level = 0 
> Info: Label = 
> Info: Trim is enabled
> Info: Segments per section = 1 
> Info: Sections per zone = 1 
> Info: sector size = 512 
> Info: total sectors = 51200 (25 MB) 
> Info: zone aligned segment0 blkaddr: 512 
>     Error: Device size is not sufficient for F2FS volume
>     Error: Failed to prepare a super block!!!
>     Error: Could not format the device!!!
> 
> Change the device size to 40M, it can pass. But I am not sure if it will change 
> the test's intention. 
> 
> [root@localhost xfstests]# ./check tests/generic/042
> FSTYP         -- f2fs
> PLATFORM      -- Linux/x86_64 localhost 5.12.0-rc5-next-20210330 #2 SMP Thu Apr 15 00:58:54 EDT 2021
> MKFS_OPTIONS  -- /dev/sdb
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch
> 
> generic/042 3s ...  4s
> Ran: generic/042
> Passed all 1 tests
> 
> Other tests also use _mkfs_dev(), if making _mkfs_dev return 
> mkfs_status is ok, I will modify them in V2.
> 
> [root@localhost xfstests]# ./check tests/generic/042
> FSTYP         -- f2fs
> PLATFORM      -- Linux/x86_64 localhost 5.12.0-rc5-next-20210330 #3 SMP
> Wed Apr 21 22:29:25 EDT 2021
> MKFS_OPTIONS  -- /dev/sdb
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch
> 
> generic/042 2s ... [not run] mkfs failed!
> Ran: generic/042
> Not run: generic/042
> Passed all 1 tests
> 
> 
>  common/rc         | 14 ++++++++------
>  tests/generic/042 |  2 +-
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 0ce3cb0d..4254806a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -642,6 +642,8 @@ _test_mkfs()
>  _mkfs_dev()
>  {
>      local tmp=`mktemp -u`
> +    local mkfs_status
> +
>      case $FSTYP in
>      nfs*)
>  	# do nothing for nfs
> @@ -677,15 +679,15 @@ _mkfs_dev()
>  		2>$tmp.mkfserr 1>$tmp.mkfsstd
>  	;;
>      esac
> +    mkfs_status=$?
>  
> -    if [ $? -ne 0 ]; then
> -	# output stored mkfs output
> -	cat $tmp.mkfserr >&2
> -	cat $tmp.mkfsstd
> -	status=1
> -	exit 1
> +    if [ $mkfs_status -ne 0 ]; then
> +        # output stored mkfs output
> +        cat $tmp.mkfserr >&2
> +        cat $tmp.mkfsstd
>      fi
>      rm -f $tmp.mkfserr $tmp.mkfsstd
> +    return $mkfs_status
>  }
>  
>  # remove all files in $SCRATCH_MNT, useful when testing on NFS/CIFS
> diff --git a/tests/generic/042 b/tests/generic/042
> index 35727bcb..68bc15ca 100755
> --- a/tests/generic/042
> +++ b/tests/generic/042
> @@ -46,7 +46,7 @@ _crashtest()
>  	# the image to detect stale data exposure.
>  	$XFS_IO_PROG -f -c "truncate 0" -c "pwrite -S 0xCD 0 25M" $img \
>  		>> $seqres.full 2>&1
> -	_mkfs_dev $img >> $seqres.full 2>&1
> +	_mkfs_dev $img >> $seqres.full 2>&1 || _notrun "mkfs failed!"
>  
>  	mkdir -p $mnt
>  	_mount $img $mnt
> -- 
> 2.13.6

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-16 14:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-15 11:28 [PATCH] common/rc: make _mkfs_dev return mkfs_status Sun Ke
2021-05-15 15:49 ` Darrick J. Wong
2021-05-16 14:00 ` Eryu Guan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).