[PATCH 00/24] fstests: tons of random fixes

[PATCH 00/24] fstests: tons of random fixes

[Darrick J. Wong]

Hi all,

So now that I have a cloud account, I suddenly have much more resources
to find out if things like external log devices and realtime volumes
actually work.  This means that I've found a couple dozen minor issues
all over fstests; most of the front patches in this series try to
address them.

At the end, there is also a new test to try unloading and reloading the
filesystem module between tests, to detect problems with resource
allocations not being freed.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-stuff
---
 README            |    3 +++
 check             |    9 +++++++++
 common/rc         |   20 ++++++++++++++++----
 common/xfs        |   28 +++++++++++++++++++++++++++-
 tests/generic/204 |    9 +++++++--
 tests/generic/600 |    4 +++-
 tests/generic/601 |    4 +++-
 tests/generic/607 |    5 +++++
 tests/overlay/020 |    6 ++++++
 tests/overlay/069 |    2 +-
 tests/overlay/071 |    2 +-
 tests/xfs/003     |   14 +++++++-------
 tests/xfs/016     |    4 ++--
 tests/xfs/019     |    5 +++++
 tests/xfs/031     |    5 +++++
 tests/xfs/032     |    4 +---
 tests/xfs/045     |    2 +-
 tests/xfs/070     |    4 +++-
 tests/xfs/073     |    5 +----
 tests/xfs/077     |    3 +--
 tests/xfs/098     |    8 +++++++-
 tests/xfs/111     |    2 +-
 tests/xfs/135     |    2 +-
 tests/xfs/137     |    4 ++--
 tests/xfs/141     |    4 +++-
 tests/xfs/171     |    1 +
 tests/xfs/172     |    1 +
 tests/xfs/173     |    1 +
 tests/xfs/174     |    1 +
 tests/xfs/194     |    3 +++
 tests/xfs/205     |    1 +
 tests/xfs/284     |    3 +--
 tests/xfs/291     |    6 ++++--
 tests/xfs/306     |    1 +
 tests/xfs/318     |    1 +
 tests/xfs/331     |    1 +
 tests/xfs/424     |    8 +++++++-
 tests/xfs/444     |    1 +
 tests/xfs/449     |    4 +++-
 tests/xfs/503     |    1 +
 40 files changed, 149 insertions(+), 43 deletions(-)

[PATCH 01/24] xfs/331: don't run this test if fallocate isn't supported

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

This test requires fallocate, so make sure that actually works before
running it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/331 |    1 +
 1 file changed, 1 insertion(+)


diff --git a/tests/xfs/331 b/tests/xfs/331
index b26ea155..501dd1f8 100755
--- a/tests/xfs/331
+++ b/tests/xfs/331
@@ -32,6 +32,7 @@ _supported_fs xfs
 _require_scratch
 _require_xfs_scratch_rmapbt
 _require_scratch_reflink
+_require_xfs_io_command "falloc"
 _require_test_program "punch-alternating"
 
 rm -f "$seqres.full"

Re: [PATCH 01/24] xfs/331: don't run this test if fallocate isn't supported

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:00PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test requires fallocate, so make sure that actually works before
> running it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/331 |    1 +
>  1 file changed, 1 insertion(+)
> 
> 
> diff --git a/tests/xfs/331 b/tests/xfs/331
> index b26ea155..501dd1f8 100755
> --- a/tests/xfs/331
> +++ b/tests/xfs/331
> @@ -32,6 +32,7 @@ _supported_fs xfs
>  _require_scratch
>  _require_xfs_scratch_rmapbt
>  _require_scratch_reflink
> +_require_xfs_io_command "falloc"

Good to me.
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  _require_test_program "punch-alternating"
>  
>  rm -f "$seqres.full"
> 

Re: [PATCH 01/24] xfs/331: don't run this test if fallocate isn't supported

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:00PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test requires fallocate, so make sure that actually works before
> running it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 02/24] generic/60[01]: fix test failure when setting new grace limit

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

The setquota command can extend a quota grace period by a certain number
of seconds.  The extension is provided as a number of seconds relative
to right now.  However, if the system clock increments the seconds count
after this test assigns $now but before setquota gets called, the test
will fail because $get and $set will be off by that 1 second.  Allow for
that.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/600 |    4 +++-
 tests/generic/601 |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)


diff --git a/tests/generic/600 b/tests/generic/600
index 7ea0d6c2..42e4ee55 100755
--- a/tests/generic/600
+++ b/tests/generic/600
@@ -60,7 +60,9 @@ let set=now+100
 setquota -T -u $qa_user 0 100 $SCRATCH_MNT 2>&1 | grep -v "^setquota"
 get=`repquota -up $SCRATCH_MNT | grep  "^$qa_user" | awk '{print $NF}'`
 
-if [ "$get" != "$set" ]; then
+# Either the new expiry must match; or be one second after the set time, to
+# deal with the seconds counter incrementing.
+if [ "$get" != "$set" ] && [ "$get" -ne "$((set + 1))" ]; then
 	echo "set grace to $set but got grace $get"
 fi
 
diff --git a/tests/generic/601 b/tests/generic/601
index 1baa6a90..b412ee8a 100755
--- a/tests/generic/601
+++ b/tests/generic/601
@@ -71,7 +71,9 @@ $XFS_QUOTA_PROG -x -c "timer -u -i 100 $qa_user" $SCRATCH_MNT
 # raw ("since epoch") grace expiry
 get=`repquota -up $SCRATCH_MNT | grep  "^$qa_user" | awk '{print $NF}'`
 
-if [ "$get" != "$set" ]; then
+# Either the new expiry must match; or be one second after the set time, to
+# deal with the seconds counter incrementing.
+if [ "$get" != "$set" ] && [ "$get" -ne "$((set + 1))" ]; then
 	echo "set grace to $set but got grace $get"
 fi
 

Re: [PATCH 02/24] generic/60[01]: fix test failure when setting new grace limit

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The setquota command can extend a quota grace period by a certain number
> of seconds.  The extension is provided as a number of seconds relative
> to right now.  However, if the system clock increments the seconds count
> after this test assigns $now but before setquota gets called, the test
> will fail because $get and $set will be off by that 1 second.  Allow for
> that.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/generic/600 |    4 +++-
>  tests/generic/601 |    4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/tests/generic/600 b/tests/generic/600
> index 7ea0d6c2..42e4ee55 100755
> --- a/tests/generic/600
> +++ b/tests/generic/600
> @@ -60,7 +60,9 @@ let set=now+100
>  setquota -T -u $qa_user 0 100 $SCRATCH_MNT 2>&1 | grep -v "^setquota"
>  get=`repquota -up $SCRATCH_MNT | grep  "^$qa_user" | awk '{print $NF}'`
>  
> -if [ "$get" != "$set" ]; then
> +# Either the new expiry must match; or be one second after the set time, to
> +# deal with the seconds counter incrementing.
> +if [ "$get" != "$set" ] && [ "$get" -ne "$((set + 1))" ]; then
>  	echo "set grace to $set but got grace $get"
>  fi
>  
> diff --git a/tests/generic/601 b/tests/generic/601
> index 1baa6a90..b412ee8a 100755
> --- a/tests/generic/601
> +++ b/tests/generic/601
> @@ -71,7 +71,9 @@ $XFS_QUOTA_PROG -x -c "timer -u -i 100 $qa_user" $SCRATCH_MNT
>  # raw ("since epoch") grace expiry
>  get=`repquota -up $SCRATCH_MNT | grep  "^$qa_user" | awk '{print $NF}'`
>  
> -if [ "$get" != "$set" ]; then
> +# Either the new expiry must match; or be one second after the set time, to
> +# deal with the seconds counter incrementing.
> +if [ "$get" != "$set" ] && [ "$get" -ne "$((set + 1))" ]; then
>  	echo "set grace to $set but got grace $get"
>  fi
>  
> 

Re: [PATCH 02/24] generic/60[01]: fix test failure when setting new grace limit

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The setquota command can extend a quota grace period by a certain number
> of seconds.  The extension is provided as a number of seconds relative
> to right now.  However, if the system clock increments the seconds count
> after this test assigns $now but before setquota gets called, the test
> will fail because $get and $set will be off by that 1 second.  Allow for
> that.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

This test requires that the filesystem support calling FSGETXATTR on
regular files and directories to make sure the FS_XFLAG_DAX flag works.
The _require_xfs_io_command tests a regular file but doesn't check
directories, so generic/607 should do that itself.  Also fix some typos.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/rc         |   10 ++++++++--
 tests/generic/607 |    5 +++++
 2 files changed, 13 insertions(+), 2 deletions(-)


diff --git a/common/rc b/common/rc
index aa5a7409..f78b1cfc 100644
--- a/common/rc
+++ b/common/rc
@@ -2162,6 +2162,12 @@ _require_xfs_io_command()
 	local testfile=$TEST_DIR/$$.xfs_io
 	local testio
 	case $command in
+	"lsattr")
+		# Test xfs_io lsattr support and filesystem FS_IOC_FSSETXATTR
+		# support.
+		testio=`$XFS_IO_PROG -F -f -c "lsattr $param" $testfile 2>&1`
+		param_checked="$param"
+		;;
 	"chattr")
 		if [ -z "$param" ]; then
 			param=s
@@ -3205,7 +3211,7 @@ _check_s_dax()
 	if [ $exp_s_dax -eq 0 ]; then
 		(( attributes & 0x2000 )) && echo "$target has unexpected S_DAX flag"
 	else
-		(( attributes & 0x2000 )) || echo "$target doen't have expected S_DAX flag"
+		(( attributes & 0x2000 )) || echo "$target doesn't have expected S_DAX flag"
 	fi
 }
 
@@ -3217,7 +3223,7 @@ _check_xflag()
 	if [ $exp_xflag -eq 0 ]; then
 		_test_inode_flag dax $target && echo "$target has unexpected FS_XFLAG_DAX flag"
 	else
-		_test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
+		_test_inode_flag dax $target || echo "$target doesn't have expected FS_XFLAG_DAX flag"
 	fi
 }
 
diff --git a/tests/generic/607 b/tests/generic/607
index b15085ea..14d2c05f 100755
--- a/tests/generic/607
+++ b/tests/generic/607
@@ -38,6 +38,11 @@ _require_scratch
 _require_dax_iflag
 _require_xfs_io_command "lsattr" "-v"
 
+# Make sure we can call FSGETXATTR on a directory...
+output="$($XFS_IO_PROG -c "lsattr -v" $TEST_DIR 2>&1)"
+echo "$output" | grep -q "Inappropriate ioctl for device" && \
+	_notrun "$FSTYP: FSGETXATTR not supported on directories."
+
 # If a/ is +x, check that a's new children
 # inherit +x from a/.
 test_xflag_inheritance1()

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test requires that the filesystem support calling FSGETXATTR on
> regular files and directories to make sure the FS_XFLAG_DAX flag works.
> The _require_xfs_io_command tests a regular file but doesn't check
> directories, so generic/607 should do that itself.  Also fix some typos.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/rc         |   10 ++++++++--
>  tests/generic/607 |    5 +++++
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index aa5a7409..f78b1cfc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2162,6 +2162,12 @@ _require_xfs_io_command()
>  	local testfile=$TEST_DIR/$$.xfs_io
>  	local testio
>  	case $command in
> +	"lsattr")
> +		# Test xfs_io lsattr support and filesystem FS_IOC_FSSETXATTR
> +		# support.
> +		testio=`$XFS_IO_PROG -F -f -c "lsattr $param" $testfile 2>&1`
> +		param_checked="$param"
> +		;;
>  	"chattr")
>  		if [ -z "$param" ]; then
>  			param=s
> @@ -3205,7 +3211,7 @@ _check_s_dax()
>  	if [ $exp_s_dax -eq 0 ]; then
>  		(( attributes & 0x2000 )) && echo "$target has unexpected S_DAX flag"
>  	else
> -		(( attributes & 0x2000 )) || echo "$target doen't have expected S_DAX flag"
> +		(( attributes & 0x2000 )) || echo "$target doesn't have expected S_DAX flag"
>  	fi
>  }
>  
> @@ -3217,7 +3223,7 @@ _check_xflag()
>  	if [ $exp_xflag -eq 0 ]; then
>  		_test_inode_flag dax $target && echo "$target has unexpected FS_XFLAG_DAX flag"
>  	else
> -		_test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
> +		_test_inode_flag dax $target || echo "$target doesn't have expected FS_XFLAG_DAX flag"
>  	fi
>  }
>  
> diff --git a/tests/generic/607 b/tests/generic/607
> index b15085ea..14d2c05f 100755
> --- a/tests/generic/607
> +++ b/tests/generic/607
> @@ -38,6 +38,11 @@ _require_scratch
>  _require_dax_iflag
>  _require_xfs_io_command "lsattr" "-v"
>  
> +# Make sure we can call FSGETXATTR on a directory...
> +output="$($XFS_IO_PROG -c "lsattr -v" $TEST_DIR 2>&1)"
> +echo "$output" | grep -q "Inappropriate ioctl for device" && \
> +	_notrun "$FSTYP: FSGETXATTR not supported on directories."
> +
>  # If a/ is +x, check that a's new children
>  # inherit +x from a/.
>  test_xflag_inheritance1()
> 

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Xiao Yang]

On 2020/9/15 9:43, Darrick J. Wong wrote:
> From: Darrick J. Wong<darrick.wong@oracle.com>
>
> This test requires that the filesystem support calling FSGETXATTR on
> regular files and directories to make sure the FS_XFLAG_DAX flag works.
> The _require_xfs_io_command tests a regular file but doesn't check
> directories, so generic/607 should do that itself.  Also fix some typos.
>
> Signed-off-by: Darrick J. Wong<darrick.wong@oracle.com>
> ---
>   common/rc         |   10 ++++++++--
>   tests/generic/607 |    5 +++++
>   2 files changed, 13 insertions(+), 2 deletions(-)
>
>
> diff --git a/common/rc b/common/rc
> index aa5a7409..f78b1cfc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2162,6 +2162,12 @@ _require_xfs_io_command()
>   	local testfile=$TEST_DIR/$$.xfs_io
>   	local testio
>   	case $command in
> +	"lsattr")
> +		# Test xfs_io lsattr support and filesystem FS_IOC_FSSETXATTR
> +		# support.
> +		testio=`$XFS_IO_PROG -F -f -c "lsattr $param" $testfile 2>&1`
> +		param_checked="$param"
> +		;;
>   	"chattr")
>   		if [ -z "$param" ]; then
>   			param=s
> @@ -3205,7 +3211,7 @@ _check_s_dax()
>   	if [ $exp_s_dax -eq 0 ]; then
>   		(( attributes&  0x2000 ))&&  echo "$target has unexpected S_DAX flag"
>   	else
> -		(( attributes&  0x2000 )) || echo "$target doen't have expected S_DAX flag"
> +		(( attributes&  0x2000 )) || echo "$target doesn't have expected S_DAX flag"
>   	fi
>   }
>
> @@ -3217,7 +3223,7 @@ _check_xflag()
>   	if [ $exp_xflag -eq 0 ]; then
>   		_test_inode_flag dax $target&&  echo "$target has unexpected FS_XFLAG_DAX flag"
>   	else
> -		_test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
> +		_test_inode_flag dax $target || echo "$target doesn't have expected FS_XFLAG_DAX flag"
>   	fi
>   }
>
> diff --git a/tests/generic/607 b/tests/generic/607
> index b15085ea..14d2c05f 100755
> --- a/tests/generic/607
> +++ b/tests/generic/607
> @@ -38,6 +38,11 @@ _require_scratch
>   _require_dax_iflag
>   _require_xfs_io_command "lsattr" "-v"
>
> +# Make sure we can call FSGETXATTR on a directory...
> +output="$($XFS_IO_PROG -c "lsattr -v" $TEST_DIR 2>&1)"
> +echo "$output" | grep -q "Inappropriate ioctl for device"&&  \
> +	_notrun "$FSTYP: FSGETXATTR not supported on directories."
Hi Darrick,

Could you tell me which kernel version gets the issue? :-)

Best Regards,
Xiao Yang
> +
>   # If a/ is +x, check that a's new children
>   # inherit +x from a/.
>   test_xflag_inheritance1()
>
>
>
> .
>

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 09:41:01AM +0800, Xiao Yang wrote:
> On 2020/9/15 9:43, Darrick J. Wong wrote:
> > From: Darrick J. Wong<darrick.wong@oracle.com>
> > 
> > This test requires that the filesystem support calling FSGETXATTR on
> > regular files and directories to make sure the FS_XFLAG_DAX flag works.
> > The _require_xfs_io_command tests a regular file but doesn't check
> > directories, so generic/607 should do that itself.  Also fix some typos.
> > 
> > Signed-off-by: Darrick J. Wong<darrick.wong@oracle.com>
> > ---
> >   common/rc         |   10 ++++++++--
> >   tests/generic/607 |    5 +++++
> >   2 files changed, 13 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/common/rc b/common/rc
> > index aa5a7409..f78b1cfc 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -2162,6 +2162,12 @@ _require_xfs_io_command()
> >   	local testfile=$TEST_DIR/$$.xfs_io
> >   	local testio
> >   	case $command in
> > +	"lsattr")
> > +		# Test xfs_io lsattr support and filesystem FS_IOC_FSSETXATTR
> > +		# support.
> > +		testio=`$XFS_IO_PROG -F -f -c "lsattr $param" $testfile 2>&1`
> > +		param_checked="$param"
> > +		;;
> >   	"chattr")
> >   		if [ -z "$param" ]; then
> >   			param=s
> > @@ -3205,7 +3211,7 @@ _check_s_dax()
> >   	if [ $exp_s_dax -eq 0 ]; then
> >   		(( attributes&  0x2000 ))&&  echo "$target has unexpected S_DAX flag"
> >   	else
> > -		(( attributes&  0x2000 )) || echo "$target doen't have expected S_DAX flag"
> > +		(( attributes&  0x2000 )) || echo "$target doesn't have expected S_DAX flag"
> >   	fi
> >   }
> > 
> > @@ -3217,7 +3223,7 @@ _check_xflag()
> >   	if [ $exp_xflag -eq 0 ]; then
> >   		_test_inode_flag dax $target&&  echo "$target has unexpected FS_XFLAG_DAX flag"
> >   	else
> > -		_test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
> > +		_test_inode_flag dax $target || echo "$target doesn't have expected FS_XFLAG_DAX flag"
> >   	fi
> >   }
> > 
> > diff --git a/tests/generic/607 b/tests/generic/607
> > index b15085ea..14d2c05f 100755
> > --- a/tests/generic/607
> > +++ b/tests/generic/607
> > @@ -38,6 +38,11 @@ _require_scratch
> >   _require_dax_iflag
> >   _require_xfs_io_command "lsattr" "-v"
> > 
> > +# Make sure we can call FSGETXATTR on a directory...
> > +output="$($XFS_IO_PROG -c "lsattr -v" $TEST_DIR 2>&1)"
> > +echo "$output" | grep -q "Inappropriate ioctl for device"&&  \
> > +	_notrun "$FSTYP: FSGETXATTR not supported on directories."
> Hi Darrick,
> 
> Could you tell me which kernel version gets the issue? :-)

ext4.

--D

> Best Regards,
> Xiao Yang
> > +
> >   # If a/ is +x, check that a's new children
> >   # inherit +x from a/.
> >   test_xflag_inheritance1()
> > 
> > 
> > 
> > .
> > 
> 
> 
> 

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Xiao Yang]

于 2020/9/17 11:27, Darrick J. Wong 写道:
> On Thu, Sep 17, 2020 at 09:41:01AM +0800, Xiao Yang wrote:
>> On 2020/9/15 9:43, Darrick J. Wong wrote:
>>> From: Darrick J. Wong<darrick.wong@oracle.com>
>>>
>>> This test requires that the filesystem support calling FSGETXATTR on
>>> regular files and directories to make sure the FS_XFLAG_DAX flag works.
>>> The _require_xfs_io_command tests a regular file but doesn't check
>>> directories, so generic/607 should do that itself.  Also fix some typos.
>>>
>>> Signed-off-by: Darrick J. Wong<darrick.wong@oracle.com>
>>> ---
>>>    common/rc         |   10 ++++++++--
>>>    tests/generic/607 |    5 +++++
>>>    2 files changed, 13 insertions(+), 2 deletions(-)
>>>
>>>
>>> diff --git a/common/rc b/common/rc
>>> index aa5a7409..f78b1cfc 100644
>>> --- a/common/rc
>>> +++ b/common/rc
>>> @@ -2162,6 +2162,12 @@ _require_xfs_io_command()
>>>    	local testfile=$TEST_DIR/$$.xfs_io
>>>    	local testio
>>>    	case $command in
>>> +	"lsattr")
>>> +		# Test xfs_io lsattr support and filesystem FS_IOC_FSSETXATTR
>>> +		# support.
>>> +		testio=`$XFS_IO_PROG -F -f -c "lsattr $param" $testfile 2>&1`
>>> +		param_checked="$param"
>>> +		;;
>>>    	"chattr")
>>>    		if [ -z "$param" ]; then
>>>    			param=s
>>> @@ -3205,7 +3211,7 @@ _check_s_dax()
>>>    	if [ $exp_s_dax -eq 0 ]; then
>>>    		(( attributes&   0x2000 ))&&   echo "$target has unexpected S_DAX flag"
>>>    	else
>>> -		(( attributes&   0x2000 )) || echo "$target doen't have expected S_DAX flag"
>>> +		(( attributes&   0x2000 )) || echo "$target doesn't have expected S_DAX flag"
>>>    	fi
>>>    }
>>>
>>> @@ -3217,7 +3223,7 @@ _check_xflag()
>>>    	if [ $exp_xflag -eq 0 ]; then
>>>    		_test_inode_flag dax $target&&   echo "$target has unexpected FS_XFLAG_DAX flag"
>>>    	else
>>> -		_test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
>>> +		_test_inode_flag dax $target || echo "$target doesn't have expected FS_XFLAG_DAX flag"
>>>    	fi
>>>    }
>>>
>>> diff --git a/tests/generic/607 b/tests/generic/607
>>> index b15085ea..14d2c05f 100755
>>> --- a/tests/generic/607
>>> +++ b/tests/generic/607
>>> @@ -38,6 +38,11 @@ _require_scratch
>>>    _require_dax_iflag
>>>    _require_xfs_io_command "lsattr" "-v"
>>>
>>> +# Make sure we can call FSGETXATTR on a directory...
>>> +output="$($XFS_IO_PROG -c "lsattr -v" $TEST_DIR 2>&1)"
>>> +echo "$output" | grep -q "Inappropriate ioctl for device"&&   \
>>> +	_notrun "$FSTYP: FSGETXATTR not supported on directories."
>> Hi Darrick,
>>
>> Could you tell me which kernel version gets the issue? :-)
> ext4.
Hi Darrick,

I didn't get the issue by v5.7.0 xfs_io on v5.8.0 kernel:
----------------------------------------------------------------------------------
# blkid /dev/pmem0
/dev/pmem0: UUID="181f4d76-bc21-45b7-a6d2-e486f6cc479b" TYPE="ext4"
# df -h | grep pmem0
/dev/pmem0 2.0G 28K 1.8G 1% /mnt/xfstests/test
# strace -e ioctl xfs_io -c "lsattr -v" /mnt/xfstests/test
ioctl(3, FS_IOC_FSGETXATTR, 0x7ffdc7061d10) = 0
[] /mnt/xfstests/test
----------------------------------------------------------------------------------
Do I miss something?

Thanks,
Xiao Yang
> --D
>
>> Best Regards,
>> Xiao Yang
>>> +
>>>    # If a/ is +x, check that a's new children
>>>    # inherit +x from a/.
>>>    test_xflag_inheritance1()
>>>
>>>
>>>
>>> .
>>>
>>
>>
>
> .
>

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 11:43:10AM +0800, Xiao Yang wrote:
> 于 2020/9/17 11:27, Darrick J. Wong 写道:
> > On Thu, Sep 17, 2020 at 09:41:01AM +0800, Xiao Yang wrote:
> > > On 2020/9/15 9:43, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong<darrick.wong@oracle.com>
> > > > 
> > > > This test requires that the filesystem support calling FSGETXATTR on
> > > > regular files and directories to make sure the FS_XFLAG_DAX flag works.
> > > > The _require_xfs_io_command tests a regular file but doesn't check
> > > > directories, so generic/607 should do that itself.  Also fix some typos.
> > > > 
> > > > Signed-off-by: Darrick J. Wong<darrick.wong@oracle.com>
> > > > ---
> > > >    common/rc         |   10 ++++++++--
> > > >    tests/generic/607 |    5 +++++
> > > >    2 files changed, 13 insertions(+), 2 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/common/rc b/common/rc
> > > > index aa5a7409..f78b1cfc 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -2162,6 +2162,12 @@ _require_xfs_io_command()
> > > >    	local testfile=$TEST_DIR/$$.xfs_io
> > > >    	local testio
> > > >    	case $command in
> > > > +	"lsattr")
> > > > +		# Test xfs_io lsattr support and filesystem FS_IOC_FSSETXATTR
> > > > +		# support.
> > > > +		testio=`$XFS_IO_PROG -F -f -c "lsattr $param" $testfile 2>&1`
> > > > +		param_checked="$param"
> > > > +		;;
> > > >    	"chattr")
> > > >    		if [ -z "$param" ]; then
> > > >    			param=s
> > > > @@ -3205,7 +3211,7 @@ _check_s_dax()
> > > >    	if [ $exp_s_dax -eq 0 ]; then
> > > >    		(( attributes&   0x2000 ))&&   echo "$target has unexpected S_DAX flag"
> > > >    	else
> > > > -		(( attributes&   0x2000 )) || echo "$target doen't have expected S_DAX flag"
> > > > +		(( attributes&   0x2000 )) || echo "$target doesn't have expected S_DAX flag"
> > > >    	fi
> > > >    }
> > > > 
> > > > @@ -3217,7 +3223,7 @@ _check_xflag()
> > > >    	if [ $exp_xflag -eq 0 ]; then
> > > >    		_test_inode_flag dax $target&&   echo "$target has unexpected FS_XFLAG_DAX flag"
> > > >    	else
> > > > -		_test_inode_flag dax $target || echo "$target doen't have expected FS_XFLAG_DAX flag"
> > > > +		_test_inode_flag dax $target || echo "$target doesn't have expected FS_XFLAG_DAX flag"
> > > >    	fi
> > > >    }
> > > > 
> > > > diff --git a/tests/generic/607 b/tests/generic/607
> > > > index b15085ea..14d2c05f 100755
> > > > --- a/tests/generic/607
> > > > +++ b/tests/generic/607
> > > > @@ -38,6 +38,11 @@ _require_scratch
> > > >    _require_dax_iflag
> > > >    _require_xfs_io_command "lsattr" "-v"
> > > > 
> > > > +# Make sure we can call FSGETXATTR on a directory...
> > > > +output="$($XFS_IO_PROG -c "lsattr -v" $TEST_DIR 2>&1)"
> > > > +echo "$output" | grep -q "Inappropriate ioctl for device"&&   \
> > > > +	_notrun "$FSTYP: FSGETXATTR not supported on directories."
> > > Hi Darrick,
> > > 
> > > Could you tell me which kernel version gets the issue? :-)
> > ext4.
> Hi Darrick,
> 
> I didn't get the issue by v5.7.0 xfs_io on v5.8.0 kernel:
> ----------------------------------------------------------------------------------
> # blkid /dev/pmem0
> /dev/pmem0: UUID="181f4d76-bc21-45b7-a6d2-e486f6cc479b" TYPE="ext4"
> # df -h | grep pmem0
> /dev/pmem0 2.0G 28K 1.8G 1% /mnt/xfstests/test
> # strace -e ioctl xfs_io -c "lsattr -v" /mnt/xfstests/test
> ioctl(3, FS_IOC_FSGETXATTR, 0x7ffdc7061d10) = 0
> [] /mnt/xfstests/test
> ----------------------------------------------------------------------------------
> Do I miss something?

Oops, sorry, I was reading the wrong VM report.  It's overlayfs (atop
xfs though I don't think that matters) that doesn't support FSGETXATTR
on directories.

--D

> Thanks,
> Xiao Yang
> > --D
> > 
> > > Best Regards,
> > > Xiao Yang
> > > > +
> > > >    # If a/ is +x, check that a's new children
> > > >    # inherit +x from a/.
> > > >    test_xflag_inheritance1()
> > > > 
> > > > 
> > > > 
> > > > .
> > > > 
> > > 
> > > 
> > 
> > .
> > 
> 
> 
> 

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Christoph Hellwig]

On Wed, Sep 16, 2020 at 08:56:20PM -0700, Darrick J. Wong wrote:
> Oops, sorry, I was reading the wrong VM report.  It's overlayfs (atop
> xfs though I don't think that matters) that doesn't support FSGETXATTR
> on directories.

I think we should overlayfs to support FSGETXATTR on all files that
can be opened instead.

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 08:52:45AM +0100, Christoph Hellwig wrote:
> On Wed, Sep 16, 2020 at 08:56:20PM -0700, Darrick J. Wong wrote:
> > Oops, sorry, I was reading the wrong VM report.  It's overlayfs (atop
> > xfs though I don't think that matters) that doesn't support FSGETXATTR
> > on directories.
> 
> I think we should overlayfs to support FSGETXATTR on all files that
> can be opened instead.

Heh, yes, that would be a better option. :)

Even if they do add it, though, I still think we need to be able to
_notrun this test to avoid failures on unpatched kernels?

This also makes me wonder, we lose all the FSGETXATTR state on copy-up,
don't we?  Since the VFS doesn't have a primitive for cloning all the
metadata?

--D

Re: [PATCH 03/24] generic/607: don't break on filesystems that don't support FSGETXATTR on dirs

[Christoph Hellwig]

On Thu, Sep 17, 2020 at 08:54:39AM -0700, Darrick J. Wong wrote:
> On Thu, Sep 17, 2020 at 08:52:45AM +0100, Christoph Hellwig wrote:
> > On Wed, Sep 16, 2020 at 08:56:20PM -0700, Darrick J. Wong wrote:
> > > Oops, sorry, I was reading the wrong VM report.  It's overlayfs (atop
> > > xfs though I don't think that matters) that doesn't support FSGETXATTR
> > > on directories.
> > 
> > I think we should overlayfs to support FSGETXATTR on all files that
> > can be opened instead.
> 
> Heh, yes, that would be a better option. :)
> 
> Even if they do add it, though, I still think we need to be able to
> _notrun this test to avoid failures on unpatched kernels?

I think supporting FSGETXATTR only on regular files is a bug and should
fail.

> This also makes me wonder, we lose all the FSGETXATTR state on copy-up,
> don't we?  Since the VFS doesn't have a primitive for cloning all the
> metadata?

Good question.

[PATCH 04/24] xfs: skip tests that rely on allocation behaviors of the data device

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

A large number of xfs-specific tests rely on specific behaviors of the
data device allocator, such as fragmenting free space, carefully curated
inode and free space counts, or features like filestreams that only
exist on the data device.

These tests fail horribly if the test runner specified rtinherit=1 on
the mkfs command line, so skip them all.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/171 |    1 +
 tests/xfs/172 |    1 +
 tests/xfs/173 |    1 +
 tests/xfs/174 |    1 +
 tests/xfs/205 |    1 +
 tests/xfs/306 |    1 +
 tests/xfs/318 |    1 +
 tests/xfs/444 |    1 +
 8 files changed, 8 insertions(+)


diff --git a/tests/xfs/171 b/tests/xfs/171
index 050d02ab..a0d6e3f9 100755
--- a/tests/xfs/171
+++ b/tests/xfs/171
@@ -33,6 +33,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"
 
diff --git a/tests/xfs/172 b/tests/xfs/172
index c100481c..13f1f381 100755
--- a/tests/xfs/172
+++ b/tests/xfs/172
@@ -33,6 +33,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"
 
diff --git a/tests/xfs/173 b/tests/xfs/173
index 1569146f..47e1d029 100755
--- a/tests/xfs/173
+++ b/tests/xfs/173
@@ -33,6 +33,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"
 
diff --git a/tests/xfs/174 b/tests/xfs/174
index 781a1967..348f38c3 100755
--- a/tests/xfs/174
+++ b/tests/xfs/174
@@ -33,6 +33,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch
+_require_no_rtinherit
 
 _check_filestreams_support || _notrun "filestreams not available"
 
diff --git a/tests/xfs/205 b/tests/xfs/205
index 645e509a..6f1c2058 100755
--- a/tests/xfs/205
+++ b/tests/xfs/205
@@ -24,6 +24,7 @@ _supported_os Linux
 
 # single AG will cause xfs_repair to fail checks.
 _require_scratch_nocheck
+_require_no_rtinherit
 
 rm -f $seqres.full
 
diff --git a/tests/xfs/306 b/tests/xfs/306
index edbb6076..d2a0dd7f 100755
--- a/tests/xfs/306
+++ b/tests/xfs/306
@@ -35,6 +35,7 @@ _supported_fs xfs
 _supported_os Linux
 
 _require_scratch_nocheck	# check complains about single AG fs
+_require_no_rtinherit
 _require_xfs_io_command "fpunch"
 _require_command $UUIDGEN_PROG uuidgen
 
diff --git a/tests/xfs/318 b/tests/xfs/318
index 83b858ea..a45301e2 100755
--- a/tests/xfs/318
+++ b/tests/xfs/318
@@ -33,6 +33,7 @@ _supported_fs xfs
 _require_scratch
 _require_error_injection
 _require_xfs_io_error_injection "rmap_finish_one"
+_require_no_rtinherit
 
 rm -f $seqres.full
 
diff --git a/tests/xfs/444 b/tests/xfs/444
index 6ed88715..e84b2cca 100755
--- a/tests/xfs/444
+++ b/tests/xfs/444
@@ -40,6 +40,7 @@ _require_scratch
 _require_test_program "punch-alternating"
 _require_xfs_io_command "falloc"
 _require_xfs_db_write_array
+_require_no_rtinherit
 
 # This is only a v5 filesystem problem
 _require_scratch_xfs_crc

Re: [PATCH 04/24] xfs: skip tests that rely on allocation behaviors of the data device

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> A large number of xfs-specific tests rely on specific behaviors of the
> data device allocator, such as fragmenting free space, carefully curated
> inode and free space counts, or features like filestreams that only
> exist on the data device.
> 
> These tests fail horribly if the test runner specified rtinherit=1 on
> the mkfs command line, so skip them all.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/171 |    1 +
>  tests/xfs/172 |    1 +
>  tests/xfs/173 |    1 +
>  tests/xfs/174 |    1 +
>  tests/xfs/205 |    1 +
>  tests/xfs/306 |    1 +
>  tests/xfs/318 |    1 +
>  tests/xfs/444 |    1 +
>  8 files changed, 8 insertions(+)
> 
> 
> diff --git a/tests/xfs/171 b/tests/xfs/171
> index 050d02ab..a0d6e3f9 100755
> --- a/tests/xfs/171
> +++ b/tests/xfs/171
> @@ -33,6 +33,7 @@ _supported_fs xfs
>  _supported_os Linux
>  
>  _require_scratch
> +_require_no_rtinherit
>  
>  _check_filestreams_support || _notrun "filestreams not available"
>  
> diff --git a/tests/xfs/172 b/tests/xfs/172
> index c100481c..13f1f381 100755
> --- a/tests/xfs/172
> +++ b/tests/xfs/172
> @@ -33,6 +33,7 @@ _supported_fs xfs
>  _supported_os Linux
>  
>  _require_scratch
> +_require_no_rtinherit
>  
>  _check_filestreams_support || _notrun "filestreams not available"
>  
> diff --git a/tests/xfs/173 b/tests/xfs/173
> index 1569146f..47e1d029 100755
> --- a/tests/xfs/173
> +++ b/tests/xfs/173
> @@ -33,6 +33,7 @@ _supported_fs xfs
>  _supported_os Linux
>  
>  _require_scratch
> +_require_no_rtinherit
>  
>  _check_filestreams_support || _notrun "filestreams not available"
>  
> diff --git a/tests/xfs/174 b/tests/xfs/174
> index 781a1967..348f38c3 100755
> --- a/tests/xfs/174
> +++ b/tests/xfs/174
> @@ -33,6 +33,7 @@ _supported_fs xfs
>  _supported_os Linux
>  
>  _require_scratch
> +_require_no_rtinherit
>  
>  _check_filestreams_support || _notrun "filestreams not available"
>  
> diff --git a/tests/xfs/205 b/tests/xfs/205
> index 645e509a..6f1c2058 100755
> --- a/tests/xfs/205
> +++ b/tests/xfs/205
> @@ -24,6 +24,7 @@ _supported_os Linux
>  
>  # single AG will cause xfs_repair to fail checks.
>  _require_scratch_nocheck
> +_require_no_rtinherit
>  
>  rm -f $seqres.full
>  
> diff --git a/tests/xfs/306 b/tests/xfs/306
> index edbb6076..d2a0dd7f 100755
> --- a/tests/xfs/306
> +++ b/tests/xfs/306
> @@ -35,6 +35,7 @@ _supported_fs xfs
>  _supported_os Linux
>  
>  _require_scratch_nocheck	# check complains about single AG fs
> +_require_no_rtinherit
>  _require_xfs_io_command "fpunch"
>  _require_command $UUIDGEN_PROG uuidgen
>  
> diff --git a/tests/xfs/318 b/tests/xfs/318
> index 83b858ea..a45301e2 100755
> --- a/tests/xfs/318
> +++ b/tests/xfs/318
> @@ -33,6 +33,7 @@ _supported_fs xfs
>  _require_scratch
>  _require_error_injection
>  _require_xfs_io_error_injection "rmap_finish_one"
> +_require_no_rtinherit
>  
>  rm -f $seqres.full
>  
> diff --git a/tests/xfs/444 b/tests/xfs/444
> index 6ed88715..e84b2cca 100755
> --- a/tests/xfs/444
> +++ b/tests/xfs/444
> @@ -40,6 +40,7 @@ _require_scratch
>  _require_test_program "punch-alternating"
>  _require_xfs_io_command "falloc"
>  _require_xfs_db_write_array
> +_require_no_rtinherit
>  
>  # This is only a v5 filesystem problem
>  _require_scratch_xfs_crc
> 

Re: [PATCH 04/24] xfs: skip tests that rely on allocation behaviors of the data device

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> A large number of xfs-specific tests rely on specific behaviors of the
> data device allocator, such as fragmenting free space, carefully curated
> inode and free space counts, or features like filestreams that only
> exist on the data device.
> 
> These tests fail horribly if the test runner specified rtinherit=1 on
> the mkfs command line, so skip them all.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 05/24] xfs/031: make sure we don't set rtinherit=1 on mkfs

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

mkfs.xfs does not support setting rtinherit on the root directory /and/
pre-populating the filesystem with protofiles, so don't run this test if
rtinherit is in the mkfs options.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/019 |    5 +++++
 tests/xfs/031 |    5 +++++
 2 files changed, 10 insertions(+)


diff --git a/tests/xfs/019 b/tests/xfs/019
index f42b62bb..aa5365e7 100755
--- a/tests/xfs/019
+++ b/tests/xfs/019
@@ -51,6 +51,11 @@ _supported_os Linux
 
 _require_scratch
 
+# mkfs cannot create a filesystem with protofiles if realtime is enabled, so
+# don't run this test if the rtinherit is anywhere in the mkfs options.
+echo "$MKFS_OPTIONS" | grep -q "rtinherit" && \
+	_notrun "Cannot mkfs with a protofile and -d rtinherit."
+
 protofile=$tmp.proto
 tempfile=$tmp.file
 
diff --git a/tests/xfs/031 b/tests/xfs/031
index 6403fd40..671b6727 100755
--- a/tests/xfs/031
+++ b/tests/xfs/031
@@ -81,6 +81,11 @@ _supported_os Linux
 _require_scratch
 _require_no_large_scratch_dev
 
+# mkfs cannot create a filesystem with protofiles if realtime is enabled, so
+# don't run this test if the rtinherit is anywhere in the mkfs options.
+echo "$MKFS_OPTIONS" | grep -q "rtinherit" && \
+	_notrun "Cannot mkfs with a protofile and -d rtinherit."
+
 # sanity test - default + one root directory entry
 # Note: must do this proto/mkfs now for later inode size calcs
 _create_proto 0

Re: [PATCH 05/24] xfs/031: make sure we don't set rtinherit=1 on mkfs

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:27PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> mkfs.xfs does not support setting rtinherit on the root directory /and/
> pre-populating the filesystem with protofiles, so don't run this test if
> rtinherit is in the mkfs options.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/019 |    5 +++++
>  tests/xfs/031 |    5 +++++
>  2 files changed, 10 insertions(+)
> 
> 
> diff --git a/tests/xfs/019 b/tests/xfs/019
> index f42b62bb..aa5365e7 100755
> --- a/tests/xfs/019
> +++ b/tests/xfs/019
> @@ -51,6 +51,11 @@ _supported_os Linux
>  
>  _require_scratch
>  
> +# mkfs cannot create a filesystem with protofiles if realtime is enabled, so
> +# don't run this test if the rtinherit is anywhere in the mkfs options.
> +echo "$MKFS_OPTIONS" | grep -q "rtinherit" && \
> +	_notrun "Cannot mkfs with a protofile and -d rtinherit."
> +
>  protofile=$tmp.proto
>  tempfile=$tmp.file
>  
> diff --git a/tests/xfs/031 b/tests/xfs/031
> index 6403fd40..671b6727 100755
> --- a/tests/xfs/031
> +++ b/tests/xfs/031
> @@ -81,6 +81,11 @@ _supported_os Linux
>  _require_scratch
>  _require_no_large_scratch_dev
>  
> +# mkfs cannot create a filesystem with protofiles if realtime is enabled, so
> +# don't run this test if the rtinherit is anywhere in the mkfs options.
> +echo "$MKFS_OPTIONS" | grep -q "rtinherit" && \
> +	_notrun "Cannot mkfs with a protofile and -d rtinherit."
> +
>  # sanity test - default + one root directory entry
>  # Note: must do this proto/mkfs now for later inode size calcs
>  _create_proto 0
> 

Re: [PATCH 05/24] xfs/031: make sure we don't set rtinherit=1 on mkfs

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:27PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> mkfs.xfs does not support setting rtinherit on the root directory /and/
> pre-populating the filesystem with protofiles, so don't run this test if
> rtinherit is in the mkfs options.

That is a bit of a weird limitation.  Any reason we can't fix this in
mkfs instead?

Re: [PATCH 05/24] xfs/031: make sure we don't set rtinherit=1 on mkfs

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 08:53:51AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 06:43:27PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > mkfs.xfs does not support setting rtinherit on the root directory /and/
> > pre-populating the filesystem with protofiles, so don't run this test if
> > rtinherit is in the mkfs options.
> 
> That is a bit of a weird limitation.  Any reason we can't fix this in
> mkfs instead?

Userspace doesn't implement the rt allocator at all, and the last few
times I've tried to do any serious surgery in the protofile code, Dave
grumbled that we should just kill it off instead.

Do people actually /use/ protofile support?  Do they like it?  Or would
they rather have an option like mke2fs -D, where you point it at a
directory and it uses ftw to copy-in all the files in that directory
tree, attributes and all?

(I guess I could propose deprecating it and see who pops out. :P)

--D

Re: [PATCH 05/24] xfs/031: make sure we don't set rtinherit=1 on mkfs

[Christoph Hellwig]

On Thu, Sep 17, 2020 at 08:57:47AM -0700, Darrick J. Wong wrote:
> On Thu, Sep 17, 2020 at 08:53:51AM +0100, Christoph Hellwig wrote:
> > On Mon, Sep 14, 2020 at 06:43:27PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > mkfs.xfs does not support setting rtinherit on the root directory /and/
> > > pre-populating the filesystem with protofiles, so don't run this test if
> > > rtinherit is in the mkfs options.
> > 
> > That is a bit of a weird limitation.  Any reason we can't fix this in
> > mkfs instead?
> 
> Userspace doesn't implement the rt allocator at all, and the last few
> times I've tried to do any serious surgery in the protofile code, Dave
> grumbled that we should just kill it off instead.

Maybe killing it off is indeed the better option.  And in that case this
patch to disable it for RT devices would be a good start.

> 
> Do people actually /use/ protofile support?  Do they like it?  Or would
> they rather have an option like mke2fs -D, where you point it at a
> directory and it uses ftw to copy-in all the files in that directory
> tree, attributes and all?

I always found the protofile stuff really weird and not actually useful.

mke2fs -D actually does seem useful if you want to generate an xfs
file systems and fill it without having admin privileges.  I think that
is what the mke2fs feature is used for.

[PATCH 06/24] xfs: wrap xfs_db calls to the test device

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a _test_xfs_db analogue to _scratch_xfs_db so that we can
encapsulate whatever strange test fs options were fed to us by the test
runner.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/xfs    |   13 +++++++++++++
 tests/xfs/003 |   14 +++++++-------
 tests/xfs/016 |    4 ++--
 tests/xfs/045 |    2 +-
 4 files changed, 23 insertions(+), 10 deletions(-)


diff --git a/common/xfs b/common/xfs
index d9a9784f..eab5e292 100644
--- a/common/xfs
+++ b/common/xfs
@@ -218,6 +218,19 @@ _scratch_xfs_db()
 	$XFS_DB_PROG "$@" $(_scratch_xfs_db_options)
 }
 
+_test_xfs_db_options()
+{
+	TEST_OPTIONS=""
+	[ "$USE_EXTERNAL" = yes -a ! -z "$TEST_LOGDEV" ] && \
+		TEST_OPTIONS="-l$TEST_LOGDEV"
+	echo $TEST_OPTIONS $* $TEST_DEV
+}
+
+_test_xfs_db()
+{
+	$XFS_DB_PROG "$@" $(_test_xfs_db_options)
+}
+
 _scratch_xfs_admin()
 {
 	local options=("$SCRATCH_DEV")
diff --git a/tests/xfs/003 b/tests/xfs/003
index 1ce3e5ce..736bf206 100755
--- a/tests/xfs/003
+++ b/tests/xfs/003
@@ -38,32 +38,32 @@ test_done()
 # real QA test starts here
 
 echo "=== TEST 1 ==="
-xfs_db -r -c 'pop' -c 'type sb' $TEST_DEV
+_test_xfs_db -r -c 'pop' -c 'type sb'
 test_done
 
 echo "=== TEST 2 ==="
-xfs_db -r -c 'push sb' $TEST_DEV
+_test_xfs_db -r -c 'push sb'
 test_done
 
 echo "=== TEST 3 ==="
-xfs_db -r -c 'pop' -c 'push sb' $TEST_DEV
+_test_xfs_db -r -c 'pop' -c 'push sb'
 test_done
 
 echo "=== TEST 4 ==="
-xfs_db -r -c 'type sb' -c 'print' $TEST_DEV
+_test_xfs_db -r -c 'type sb' -c 'print'
 test_done
 
 echo "=== TEST 5 ==="
-xfs_db -r -c 'inode 128' -c 'push' -c 'type' $TEST_DEV >$tmp.out 2>&1
+_test_xfs_db -r -c 'inode 128' -c 'push' -c 'type' >$tmp.out 2>&1
 test_done
 if ! grep -q "current type is \"inode\"" $tmp.out; then
     cat $tmp.out
 fi
 
 echo "=== TEST 6 ==="
-xfs_db -r -c 'sb' -c 'a' $TEST_DEV >$tmp.out 2>&1 # don't care about output
+_test_xfs_db -r -c 'sb' -c 'a' >$tmp.out 2>&1 # don't care about output
 test_done
 
 echo "=== TEST 7 ==="
-xfs_db -r -c 'ring' $TEST_DEV
+_test_xfs_db -r -c 'ring'
 test_done
diff --git a/tests/xfs/016 b/tests/xfs/016
index 2b41127e..e1b1b6b8 100755
--- a/tests/xfs/016
+++ b/tests/xfs/016
@@ -149,7 +149,7 @@ _log_sunit()
 
 _after_log()
 {
-    xfs_db -r $1 -c "sb" -c "print" | $AWK_PROG '
+    _scratch_xfs_db -r -c "sb" -c "print" | $AWK_PROG '
         /logstart/  { logstart = $3 }
         /logblocks/ { logblocks = $3 }
         END {
@@ -163,7 +163,7 @@ _check_corrupt()
     f="c6c6c6c6"
     echo "*** check for corruption"
     echo "expect $f..." >>$seqres.full
-    xfs_db -r -c "fsblock $2" -c "print" $1 | head | tee -a $seqres.full | \
+    _scratch_xfs_db -r -c "fsblock $2" -c "print" | head | tee -a $seqres.full | \
         grep -q -v "$f $f $f $f $f $f $f $f" && \
             _fail "!!! block $2 corrupted!"
 }
diff --git a/tests/xfs/045 b/tests/xfs/045
index 0c9e1b79..6a44da56 100755
--- a/tests/xfs/045
+++ b/tests/xfs/045
@@ -21,7 +21,7 @@ trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
 
 _get_existing_uuid()
 {
-    xfs_db -r $TEST_DEV -c "uuid" | $AWK_PROG '/^UUID/ { print $3 }'
+	_test_xfs_db -r -c "uuid" | $AWK_PROG '/^UUID/ { print $3 }'
 }
 
 # real QA test starts here

Re: [PATCH 06/24] xfs: wrap xfs_db calls to the test device

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create a _test_xfs_db analogue to _scratch_xfs_db so that we can
> encapsulate whatever strange test fs options were fed to us by the test
> runner.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/xfs    |   13 +++++++++++++
>  tests/xfs/003 |   14 +++++++-------
>  tests/xfs/016 |    4 ++--
>  tests/xfs/045 |    2 +-
>  4 files changed, 23 insertions(+), 10 deletions(-)
> 
> 
> diff --git a/common/xfs b/common/xfs
> index d9a9784f..eab5e292 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -218,6 +218,19 @@ _scratch_xfs_db()
>  	$XFS_DB_PROG "$@" $(_scratch_xfs_db_options)
>  }
>  
> +_test_xfs_db_options()
> +{
> +	TEST_OPTIONS=""
> +	[ "$USE_EXTERNAL" = yes -a ! -z "$TEST_LOGDEV" ] && \
> +		TEST_OPTIONS="-l$TEST_LOGDEV"
> +	echo $TEST_OPTIONS $* $TEST_DEV
> +}
> +
> +_test_xfs_db()
> +{
> +	$XFS_DB_PROG "$@" $(_test_xfs_db_options)
> +}
> +
>  _scratch_xfs_admin()
>  {
>  	local options=("$SCRATCH_DEV")
> diff --git a/tests/xfs/003 b/tests/xfs/003
> index 1ce3e5ce..736bf206 100755
> --- a/tests/xfs/003
> +++ b/tests/xfs/003
> @@ -38,32 +38,32 @@ test_done()
>  # real QA test starts here
>  
>  echo "=== TEST 1 ==="
> -xfs_db -r -c 'pop' -c 'type sb' $TEST_DEV
> +_test_xfs_db -r -c 'pop' -c 'type sb'
>  test_done
>  
>  echo "=== TEST 2 ==="
> -xfs_db -r -c 'push sb' $TEST_DEV
> +_test_xfs_db -r -c 'push sb'
>  test_done
>  
>  echo "=== TEST 3 ==="
> -xfs_db -r -c 'pop' -c 'push sb' $TEST_DEV
> +_test_xfs_db -r -c 'pop' -c 'push sb'
>  test_done
>  
>  echo "=== TEST 4 ==="
> -xfs_db -r -c 'type sb' -c 'print' $TEST_DEV
> +_test_xfs_db -r -c 'type sb' -c 'print'
>  test_done
>  
>  echo "=== TEST 5 ==="
> -xfs_db -r -c 'inode 128' -c 'push' -c 'type' $TEST_DEV >$tmp.out 2>&1
> +_test_xfs_db -r -c 'inode 128' -c 'push' -c 'type' >$tmp.out 2>&1
>  test_done
>  if ! grep -q "current type is \"inode\"" $tmp.out; then
>      cat $tmp.out
>  fi
>  
>  echo "=== TEST 6 ==="
> -xfs_db -r -c 'sb' -c 'a' $TEST_DEV >$tmp.out 2>&1 # don't care about output
> +_test_xfs_db -r -c 'sb' -c 'a' >$tmp.out 2>&1 # don't care about output
>  test_done
>  
>  echo "=== TEST 7 ==="
> -xfs_db -r -c 'ring' $TEST_DEV
> +_test_xfs_db -r -c 'ring'
>  test_done
> diff --git a/tests/xfs/016 b/tests/xfs/016
> index 2b41127e..e1b1b6b8 100755
> --- a/tests/xfs/016
> +++ b/tests/xfs/016
> @@ -149,7 +149,7 @@ _log_sunit()
>  
>  _after_log()
>  {
> -    xfs_db -r $1 -c "sb" -c "print" | $AWK_PROG '
> +    _scratch_xfs_db -r -c "sb" -c "print" | $AWK_PROG '
>          /logstart/  { logstart = $3 }
>          /logblocks/ { logblocks = $3 }
>          END {
> @@ -163,7 +163,7 @@ _check_corrupt()
>      f="c6c6c6c6"
>      echo "*** check for corruption"
>      echo "expect $f..." >>$seqres.full
> -    xfs_db -r -c "fsblock $2" -c "print" $1 | head | tee -a $seqres.full | \
> +    _scratch_xfs_db -r -c "fsblock $2" -c "print" | head | tee -a $seqres.full | \
>          grep -q -v "$f $f $f $f $f $f $f $f" && \
>              _fail "!!! block $2 corrupted!"
>  }
> diff --git a/tests/xfs/045 b/tests/xfs/045
> index 0c9e1b79..6a44da56 100755
> --- a/tests/xfs/045
> +++ b/tests/xfs/045
> @@ -21,7 +21,7 @@ trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
>  
>  _get_existing_uuid()
>  {
> -    xfs_db -r $TEST_DEV -c "uuid" | $AWK_PROG '/^UUID/ { print $3 }'
> +	_test_xfs_db -r -c "uuid" | $AWK_PROG '/^UUID/ { print $3 }'
>  }
>  
>  # real QA test starts here
> 

Re: [PATCH 06/24] xfs: wrap xfs_db calls to the test device

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create a _test_xfs_db analogue to _scratch_xfs_db so that we can
> encapsulate whatever strange test fs options were fed to us by the test
> runner.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 07/24] xfs/{111,137}: replace open-coded calls to repair with _scratch_xfs_repair

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/111 |    2 +-
 tests/xfs/137 |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)


diff --git a/tests/xfs/111 b/tests/xfs/111
index 0db33cb8..1f778966 100755
--- a/tests/xfs/111
+++ b/tests/xfs/111
@@ -62,7 +62,7 @@ $XFS_FSR_PROG -g $SCRATCH_MNT | _filter_scratch
 
 # Fix up intentional corruption so test can pass
 _scratch_unmount
-$XFS_REPAIR_PROG $SCRATCH_DEV >> $seqres.full 2>&1
+_scratch_xfs_repair >> $seqres.full 2>&1
 
 status=0
 exit
diff --git a/tests/xfs/137 b/tests/xfs/137
index 79106c66..c10860bb 100755
--- a/tests/xfs/137
+++ b/tests/xfs/137
@@ -64,11 +64,11 @@ else
 fi
 
 # verify that repair detects invalid LSNs as well
-$XFS_REPAIR_PROG -n $SCRATCH_DEV >> $seqres.full 2>&1 || \
+_scratch_xfs_repair -n >> $seqres.full 2>&1 || \
 	echo repair failure detected
 
 # repair for real so the post-test check can verify repair fixed things up
-$XFS_REPAIR_PROG $SCRATCH_DEV >> $seqres.full 2>&1
+_scratch_xfs_repair >> $seqres.full 2>&1
 
 # success, all done
 status=0

Re: [PATCH 07/24] xfs/{111,137}: replace open-coded calls to repair with _scratch_xfs_repair

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:40PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/111 |    2 +-
>  tests/xfs/137 |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/tests/xfs/111 b/tests/xfs/111
> index 0db33cb8..1f778966 100755
> --- a/tests/xfs/111
> +++ b/tests/xfs/111
> @@ -62,7 +62,7 @@ $XFS_FSR_PROG -g $SCRATCH_MNT | _filter_scratch
>  
>  # Fix up intentional corruption so test can pass
>  _scratch_unmount
> -$XFS_REPAIR_PROG $SCRATCH_DEV >> $seqres.full 2>&1
> +_scratch_xfs_repair >> $seqres.full 2>&1
>  
>  status=0
>  exit
> diff --git a/tests/xfs/137 b/tests/xfs/137
> index 79106c66..c10860bb 100755
> --- a/tests/xfs/137
> +++ b/tests/xfs/137
> @@ -64,11 +64,11 @@ else
>  fi
>  
>  # verify that repair detects invalid LSNs as well
> -$XFS_REPAIR_PROG -n $SCRATCH_DEV >> $seqres.full 2>&1 || \
> +_scratch_xfs_repair -n >> $seqres.full 2>&1 || \
>  	echo repair failure detected
>  
>  # repair for real so the post-test check can verify repair fixed things up
> -$XFS_REPAIR_PROG $SCRATCH_DEV >> $seqres.full 2>&1
> +_scratch_xfs_repair >> $seqres.full 2>&1
>  
>  # success, all done
>  status=0
> 

Re: [PATCH 07/24] xfs/{111,137}: replace open-coded calls to repair with _scratch_xfs_repair

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 08/24] xfs: replace open-coded calls to xfs_logprint with helpers

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use the test/scratch xfs_logprint helpers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/135 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/tests/xfs/135 b/tests/xfs/135
index cc505980..9783cfc2 100755
--- a/tests/xfs/135
+++ b/tests/xfs/135
@@ -48,7 +48,7 @@ for i in 16 32 64 128 256; do
 	_scratch_xfs_db -x -c "logformat -c 3 -s $lsunit" | \
 		tee -a $seqres.full
 	# don't redirect error output so it causes test failure
-	$XFS_LOGPRINT_PROG $SCRATCH_DEV >> $seqres.full
+	_scratch_xfs_logprint >> $seqres.full
 done
 
 # success, all done

Re: [PATCH 08/24] xfs: replace open-coded calls to xfs_logprint with helpers

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the test/scratch xfs_logprint helpers.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/135 |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/135 b/tests/xfs/135
> index cc505980..9783cfc2 100755
> --- a/tests/xfs/135
> +++ b/tests/xfs/135
> @@ -48,7 +48,7 @@ for i in 16 32 64 128 256; do
>  	_scratch_xfs_db -x -c "logformat -c 3 -s $lsunit" | \
>  		tee -a $seqres.full
>  	# don't redirect error output so it causes test failure
> -	$XFS_LOGPRINT_PROG $SCRATCH_DEV >> $seqres.full
> +	_scratch_xfs_logprint >> $seqres.full
>  done
>  
>  # success, all done
> 

Re: [PATCH 08/24] xfs: replace open-coded calls to xfs_logprint with helpers

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:47PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the test/scratch xfs_logprint helpers.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 09/24] xfs/070: add scratch log device options to direct repair invocation

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/070 |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/070 b/tests/xfs/070
index 5d52a830..313864b7 100755
--- a/tests/xfs/070
+++ b/tests/xfs/070
@@ -41,9 +41,11 @@ _cleanup()
 _xfs_repair_noscan()
 {
 	# invoke repair directly so we can kill the process if need be
+	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+		log_repair_opts="-l $SCRATCH_LOGDEV"
 	[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
 		rt_repair_opts="-r $SCRATCH_RTDEV"
-	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 |
+	$XFS_REPAIR_PROG $log_repair_opts $rt_repair_opts $SCRATCH_DEV 2>&1 |
 		tee -a $seqres.full > $tmp.repair &
 	repair_pid=$!
 

Re: [PATCH 09/24] xfs/070: add scratch log device options to direct repair invocation

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/070 |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/070 b/tests/xfs/070
> index 5d52a830..313864b7 100755
> --- a/tests/xfs/070
> +++ b/tests/xfs/070
> @@ -41,9 +41,11 @@ _cleanup()
>  _xfs_repair_noscan()
>  {
>  	# invoke repair directly so we can kill the process if need be
> +	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> +		log_repair_opts="-l $SCRATCH_LOGDEV"
>  	[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
>  		rt_repair_opts="-r $SCRATCH_RTDEV"
> -	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 |
> +	$XFS_REPAIR_PROG $log_repair_opts $rt_repair_opts $SCRATCH_DEV 2>&1 |
>  		tee -a $seqres.full > $tmp.repair &

Why not use _scratch_xfs_repair at here?

Thanks,
Zorro

>  	repair_pid=$!
>  
> 

Re: [PATCH 09/24] xfs/070: add scratch log device options to direct repair invocation

[Darrick J. Wong]

On Wed, Sep 16, 2020 at 10:42:47AM +0800, Zorro Lang wrote:
> On Mon, Sep 14, 2020 at 06:43:53PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/xfs/070 |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > 
> > diff --git a/tests/xfs/070 b/tests/xfs/070
> > index 5d52a830..313864b7 100755
> > --- a/tests/xfs/070
> > +++ b/tests/xfs/070
> > @@ -41,9 +41,11 @@ _cleanup()
> >  _xfs_repair_noscan()
> >  {
> >  	# invoke repair directly so we can kill the process if need be
> > +	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> > +		log_repair_opts="-l $SCRATCH_LOGDEV"
> >  	[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
> >  		rt_repair_opts="-r $SCRATCH_RTDEV"
> > -	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 |
> > +	$XFS_REPAIR_PROG $log_repair_opts $rt_repair_opts $SCRATCH_DEV 2>&1 |
> >  		tee -a $seqres.full > $tmp.repair &
> 
> Why not use _scratch_xfs_repair at here?
> 
> Thanks,
> Zorro
> 
> >  	repair_pid=$!

        ^^^^^^^^^^^^^
Because this test needs to hang on to the pid of the repair process in
order to kill it, which you can't do if do if you use the wrapper.

--D

> >  
> > 
> 

Re: [PATCH 09/24] xfs/070: add scratch log device options to direct repair invocation

[Zorro Lang]

On Tue, Sep 15, 2020 at 08:42:01PM -0700, Darrick J. Wong wrote:
> On Wed, Sep 16, 2020 at 10:42:47AM +0800, Zorro Lang wrote:
> > On Mon, Sep 14, 2020 at 06:43:53PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > ---
> > >  tests/xfs/070 |    4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > 
> > > diff --git a/tests/xfs/070 b/tests/xfs/070
> > > index 5d52a830..313864b7 100755
> > > --- a/tests/xfs/070
> > > +++ b/tests/xfs/070
> > > @@ -41,9 +41,11 @@ _cleanup()
> > >  _xfs_repair_noscan()
> > >  {
> > >  	# invoke repair directly so we can kill the process if need be
> > > +	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> > > +		log_repair_opts="-l $SCRATCH_LOGDEV"
> > >  	[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
> > >  		rt_repair_opts="-r $SCRATCH_RTDEV"
> > > -	$XFS_REPAIR_PROG $rt_repair_opts $SCRATCH_DEV 2>&1 |
> > > +	$XFS_REPAIR_PROG $log_repair_opts $rt_repair_opts $SCRATCH_DEV 2>&1 |
> > >  		tee -a $seqres.full > $tmp.repair &
> > 
> > Why not use _scratch_xfs_repair at here?
> > 
> > Thanks,
> > Zorro
> > 
> > >  	repair_pid=$!
> 
>         ^^^^^^^^^^^^^

Oh, right! That's good to me now.
Reviewed-by: Zorro Lang <zlang@redhat.com>

> Because this test needs to hang on to the pid of the repair process in
> order to kill it, which you can't do if do if you use the wrapper.
> 
> --D
> 
> > >  
> > > 
> > 
> 

Re: [PATCH 09/24] xfs/070: add scratch log device options to direct repair invocation

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:53PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 10/24] xfs: add a _require_xfs_copy helper

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Add a _require helper so that tests can ensure that they're running in
the correct environment for xfs_copy to work (no external devices).

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/xfs    |    7 +++++++
 tests/xfs/032 |    4 +---
 tests/xfs/073 |    5 +----
 tests/xfs/077 |    3 +--
 tests/xfs/284 |    3 +--
 tests/xfs/503 |    1 +
 6 files changed, 12 insertions(+), 11 deletions(-)


diff --git a/common/xfs b/common/xfs
index eab5e292..f4a47dfb 100644
--- a/common/xfs
+++ b/common/xfs
@@ -945,3 +945,10 @@ _try_wipe_scratch_xfs()
 	fi
 	rm -f $tmp.mkfs
 }
+
+_require_xfs_copy()
+{
+	[ -n "$XFS_COPY_PROG" ] || _notrun "xfs_copy binary not yet installed"
+	[ "$USE_EXTERNAL" = yes ] && \
+		_notrun "Cannot xfs_copy with external devices"
+}
diff --git a/tests/xfs/032 b/tests/xfs/032
index af18f755..8ece957d 100755
--- a/tests/xfs/032
+++ b/tests/xfs/032
@@ -25,9 +25,7 @@ _supported_os Linux
 
 _require_scratch
 _require_test_program "feature"
-
-[ "$USE_EXTERNAL" = yes ] && _notrun "Cannot xfs_copy with external devices"
-[ -n "$XFS_COPY_PROG" ] || _notrun "xfs_copy binary not yet installed"
+_require_xfs_copy
 
 rm -f $seqres.full
 
diff --git a/tests/xfs/073 b/tests/xfs/073
index 6939e102..dc0cde4f 100755
--- a/tests/xfs/073
+++ b/tests/xfs/073
@@ -113,10 +113,7 @@ _supported_fs xfs
 _supported_os Linux
 _require_test
 _require_attrs
-
-[ "$USE_EXTERNAL" = yes ] && _notrun "Cannot xfs_copy with external devices"
-[ -n "$XFS_COPY_PROG" ] || _notrun "xfs_copy binary not yet installed"
-
+_require_xfs_copy
 _require_scratch
 _require_loop
 
diff --git a/tests/xfs/077 b/tests/xfs/077
index 02bcc9f7..b11b2c07 100755
--- a/tests/xfs/077
+++ b/tests/xfs/077
@@ -35,8 +35,7 @@ _cleanup()
 
 _supported_fs xfs
 _supported_os Linux
-# xfs_copy does not support realtime devices
-_require_no_realtime
+_require_xfs_copy
 _require_scratch
 _require_no_large_scratch_dev
 _require_xfs_crc
diff --git a/tests/xfs/284 b/tests/xfs/284
index 7af77634..8f997e17 100755
--- a/tests/xfs/284
+++ b/tests/xfs/284
@@ -34,8 +34,7 @@ rm -f $seqres.full
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
-# xfs_copy does not support realtime devices
-_require_no_realtime
+_require_xfs_copy
 _require_test
 _require_scratch
 _require_no_large_scratch_dev
diff --git a/tests/xfs/503 b/tests/xfs/503
index 41754353..756bcda3 100755
--- a/tests/xfs/503
+++ b/tests/xfs/503
@@ -33,6 +33,7 @@ testdir=$TEST_DIR/test-$seq
 _supported_os Linux
 _supported_fs xfs
 
+_require_xfs_copy
 _require_scratch_nocheck
 _require_populate_commands
 

Re: [PATCH 10/24] xfs: add a _require_xfs_copy helper

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:43:59PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a _require helper so that tests can ensure that they're running in
> the correct environment for xfs_copy to work (no external devices).
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/xfs    |    7 +++++++
>  tests/xfs/032 |    4 +---
>  tests/xfs/073 |    5 +----
>  tests/xfs/077 |    3 +--
>  tests/xfs/284 |    3 +--
>  tests/xfs/503 |    1 +
>  6 files changed, 12 insertions(+), 11 deletions(-)
> 
> 
> diff --git a/common/xfs b/common/xfs
> index eab5e292..f4a47dfb 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -945,3 +945,10 @@ _try_wipe_scratch_xfs()
>  	fi
>  	rm -f $tmp.mkfs
>  }
> +
> +_require_xfs_copy()
> +{
> +	[ -n "$XFS_COPY_PROG" ] || _notrun "xfs_copy binary not yet installed"
> +	[ "$USE_EXTERNAL" = yes ] && \
> +		_notrun "Cannot xfs_copy with external devices"
> +}
> diff --git a/tests/xfs/032 b/tests/xfs/032
> index af18f755..8ece957d 100755
> --- a/tests/xfs/032
> +++ b/tests/xfs/032
> @@ -25,9 +25,7 @@ _supported_os Linux
>  
>  _require_scratch
>  _require_test_program "feature"
> -
> -[ "$USE_EXTERNAL" = yes ] && _notrun "Cannot xfs_copy with external devices"
> -[ -n "$XFS_COPY_PROG" ] || _notrun "xfs_copy binary not yet installed"
> +_require_xfs_copy
>  
>  rm -f $seqres.full
>  
> diff --git a/tests/xfs/073 b/tests/xfs/073
> index 6939e102..dc0cde4f 100755
> --- a/tests/xfs/073
> +++ b/tests/xfs/073
> @@ -113,10 +113,7 @@ _supported_fs xfs
>  _supported_os Linux
>  _require_test
>  _require_attrs
> -
> -[ "$USE_EXTERNAL" = yes ] && _notrun "Cannot xfs_copy with external devices"
> -[ -n "$XFS_COPY_PROG" ] || _notrun "xfs_copy binary not yet installed"
> -
> +_require_xfs_copy
>  _require_scratch
>  _require_loop
>  
> diff --git a/tests/xfs/077 b/tests/xfs/077
> index 02bcc9f7..b11b2c07 100755
> --- a/tests/xfs/077
> +++ b/tests/xfs/077
> @@ -35,8 +35,7 @@ _cleanup()
>  
>  _supported_fs xfs
>  _supported_os Linux
> -# xfs_copy does not support realtime devices
> -_require_no_realtime
> +_require_xfs_copy
>  _require_scratch
>  _require_no_large_scratch_dev
>  _require_xfs_crc
> diff --git a/tests/xfs/284 b/tests/xfs/284
> index 7af77634..8f997e17 100755
> --- a/tests/xfs/284
> +++ b/tests/xfs/284
> @@ -34,8 +34,7 @@ rm -f $seqres.full
>  # real QA test starts here
>  _supported_fs xfs
>  _supported_os Linux
> -# xfs_copy does not support realtime devices
> -_require_no_realtime
> +_require_xfs_copy
>  _require_test
>  _require_scratch
>  _require_no_large_scratch_dev
> diff --git a/tests/xfs/503 b/tests/xfs/503
> index 41754353..756bcda3 100755
> --- a/tests/xfs/503
> +++ b/tests/xfs/503
> @@ -33,6 +33,7 @@ testdir=$TEST_DIR/test-$seq
>  _supported_os Linux
>  _supported_fs xfs
>  
> +_require_xfs_copy
>  _require_scratch_nocheck
>  _require_populate_commands
>  
> 

Re: [PATCH 10/24] xfs: add a _require_xfs_copy helper

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:43:59PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a _require helper so that tests can ensure that they're running in
> the correct environment for xfs_copy to work (no external devices).

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 11/24] overlay/{069,071}: fix undefined variables

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Change OVL_BASE_TEST_MNT -> OVL_BASE_TEST_DIR, since the former is not
actually defined anywhere.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/overlay/069 |    2 +-
 tests/overlay/071 |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/tests/overlay/069 b/tests/overlay/069
index 77dfce63..f7eaec2d 100755
--- a/tests/overlay/069
+++ b/tests/overlay/069
@@ -53,7 +53,7 @@ _require_test_program "open_by_handle"
 _require_scratch_overlay_features index nfs_export redirect_dir
 
 # Lower overlay lower layer is on test fs, upper is on scratch fs
-lower=$OVL_BASE_TEST_MNT/$OVL_LOWER-$seq
+lower=$OVL_BASE_TEST_DIR/$OVL_LOWER-$seq
 upper=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 work=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
 
diff --git a/tests/overlay/071 b/tests/overlay/071
index e083c29d..043fc4d3 100755
--- a/tests/overlay/071
+++ b/tests/overlay/071
@@ -56,7 +56,7 @@ _require_scratch_overlay_features index nfs_export redirect_dir
 _require_loop
 
 # Lower overlay lower layer is on test fs, upper is on scratch fs
-lower=$OVL_BASE_TEST_MNT/$OVL_LOWER-$seq
+lower=$OVL_BASE_TEST_DIR/$OVL_LOWER-$seq
 upper=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 work=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
 # Lower dir of nested overlay is the scratch overlay mount at SCRATCH_MNT

Re: [PATCH 11/24] overlay/{069,071}: fix undefined variables

[Amir Goldstein]

On Tue, Sep 15, 2020 at 4:44 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Change OVL_BASE_TEST_MNT -> OVL_BASE_TEST_DIR, since the former is not
> actually defined anywhere.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Oops...
Thanks for fixing this.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

[PATCH 12/24] overlay/020: make sure the system supports the required namespaces

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Don't run this test if the kernel doesn't support namespaces.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/overlay/020 |    6 ++++++
 1 file changed, 6 insertions(+)


diff --git a/tests/overlay/020 b/tests/overlay/020
index 85488b83..9029f042 100755
--- a/tests/overlay/020
+++ b/tests/overlay/020
@@ -32,10 +32,16 @@ rm -f $seqres.full
 
 # real QA test starts here
 
+require_unshare() {
+	unshare -f -r "$@" true &>/dev/null || \
+		_notrun "unshare $@: not supported"
+}
+
 # Modify as appropriate.
 _supported_fs overlay
 _supported_os Linux
 _require_scratch
+require_unshare -m -p -U
 
 # Remove all files from previous tests
 _scratch_mkfs

Re: [PATCH 12/24] overlay/020: make sure the system supports the required namespaces

[Amir Goldstein]

On Tue, Sep 15, 2020 at 4:44 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Don't run this test if the kernel doesn't support namespaces.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

>  tests/overlay/020 |    6 ++++++
>  1 file changed, 6 insertions(+)
>
>
> diff --git a/tests/overlay/020 b/tests/overlay/020
> index 85488b83..9029f042 100755
> --- a/tests/overlay/020
> +++ b/tests/overlay/020
> @@ -32,10 +32,16 @@ rm -f $seqres.full
>
>  # real QA test starts here
>
> +require_unshare() {
> +       unshare -f -r "$@" true &>/dev/null || \
> +               _notrun "unshare $@: not supported"
> +}
> +

I guess we can defer making this common until xfstests catches up on
testing FS_USERNS_MOUNT...

Thanks,
Amir.

[PATCH 13/24] generic/204: don't flood stdout with ENOSPC messages on an ENOSPC test

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

This test has been on and off my bad list for many years due to the fact
that it will spew potentially millions of "No space left on device"
errors if the file count calculations are wrong.  The calculations
should be correct for the XFS data device, but they don't apply to other
filesystems.

Therefore, filter out the ENOSPC messages when the files are not going
to be created on the xfs data device (e.g. ext4, xfs realtime, etc.)

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/204 |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


diff --git a/tests/generic/204 b/tests/generic/204
index 349f5eff..7250c00a 100755
--- a/tests/generic/204
+++ b/tests/generic/204
@@ -76,9 +76,13 @@ files=$((space / (direntlen + isize + dbsize)))
 echo files $files, resvblks $resv_blks >> $seqres.full
 _scratch_resvblks $resv_blks >> $seqres.full 2>&1
 
+filter() {
+	test $FSTYP != xfs && sed -e '/No space left on device/d'
+}
+
 for i in `seq -w 1 $files`; do
-    echo -n > $SCRATCH_MNT/$i
-    echo XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX > $SCRATCH_MNT/$i
+	(echo -n > $SCRATCH_MNT/$i;
+	 echo XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX > $SCRATCH_MNT/$i) 2>&1 | filter
 done
 
 # success, all done

Re: [PATCH 13/24] generic/204: don't flood stdout with ENOSPC messages on an ENOSPC test

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:44:18PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test has been on and off my bad list for many years due to the fact
> that it will spew potentially millions of "No space left on device"
> errors if the file count calculations are wrong.  The calculations
> should be correct for the XFS data device, but they don't apply to other
> filesystems.
> 
> Therefore, filter out the ENOSPC messages when the files are not going
> to be created on the xfs data device (e.g. ext4, xfs realtime, etc.)
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/generic/204 |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/tests/generic/204 b/tests/generic/204
> index 349f5eff..7250c00a 100755
> --- a/tests/generic/204
> +++ b/tests/generic/204
> @@ -76,9 +76,13 @@ files=$((space / (direntlen + isize + dbsize)))
>  echo files $files, resvblks $resv_blks >> $seqres.full
>  _scratch_resvblks $resv_blks >> $seqres.full 2>&1
>  
> +filter() {
> +	test $FSTYP != xfs && sed -e '/No space left on device/d'
> +}
> +
>  for i in `seq -w 1 $files`; do
> -    echo -n > $SCRATCH_MNT/$i
> -    echo XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX > $SCRATCH_MNT/$i
> +	(echo -n > $SCRATCH_MNT/$i;
> +	 echo XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX > $SCRATCH_MNT/$i) 2>&1 | filter
>  done
>  
>  # success, all done
> 

Re: [PATCH 13/24] generic/204: don't flood stdout with ENOSPC messages on an ENOSPC test

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:44:18PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test has been on and off my bad list for many years due to the fact
> that it will spew potentially millions of "No space left on device"
> errors if the file count calculations are wrong.  The calculations
> should be correct for the XFS data device, but they don't apply to other
> filesystems.
> 
> Therefore, filter out the ENOSPC messages when the files are not going
> to be created on the xfs data device (e.g. ext4, xfs realtime, etc.)

Should this move to an xfs specific test instead?

Re: [PATCH 13/24] generic/204: don't flood stdout with ENOSPC messages on an ENOSPC test

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 08:56:35AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 06:44:18PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > This test has been on and off my bad list for many years due to the fact
> > that it will spew potentially millions of "No space left on device"
> > errors if the file count calculations are wrong.  The calculations
> > should be correct for the XFS data device, but they don't apply to other
> > filesystems.
> > 
> > Therefore, filter out the ENOSPC messages when the files are not going
> > to be created on the xfs data device (e.g. ext4, xfs realtime, etc.)
> 
> Should this move to an xfs specific test instead?

I'm on the fence about that, but probably.  I've though that generic/
can have a test that formats a small fs and fills it with files until it
hits ENOSPC, and this test (with its weird calculations) can move to xfs/.

But I haven't been around long enough to know if there's a specific
reason why we calculate the number of files to create?  Is this test
really a regression test for some long-ago bug?  Or that weird noalloc
dir update mode thing that we ripped out last year?

--D

[PATCH 14/24] common/xfs: extract minimum log size message from mkfs correctly

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Modify the command that searches for the minimum log size message from
mkfs to handle external log devices correctly.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/xfs |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


diff --git a/common/xfs b/common/xfs
index f4a47dfb..6520ad29 100644
--- a/common/xfs
+++ b/common/xfs
@@ -103,7 +103,7 @@ _scratch_find_xfs_min_logblocks()
 	# try again without MKFS_OPTIONS because that's what _scratch_do_mkfs
 	# will do if we pass in the log size option.
 	if [ $mkfs_status -ne 0 ] &&
-	   ! grep -q 'log size.*too small, minimum' $tmp.mkfserr; then
+	   ! egrep -q '(log size.*too small, minimum|external log device.*too small, must be)' $tmp.mkfserr; then
 		eval "$mkfs_cmd $extra_mkfs_options $SCRATCH_DEV" \
 			2>$tmp.mkfserr 1>$tmp.mkfsstd
 		mkfs_status=$?
@@ -126,6 +126,12 @@ _scratch_find_xfs_min_logblocks()
 		rm -f $tmp.mkfsstd $tmp.mkfserr
 		return
 	fi
+	if grep -q 'external log device.*too small, must be' $tmp.mkfserr; then
+		grep 'external log device.*too small, must be' $tmp.mkfserr | \
+			sed -e 's/^.*must be at least \([0-9]*\) blocks/\1/g'
+		rm -f $tmp.mkfsstd $tmp.mkfserr
+		return
+	fi
 
 	# Don't know what to do, so fail
 	echo "Cannot determine minimum log size" >&2

Re: [PATCH 14/24] common/xfs: extract minimum log size message from mkfs correctly

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:44:24PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Modify the command that searches for the minimum log size message from
> mkfs to handle external log devices correctly.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/xfs |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/common/xfs b/common/xfs
> index f4a47dfb..6520ad29 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -103,7 +103,7 @@ _scratch_find_xfs_min_logblocks()
>  	# try again without MKFS_OPTIONS because that's what _scratch_do_mkfs
>  	# will do if we pass in the log size option.
>  	if [ $mkfs_status -ne 0 ] &&
> -	   ! grep -q 'log size.*too small, minimum' $tmp.mkfserr; then
> +	   ! egrep -q '(log size.*too small, minimum|external log device.*too small, must be)' $tmp.mkfserr; then
>  		eval "$mkfs_cmd $extra_mkfs_options $SCRATCH_DEV" \
>  			2>$tmp.mkfserr 1>$tmp.mkfsstd
>  		mkfs_status=$?
> @@ -126,6 +126,12 @@ _scratch_find_xfs_min_logblocks()
>  		rm -f $tmp.mkfsstd $tmp.mkfserr
>  		return
>  	fi
> +	if grep -q 'external log device.*too small, must be' $tmp.mkfserr; then
> +		grep 'external log device.*too small, must be' $tmp.mkfserr | \
> +			sed -e 's/^.*must be at least \([0-9]*\) blocks/\1/g'
> +		rm -f $tmp.mkfsstd $tmp.mkfserr
> +		return
> +	fi
>  
>  	# Don't know what to do, so fail
>  	echo "Cannot determine minimum log size" >&2
> 

Re: [PATCH 14/24] common/xfs: extract minimum log size message from mkfs correctly

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:44:24PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Modify the command that searches for the minimum log size message from
> mkfs to handle external log devices correctly.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 15/24] xfs/194: actually check if we got 512-byte blocks before proceeding

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

This test has specific fs block size requirements, so make sure that's
what we got before we proceed with the test.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/194 |    3 +++
 1 file changed, 3 insertions(+)


diff --git a/tests/xfs/194 b/tests/xfs/194
index 9001a146..90b74c8f 100755
--- a/tests/xfs/194
+++ b/tests/xfs/194
@@ -84,6 +84,9 @@ unset XFS_MKFS_OPTIONS
 # we need 512 byte block size, so crc's are turned off
 _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
 _scratch_mount
+test "$(_get_block_size $SCRATCH_MNT)" = 512 || \
+	_notrun "Could not get 512-byte blocks"
+
 
 # 512b block / 4k page example:
 #

Re: [PATCH 15/24] xfs/194: actually check if we got 512-byte blocks before proceeding

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:44:31PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test has specific fs block size requirements, so make sure that's
> what we got before we proceed with the test.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  tests/xfs/194 |    3 +++
>  1 file changed, 3 insertions(+)
> 
> 
> diff --git a/tests/xfs/194 b/tests/xfs/194
> index 9001a146..90b74c8f 100755
> --- a/tests/xfs/194
> +++ b/tests/xfs/194
> @@ -84,6 +84,9 @@ unset XFS_MKFS_OPTIONS
>  # we need 512 byte block size, so crc's are turned off
>  _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
>  _scratch_mount
> +test "$(_get_block_size $SCRATCH_MNT)" = 512 || \
> +	_notrun "Could not get 512-byte blocks"
> +

If this case is only for 512 byte blocksize, the "blksize=`expr $pgsize / 8`"
and all things with the "$blksize" looks redundant, right?

Thanks,
Zorro

>  
>  # 512b block / 4k page example:
>  #
> 

Re: [PATCH 15/24] xfs/194: actually check if we got 512-byte blocks before proceeding

[Darrick J. Wong]

On Wed, Sep 16, 2020 at 07:34:00PM +0800, Zorro Lang wrote:
> On Mon, Sep 14, 2020 at 06:44:31PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > This test has specific fs block size requirements, so make sure that's
> > what we got before we proceed with the test.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  tests/xfs/194 |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > 
> > diff --git a/tests/xfs/194 b/tests/xfs/194
> > index 9001a146..90b74c8f 100755
> > --- a/tests/xfs/194
> > +++ b/tests/xfs/194
> > @@ -84,6 +84,9 @@ unset XFS_MKFS_OPTIONS
> >  # we need 512 byte block size, so crc's are turned off
> >  _scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1
> >  _scratch_mount
> > +test "$(_get_block_size $SCRATCH_MNT)" = 512 || \
> > +	_notrun "Could not get 512-byte blocks"
> > +
> 
> If this case is only for 512 byte blocksize, the "blksize=`expr $pgsize / 8`"
> and all things with the "$blksize" looks redundant, right?

Hmm... in hindsight I shouldn't have hardwired the right hand argument.
This needs only to be a simple equivalence test:

test "$(_get_block_size $SCRATCH_MNT)" = $blksize || \
	_notrun "Could not get $blksize-byte blocks"

Will fix and resend.

--D

> Thanks,
> Zorro
> 
> >  
> >  # 512b block / 4k page example:
> >  #
> > 
> 

[PATCH 16/24] xfs/098: adapt to external log devices

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Teach this test to deal with external log devices correctly.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/098 |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/098 b/tests/xfs/098
index c63b9d60..b7d3253e 100755
--- a/tests/xfs/098
+++ b/tests/xfs/098
@@ -76,7 +76,13 @@ echo "+ corrupt image"
 logstart="$(_scratch_xfs_get_sb_field logstart)"
 logstart="$(_scratch_xfs_db -c "convert fsblock ${logstart} byte" | sed -e 's/^.*(\([0-9]*\).*$/\1/g')"
 logblocks="$(_scratch_xfs_get_sb_field logblocks)"
-$XFS_IO_PROG -f -c "pwrite -S 0x62 ${logstart} $((logblocks * blksz))" "${SCRATCH_DEV}" >> $seqres.full
+
+if [ "$USE_EXTERNAL" = yes ] && [ ! -z "$SCRATCH_LOGDEV" ]; then
+	logdev=$SCRATCH_LOGDEV
+else
+	logdev=$SCRATCH_DEV
+fi
+$XFS_IO_PROG -f -c "pwrite -S 0x62 ${logstart} $((logblocks * blksz))" $logdev >> $seqres.full
 
 echo "+ mount image"
 _try_scratch_mount 2>/dev/null && _fail "mount should not succeed"

Re: [PATCH 16/24] xfs/098: adapt to external log devices

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:44:37PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Teach this test to deal with external log devices correctly.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/098 |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/098 b/tests/xfs/098
> index c63b9d60..b7d3253e 100755
> --- a/tests/xfs/098
> +++ b/tests/xfs/098
> @@ -76,7 +76,13 @@ echo "+ corrupt image"
>  logstart="$(_scratch_xfs_get_sb_field logstart)"
>  logstart="$(_scratch_xfs_db -c "convert fsblock ${logstart} byte" | sed -e 's/^.*(\([0-9]*\).*$/\1/g')"
>  logblocks="$(_scratch_xfs_get_sb_field logblocks)"
> -$XFS_IO_PROG -f -c "pwrite -S 0x62 ${logstart} $((logblocks * blksz))" "${SCRATCH_DEV}" >> $seqres.full
> +
> +if [ "$USE_EXTERNAL" = yes ] && [ ! -z "$SCRATCH_LOGDEV" ]; then
> +	logdev=$SCRATCH_LOGDEV
> +else
> +	logdev=$SCRATCH_DEV
> +fi
> +$XFS_IO_PROG -f -c "pwrite -S 0x62 ${logstart} $((logblocks * blksz))" $logdev >> $seqres.full
>  
>  echo "+ mount image"
>  _try_scratch_mount 2>/dev/null && _fail "mount should not succeed"
> 

Re: [PATCH 16/24] xfs/098: adapt to external log devices

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:44:37PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Teach this test to deal with external log devices correctly.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 17/24] xfs: refactor _xfs_check calls to the scratch device

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Use _scratch_xfs_check, not _xfs_check $SCRATCH_DEV.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/291 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/tests/xfs/291 b/tests/xfs/291
index 8a4b1354..adef2536 100755
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -102,7 +102,7 @@ done
 _scratch_unmount
 # Can xfs_repair and xfs_check cope with this monster?
 _scratch_xfs_repair >> $seqres.full 2>&1 || _fail "xfs_repair failed"
-_xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
+_scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
 
 # Yes they can!  Now...
 # Can xfs_metadump cope with this monster?

Re: [PATCH 17/24] xfs: refactor _xfs_check calls to the scratch device

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:44:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use _scratch_xfs_check, not _xfs_check $SCRATCH_DEV.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/291 |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/291 b/tests/xfs/291
> index 8a4b1354..adef2536 100755
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -102,7 +102,7 @@ done
>  _scratch_unmount
>  # Can xfs_repair and xfs_check cope with this monster?
>  _scratch_xfs_repair >> $seqres.full 2>&1 || _fail "xfs_repair failed"
> -_xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
> +_scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
>  
>  # Yes they can!  Now...
>  # Can xfs_metadump cope with this monster?
> 

Re: [PATCH 17/24] xfs: refactor _xfs_check calls to the scratch device

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:44:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use _scratch_xfs_check, not _xfs_check $SCRATCH_DEV.

This looks ok:

Reviewed-by: Christoph Hellwig <hch@lst.de>

But shouldn't we finally kill off all xfs_check alls instead? :)

Re: [PATCH 17/24] xfs: refactor _xfs_check calls to the scratch device

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 08:58:56AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 06:44:43PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Use _scratch_xfs_check, not _xfs_check $SCRATCH_DEV.
> 
> This looks ok:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But shouldn't we finally kill off all xfs_check alls instead? :)

I will some day, but I keep finding more bugs and missed stuff in
xfs_repair. :(

--D

[PATCH 18/24] xfs/291: fix open-coded repair call to mdrestore'd fs image

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Attach any external log devices to the open-coded repair call.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/291 |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/291 b/tests/xfs/291
index adef2536..5d4f1ac4 100755
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -110,7 +110,9 @@ _scratch_metadump $tmp.metadump || _fail "xfs_metadump failed"
 xfs_mdrestore $tmp.metadump $tmp.img || _fail "xfs_mdrestore failed"
 [ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
 	rt_repair_opts="-r $SCRATCH_RTDEV"
-$XFS_REPAIR_PROG $rt_repair_opts -f $tmp.img >> $seqres.full 2>&1 || \
+[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_LOGDEV" ] && \
+	log_repair_opts="-l $SCRATCH_LOGDEV"
+$XFS_REPAIR_PROG $rt_repair_opts $log_repair_opts -f $tmp.img >> $seqres.full 2>&1 || \
 	_fail "xfs_repair of metadump failed"
 
 # Yes it can; success, all done

Re: [PATCH 18/24] xfs/291: fix open-coded repair call to mdrestore'd fs image

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:44:50PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Attach any external log devices to the open-coded repair call.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/291 |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/291 b/tests/xfs/291
> index adef2536..5d4f1ac4 100755
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -110,7 +110,9 @@ _scratch_metadump $tmp.metadump || _fail "xfs_metadump failed"
>  xfs_mdrestore $tmp.metadump $tmp.img || _fail "xfs_mdrestore failed"
>  [ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_RTDEV" ] && \
>  	rt_repair_opts="-r $SCRATCH_RTDEV"
> -$XFS_REPAIR_PROG $rt_repair_opts -f $tmp.img >> $seqres.full 2>&1 || \
> +[ "$USE_EXTERNAL" = yes ] && [ -n "$SCRATCH_LOGDEV" ] && \
> +	log_repair_opts="-l $SCRATCH_LOGDEV"
> +$XFS_REPAIR_PROG $rt_repair_opts $log_repair_opts -f $tmp.img >> $seqres.full 2>&1 || \
>  	_fail "xfs_repair of metadump failed"
>  
>  # Yes it can; success, all done
> 

[PATCH 19/24] xfs/424: disable external devices

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

This test uses an open-coded call to mkfs, so we need to disable the
external devices so that _scratch_xfs_db doesn't get confused.  We also
disable the post-check fsck because it's run by the parent ./check
program, which won't know that we didn't use the external devices.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/424 |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/424 b/tests/xfs/424
index 66d79458..4907cf0f 100755
--- a/tests/xfs/424
+++ b/tests/xfs/424
@@ -47,7 +47,13 @@ rm -f $seqres.full
 # Modify as appropriate
 _supported_os Linux
 _supported_fs xfs
-_require_scratch
+
+# Since we have an open-coded mkfs call, disable the external devices and
+# don't let the post-check fsck actually run since it'll trip over us not
+# using the external devices.
+_require_scratch_nocheck
+export SCRATCH_LOGDEV=
+export SCRATCH_RTDEV=
 
 echo "Silence is golden."
 

Re: [PATCH 19/24] xfs/424: disable external devices

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:44:56PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test uses an open-coded call to mkfs, so we need to disable the
> external devices so that _scratch_xfs_db doesn't get confused.  We also
> disable the post-check fsck because it's run by the parent ./check
> program, which won't know that we didn't use the external devices.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/424 |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/424 b/tests/xfs/424
> index 66d79458..4907cf0f 100755
> --- a/tests/xfs/424
> +++ b/tests/xfs/424
> @@ -47,7 +47,13 @@ rm -f $seqres.full
>  # Modify as appropriate
>  _supported_os Linux
>  _supported_fs xfs
> -_require_scratch
> +
> +# Since we have an open-coded mkfs call, disable the external devices and
> +# don't let the post-check fsck actually run since it'll trip over us not
> +# using the external devices.
> +_require_scratch_nocheck
> +export SCRATCH_LOGDEV=
> +export SCRATCH_RTDEV=
>  
>  echo "Silence is golden."
>  
> 

Re: [PATCH 19/24] xfs/424: disable external devices

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:44:56PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This test uses an open-coded call to mkfs, so we need to disable the
> external devices so that _scratch_xfs_db doesn't get confused.  We also
> disable the post-check fsck because it's run by the parent ./check
> program, which won't know that we didn't use the external devices.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 20/24] xfs/449: fix xfs info report output if realtime device specified

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Modify the mkfs.xfs output so that "realtime =/dev/XXX" becomes
"realtime =external" so that the output will match xfs_db, which doesn't
take a rt device argument and thus does not know.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/449 |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/449 b/tests/xfs/449
index 83c3c493..938dbe28 100755
--- a/tests/xfs/449
+++ b/tests/xfs/449
@@ -46,7 +46,9 @@ cat $tmp.mkfs >> $seqres.full
 _scratch_xfs_db -c "info" > $tmp.dbinfo
 echo DB >> $seqres.full
 cat $tmp.dbinfo >> $seqres.full
-diff -u $tmp.mkfs $tmp.dbinfo
+# xfs_db doesn't take a rtdev argument, so it reports "realtime=external".
+# mkfs does, so make a quick substitution
+diff -u <(cat $tmp.mkfs | sed -e 's/realtime =\/.*extsz=/realtime =external               extsz=/g') $tmp.dbinfo
 
 _scratch_mount
 

Re: [PATCH 20/24] xfs/449: fix xfs info report output if realtime device specified

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:45:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Modify the mkfs.xfs output so that "realtime =/dev/XXX" becomes
> "realtime =external" so that the output will match xfs_db, which doesn't
> take a rt device argument and thus does not know.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/449 |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/449 b/tests/xfs/449
> index 83c3c493..938dbe28 100755
> --- a/tests/xfs/449
> +++ b/tests/xfs/449
> @@ -46,7 +46,9 @@ cat $tmp.mkfs >> $seqres.full
>  _scratch_xfs_db -c "info" > $tmp.dbinfo
>  echo DB >> $seqres.full
>  cat $tmp.dbinfo >> $seqres.full
> -diff -u $tmp.mkfs $tmp.dbinfo
> +# xfs_db doesn't take a rtdev argument, so it reports "realtime=external".
> +# mkfs does, so make a quick substitution
> +diff -u <(cat $tmp.mkfs | sed -e 's/realtime =\/.*extsz=/realtime =external               extsz=/g') $tmp.dbinfo
>  
>  _scratch_mount
>  
> 

Re: [PATCH 20/24] xfs/449: fix xfs info report output if realtime device specified

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:45:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Modify the mkfs.xfs output so that "realtime =/dev/XXX" becomes
> "realtime =external" so that the output will match xfs_db, which doesn't
> take a rt device argument and thus does not know.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 21/24] common/rc: teach _scratch_mkfs_sized to set a size on an xfs realtime volume

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Generally speaking, tests that call _scratch_mkfs_sized are trying to
constrain a test's run time by formatting a filesystem that's smaller
than the device.  The current helper does this for the scratch device,
but it doesn't do this for the xfs realtime volume.

If fstests has been configured to create files on the realtime device by
default ("-d rtinherit=1) then those tests that want to run with a small
volume size will instead be running with a huge realtime device.  This
makes certain tests take forever to run, so apply the same sizing to the
rt volume if one exists.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/rc |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/common/rc b/common/rc
index f78b1cfc..b2d45fa2 100644
--- a/common/rc
+++ b/common/rc
@@ -976,14 +976,20 @@ _scratch_mkfs_sized()
 	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
     fi
 
+    if [ "$HOSTOS" == "Linux" ] && [ "$FSTYP" = "xfs" ] && [ -b "$SCRATCH_RTDEV" ]; then
+	local rtdevsize=`blockdev --getsize64 $SCRATCH_RTDEV`
+	[ "$fssize" -gt "$rtdevsize" ] && _notrun "Scratch rt device too small"
+	rt_ops="-r size=$fssize"
+    fi
+
     case $FSTYP in
     xfs)
 	# don't override MKFS_OPTIONS that set a block size.
 	echo $MKFS_OPTIONS |egrep -q "b?size="
 	if [ $? -eq 0 ]; then
-		_scratch_mkfs_xfs -d size=$fssize
+		_scratch_mkfs_xfs -d size=$fssize $rt_ops
 	else
-		_scratch_mkfs_xfs -d size=$fssize -b size=$blocksize
+		_scratch_mkfs_xfs -d size=$fssize $rt_ops -b size=$blocksize
 	fi
 	;;
     ext2|ext3|ext4|ext4dev)

Re: [PATCH 21/24] common/rc: teach _scratch_mkfs_sized to set a size on an xfs realtime volume

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:45:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Generally speaking, tests that call _scratch_mkfs_sized are trying to
> constrain a test's run time by formatting a filesystem that's smaller
> than the device.  The current helper does this for the scratch device,
> but it doesn't do this for the xfs realtime volume.
> 
> If fstests has been configured to create files on the realtime device by
> default ("-d rtinherit=1) then those tests that want to run with a small
> volume size will instead be running with a huge realtime device.  This
> makes certain tests take forever to run, so apply the same sizing to the
> rt volume if one exists.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/rc |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index f78b1cfc..b2d45fa2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -976,14 +976,20 @@ _scratch_mkfs_sized()
>  	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
>      fi
>  
> +    if [ "$HOSTOS" == "Linux" ] && [ "$FSTYP" = "xfs" ] && [ -b "$SCRATCH_RTDEV" ]; then
> +	local rtdevsize=`blockdev --getsize64 $SCRATCH_RTDEV`
> +	[ "$fssize" -gt "$rtdevsize" ] && _notrun "Scratch rt device too small"
> +	rt_ops="-r size=$fssize"
> +    fi
> +
>      case $FSTYP in
>      xfs)
>  	# don't override MKFS_OPTIONS that set a block size.
>  	echo $MKFS_OPTIONS |egrep -q "b?size="
>  	if [ $? -eq 0 ]; then
> -		_scratch_mkfs_xfs -d size=$fssize
> +		_scratch_mkfs_xfs -d size=$fssize $rt_ops
>  	else
> -		_scratch_mkfs_xfs -d size=$fssize -b size=$blocksize
> +		_scratch_mkfs_xfs -d size=$fssize $rt_ops -b size=$blocksize
>  	fi
>  	;;
>      ext2|ext3|ext4|ext4dev)
> 

Re: [PATCH 21/24] common/rc: teach _scratch_mkfs_sized to set a size on an xfs realtime volume

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:45:08PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Generally speaking, tests that call _scratch_mkfs_sized are trying to
> constrain a test's run time by formatting a filesystem that's smaller
> than the device.  The current helper does this for the scratch device,
> but it doesn't do this for the xfs realtime volume.
> 
> If fstests has been configured to create files on the realtime device by
> default ("-d rtinherit=1) then those tests that want to run with a small
> volume size will instead be running with a huge realtime device.  This
> makes certain tests take forever to run, so apply the same sizing to the
> rt volume if one exists.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  common/rc |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index f78b1cfc..b2d45fa2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -976,14 +976,20 @@ _scratch_mkfs_sized()
>  	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
>      fi
>  
> +    if [ "$HOSTOS" == "Linux" ] && [ "$FSTYP" = "xfs" ] && [ -b "$SCRATCH_RTDEV" ]; then
> +	local rtdevsize=`blockdev --getsize64 $SCRATCH_RTDEV`
> +	[ "$fssize" -gt "$rtdevsize" ] && _notrun "Scratch rt device too small"
> +	rt_ops="-r size=$fssize"
> +    fi

The indentation here looks rather weird.  I also don't think we need
the HOSTOS check.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 21/24] common/rc: teach _scratch_mkfs_sized to set a size on an xfs realtime volume

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 09:00:54AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 06:45:08PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Generally speaking, tests that call _scratch_mkfs_sized are trying to
> > constrain a test's run time by formatting a filesystem that's smaller
> > than the device.  The current helper does this for the scratch device,
> > but it doesn't do this for the xfs realtime volume.
> > 
> > If fstests has been configured to create files on the realtime device by
> > default ("-d rtinherit=1) then those tests that want to run with a small
> > volume size will instead be running with a huge realtime device.  This
> > makes certain tests take forever to run, so apply the same sizing to the
> > rt volume if one exists.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  common/rc |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/common/rc b/common/rc
> > index f78b1cfc..b2d45fa2 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -976,14 +976,20 @@ _scratch_mkfs_sized()
> >  	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
> >      fi
> >  
> > +    if [ "$HOSTOS" == "Linux" ] && [ "$FSTYP" = "xfs" ] && [ -b "$SCRATCH_RTDEV" ]; then
> > +	local rtdevsize=`blockdev --getsize64 $SCRATCH_RTDEV`
> > +	[ "$fssize" -gt "$rtdevsize" ] && _notrun "Scratch rt device too small"
> > +	rt_ops="-r size=$fssize"
> > +    fi
> 
> The indentation here looks rather weird.  I also don't think we need
> the HOSTOS check.

<nod> it's copy-pastaing the clause above it.  I guess I could just send
an indentation cleanup for that, since it's a bit fugly.

--D

> 
> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v2 21/24] common/rc: teach _scratch_mkfs_sized to set a size on an xfs realtime volume

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Generally speaking, tests that call _scratch_mkfs_sized are trying to
constrain a test's run time by formatting a filesystem that's smaller
than the device.  The current helper does this for the scratch device,
but it doesn't do this for the xfs realtime volume.

If fstests has been configured to create files on the realtime device by
default ("-d rtinherit=1) then those tests that want to run with a small
volume size will instead be running with a huge realtime device.  This
makes certain tests take forever to run, so apply the same sizing to the
rt volume if one exists.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: don't bother with HOSTOS
---
 common/rc |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 6366a7b2..b707db76 100644
--- a/common/rc
+++ b/common/rc
@@ -997,14 +997,20 @@ _scratch_mkfs_sized()
 	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
     fi
 
+    if [ "$FSTYP" = "xfs" ] && [ -b "$SCRATCH_RTDEV" ]; then
+	local rtdevsize=`blockdev --getsize64 $SCRATCH_RTDEV`
+	[ "$fssize" -gt "$rtdevsize" ] && _notrun "Scratch rt device too small"
+	rt_ops="-r size=$fssize"
+    fi
+
     case $FSTYP in
     xfs)
 	# don't override MKFS_OPTIONS that set a block size.
 	echo $MKFS_OPTIONS |egrep -q "b?size="
 	if [ $? -eq 0 ]; then
-		_scratch_mkfs_xfs -d size=$fssize
+		_scratch_mkfs_xfs -d size=$fssize $rt_ops
 	else
-		_scratch_mkfs_xfs -d size=$fssize -b size=$blocksize
+		_scratch_mkfs_xfs -d size=$fssize $rt_ops -b size=$blocksize
 	fi
 	;;
     ext2|ext3|ext4|ext4dev)

[PATCH 22/24] xfs/141: run for longer with TIME_FACTOR

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Allow the test runner to run the crash loop in this test for longer by
setting TIME_FACTOR.  This has been useful for finding bugs in log
recovery.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/xfs/141 |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/tests/xfs/141 b/tests/xfs/141
index 754ca37a..ef7210ff 100755
--- a/tests/xfs/141
+++ b/tests/xfs/141
@@ -50,7 +50,8 @@ _scratch_mount
 
 sdev=$(_short_dev $SCRATCH_DEV)
 
-for i in $(seq 1 5); do
+nr_times=$((TIME_FACTOR * 5))
+while [ $nr_times -gt 0 ]; do
 	# Enable error injection. Use a random bad crc factor up to 100
 	# (increase this value to run fsstress longer).
 	factor=$((RANDOM % 100 + 1))
@@ -65,6 +66,7 @@ for i in $(seq 1 5); do
 	# write.
 	_scratch_unmount
 	_scratch_mount
+	nr_times=$((nr_times - 1))
 done
 
 # success, all done

Re: [PATCH 22/24] xfs/141: run for longer with TIME_FACTOR

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:45:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Allow the test runner to run the crash loop in this test for longer by
> setting TIME_FACTOR.  This has been useful for finding bugs in log
> recovery.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/141 |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/141 b/tests/xfs/141
> index 754ca37a..ef7210ff 100755
> --- a/tests/xfs/141
> +++ b/tests/xfs/141
> @@ -50,7 +50,8 @@ _scratch_mount
>  
>  sdev=$(_short_dev $SCRATCH_DEV)
>  
> -for i in $(seq 1 5); do
> +nr_times=$((TIME_FACTOR * 5))
> +while [ $nr_times -gt 0 ]; do
>  	# Enable error injection. Use a random bad crc factor up to 100
>  	# (increase this value to run fsstress longer).
>  	factor=$((RANDOM % 100 + 1))
> @@ -65,6 +66,7 @@ for i in $(seq 1 5); do
>  	# write.
>  	_scratch_unmount
>  	_scratch_mount
> +	nr_times=$((nr_times - 1))
>  done
>  
>  # success, all done
> 

Re: [PATCH 22/24] xfs/141: run for longer with TIME_FACTOR

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:45:15PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Allow the test runner to run the crash loop in this test for longer by
> setting TIME_FACTOR.  This has been useful for finding bugs in log
> recovery.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 23/24] generic/204: sync before scrub hits EIO

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Let's see if we can prevent fs corruption warnings by flushing dirty
data to disk before the test ends.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/204 |    1 +
 1 file changed, 1 insertion(+)


diff --git a/tests/generic/204 b/tests/generic/204
index 7250c00a..22d6fed5 100755
--- a/tests/generic/204
+++ b/tests/generic/204
@@ -18,6 +18,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _cleanup()
 {
 	rm -f $tmp.*
+	sync
 }
 
 # get standard environment, filters and checks

Re: [PATCH 23/24] generic/204: sync before scrub hits EIO

[Zorro Lang]

On Mon, Sep 14, 2020 at 06:45:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Let's see if we can prevent fs corruption warnings by flushing dirty
> data to disk before the test ends.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

At least it won't break more things:)
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/generic/204 |    1 +
>  1 file changed, 1 insertion(+)
> 
> 
> diff --git a/tests/generic/204 b/tests/generic/204
> index 7250c00a..22d6fed5 100755
> --- a/tests/generic/204
> +++ b/tests/generic/204
> @@ -18,6 +18,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  _cleanup()
>  {
>  	rm -f $tmp.*
> +	sync
>  }
>  
>  # get standard environment, filters and checks
> 

Re: [PATCH 23/24] generic/204: sync before scrub hits EIO

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:45:21PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Let's see if we can prevent fs corruption warnings by flushing dirty
> data to disk before the test ends.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 24/24] check: try reloading modules

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Optionally reload the module between each test to try to pinpoint slab
cache errors and whatnot.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 README |    3 +++
 check  |    9 +++++++++
 2 files changed, 12 insertions(+)


diff --git a/README b/README
index d0e23fcd..4af331b4 100644
--- a/README
+++ b/README
@@ -106,6 +106,9 @@ Preparing system for tests:
              - set USE_KMEMLEAK=yes to scan for memory leaks in the kernel
                after every test, if the kernel supports kmemleak.
              - set KEEP_DMESG=yes to keep dmesg log after test
+             - Set TEST_FS_MODULE_RELOAD=1 to unload the module and reload
+               it between test invocations.  This assumes that the name of
+               the module is the same as FSTYP.
 
         - or add a case to the switch in common/config assigning
           these variables based on the hostname of your test
diff --git a/check b/check
index 5ffa8777..29306262 100755
--- a/check
+++ b/check
@@ -810,6 +810,15 @@ function run_section()
 			_check_dmesg || err=true
 		fi
 
+		# Reload the module after each test to check for leaks or
+		# other problems.
+		if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
+			_test_unmount 2> /dev/null
+			_scratch_unmount 2> /dev/null
+			modprobe -r $FSTYP
+			modprobe $FSTYP
+		fi
+
 		# Scan for memory leaks after every test so that associating
 		# a leak to a particular test will be as accurate as possible.
 		_check_kmemleak || err=true

Re: [PATCH 24/24] check: try reloading modules

[Amir Goldstein]

On Tue, Sep 15, 2020 at 4:47 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Optionally reload the module between each test to try to pinpoint slab
> cache errors and whatnot.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  README |    3 +++
>  check  |    9 +++++++++
>  2 files changed, 12 insertions(+)
>
>
> diff --git a/README b/README
> index d0e23fcd..4af331b4 100644
> --- a/README
> +++ b/README
> @@ -106,6 +106,9 @@ Preparing system for tests:
>               - set USE_KMEMLEAK=yes to scan for memory leaks in the kernel
>                 after every test, if the kernel supports kmemleak.
>               - set KEEP_DMESG=yes to keep dmesg log after test
> +             - Set TEST_FS_MODULE_RELOAD=1 to unload the module and reload
> +               it between test invocations.  This assumes that the name of
> +               the module is the same as FSTYP.
>
>          - or add a case to the switch in common/config assigning
>            these variables based on the hostname of your test
> diff --git a/check b/check
> index 5ffa8777..29306262 100755
> --- a/check
> +++ b/check
> @@ -810,6 +810,15 @@ function run_section()
>                         _check_dmesg || err=true
>                 fi
>
> +               # Reload the module after each test to check for leaks or
> +               # other problems.
> +               if [ -n "${TEST_FS_MODULE_RELOAD}" ]; then
> +                       _test_unmount 2> /dev/null
> +                       _scratch_unmount 2> /dev/null
> +                       modprobe -r $FSTYP
> +                       modprobe $FSTYP

It is safer to use fs-$FSTYP namespaced module alias.

Thanks,
Amir.

[PATCH 26/24] common: drop HOSTOS

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

We effectively support only Linux these days, so drop most of the
special casing of HOSTOS.  We'll retain the simple check just in case
someone tries to run this on a different operating system.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/config |   50 ++++++++++++++++++++++----------------------------
 common/rc     |   20 ++------------------
 common/xfs    |    2 +-
 3 files changed, 25 insertions(+), 47 deletions(-)

diff --git a/common/config b/common/config
index 1fe1e0e1..3de87ea7 100644
--- a/common/config
+++ b/common/config
@@ -46,7 +46,7 @@ export LC_ALL=C
 PATH=".:$PATH"
 
 export HOST=`hostname -s`
-export HOSTOS=`uname -s`
+test `uname -s` = "Linux" || _fatal "fstests only supports Linux"
 
 export MODULAR=0               # using XFS as a module or not
 export BOOT="/boot"            # install target for kernels
@@ -143,7 +143,7 @@ export PS_ALL_FLAGS="-ef"
 
 export DF_PROG="$(type -P df)"
 [ "$DF_PROG" = "" ] && _fatal "df not found"
-[ "$HOSTOS" = "Linux" ] && export DF_PROG="$DF_PROG -T -P"
+export DF_PROG="$DF_PROG -T -P"	# Linux
 
 export XFS_IO_PROG="$(type -P xfs_io)"
 [ "$XFS_IO_PROG" = "" ] && _fatal "xfs_io not found"
@@ -242,29 +242,25 @@ if [ "$UDEV_SETTLE_PROG" == "" ]; then
 fi
 export UDEV_SETTLE_PROG
 
-case "$HOSTOS" in
-    Linux)
-	export MKFS_XFS_PROG=$(type -P mkfs.xfs)
-	export MKFS_EXT4_PROG=$(type -P mkfs.ext4)
-	export MKFS_UDF_PROG=$(type -P mkudffs)
-	export MKFS_BTRFS_PROG=$(set_mkfs_prog_path_with_opts btrfs)
-	export MKFS_F2FS_PROG=$(set_mkfs_prog_path_with_opts f2fs)
-	export DUMP_F2FS_PROG=$(type -P dump.f2fs)
-	export F2FS_IO_PROG=$(type -P f2fs_io)
-	export BTRFS_UTIL_PROG=$(type -P btrfs)
-	export BTRFS_SHOW_SUPER_PROG=$(type -P btrfs-show-super)
-	export BTRFS_CONVERT_PROG=$(type -P btrfs-convert)
-	export BTRFS_TUNE_PROG=$(type -P btrfstune)
-	export XFS_FSR_PROG=$(type -P xfs_fsr)
-	export MKFS_NFS_PROG="false"
-	export MKFS_CIFS_PROG="false"
-	export MKFS_OVERLAY_PROG="false"
-	export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
-	export E2FSCK_PROG=$(type -P e2fsck)
-	export TUNE2FS_PROG=$(type -P tune2fs)
-	export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)
-        ;;
-esac
+export MKFS_XFS_PROG=$(type -P mkfs.xfs)
+export MKFS_EXT4_PROG=$(type -P mkfs.ext4)
+export MKFS_UDF_PROG=$(type -P mkudffs)
+export MKFS_BTRFS_PROG=$(set_mkfs_prog_path_with_opts btrfs)
+export MKFS_F2FS_PROG=$(set_mkfs_prog_path_with_opts f2fs)
+export DUMP_F2FS_PROG=$(type -P dump.f2fs)
+export F2FS_IO_PROG=$(type -P f2fs_io)
+export BTRFS_UTIL_PROG=$(type -P btrfs)
+export BTRFS_SHOW_SUPER_PROG=$(type -P btrfs-show-super)
+export BTRFS_CONVERT_PROG=$(type -P btrfs-convert)
+export BTRFS_TUNE_PROG=$(type -P btrfstune)
+export XFS_FSR_PROG=$(type -P xfs_fsr)
+export MKFS_NFS_PROG="false"
+export MKFS_CIFS_PROG="false"
+export MKFS_OVERLAY_PROG="false"
+export MKFS_REISER4_PROG=$(type -P mkfs.reiser4)
+export E2FSCK_PROG=$(type -P e2fsck)
+export TUNE2FS_PROG=$(type -P tune2fs)
+export FSCK_OVERLAY_PROG=$(type -P fsck.overlay)
 
 # SELinux adds extra xattrs which can mess up our expected output.
 # So, mount with a context, and they won't be created.
@@ -753,9 +749,7 @@ if [ -z "$CONFIG_INCLUDED" ]; then
 
 	# Autodetect fs type based on what's on $TEST_DEV unless it's been set
 	# externally
-	if [ -z "$FSTYP" ] && \
-	   [ "$HOSTOS" == "Linux" -o "$OSTYPE" == "linux-gnu" ] && \
-	   [ ! -z "$TEST_DEV" ]; then
+	if [ -z "$FSTYP" ] && [ ! -z "$TEST_DEV" ]; then
 		FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV`
 	fi
 	FSTYP=${FSTYP:=xfs}
diff --git a/common/rc b/common/rc
index bbafc73d..3adeeefe 100644
--- a/common/rc
+++ b/common/rc
@@ -49,19 +49,13 @@ _math()
 
 dd()
 {
-   if [ "$HOSTOS" == "Linux" ]
-   then	
 	command dd --help 2>&1 | grep noxfer >/dev/null
-	
 	if [ "$?" -eq 0 ]
 	    then
 		command dd status=noxfer $@
 	    else
 		command dd $@
-    	fi
-   else
-	command dd $@
-   fi
+	fi
 }
 
 # Prints the md5 checksum of a given file
@@ -992,7 +986,7 @@ _scratch_mkfs_sized()
 
     local blocks=`expr $fssize / $blocksize`
 
-    if [ "$HOSTOS" == "Linux" -a -b "$SCRATCH_DEV" ]; then
+    if [ -b "$SCRATCH_DEV" ]; then
 	local devsize=`blockdev --getsize64 $SCRATCH_DEV`
 	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
     fi
@@ -1764,11 +1758,6 @@ _require_logdev()
 #
 _require_loop()
 {
-    if [ "$HOSTOS" != "Linux" ]
-    then
-	_notrun "This test requires linux for loopback device support"
-    fi
-
     modprobe loop >/dev/null 2>&1
     if grep loop /proc/devices >/dev/null 2>&1
     then
@@ -1782,11 +1771,6 @@ _require_loop()
 #
 _require_ext2()
 {
-    if [ "$HOSTOS" != "Linux" ]
-    then
-	_notrun "This test requires linux for ext2 filesystem support"
-    fi
-
     modprobe ext2 >/dev/null 2>&1
     if grep ext2 /proc/filesystems >/dev/null 2>&1
     then
diff --git a/common/xfs b/common/xfs
index 8a3fd30c..0f4b8521 100644
--- a/common/xfs
+++ b/common/xfs
@@ -481,7 +481,7 @@ _check_xfs_filesystem()
 
 	$XFS_LOGPRINT_PROG -t $extra_log_options $device 2>&1 \
 		| tee $tmp.logprint | grep -q "<CLEAN>"
-	if [ $? -ne 0 -a "$HOSTOS" = "Linux" ]; then
+	if [ $? -ne 0 ]; then
 		_log_err "_check_xfs_filesystem: filesystem on $device has dirty log"
 		echo "*** xfs_logprint -t output ***"	>>$seqres.full
 		cat $tmp.logprint			>>$seqres.full

[PATCH 27/24] common/rc: fix indentation in _scratch_mkfs_sized

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Fix the weird indentation in _scratch_mkfs_sized.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/rc |  205 ++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 102 insertions(+), 103 deletions(-)

diff --git a/common/rc b/common/rc
index 3adeeefe..94936a8e 100644
--- a/common/rc
+++ b/common/rc
@@ -956,116 +956,115 @@ _available_memory_bytes()
 # _scratch_mkfs_sized <size in bytes> [optional blocksize]
 _scratch_mkfs_sized()
 {
-    local fssize=$1
-    local blocksize=$2
-    local def_blksz
+	local fssize=$1
+	local blocksize=$2
+	local def_blksz
 
-    case $FSTYP in
-    xfs)
-	def_blksz=`echo $MKFS_OPTIONS|sed -rn 's/.*-b ?size= ?+([0-9]+).*/\1/p'`
-	;;
-    ext2|ext3|ext4|ext4dev|udf|btrfs|reiser4|ocfs2|reiserfs)
-	def_blksz=`echo $MKFS_OPTIONS| sed -rn 's/.*-b ?+([0-9]+).*/\1/p'`
-	;;
-    jfs)
-	def_blksz=4096
-	;;
-    esac
+	case $FSTYP in
+	xfs)
+		def_blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*-b ?size= ?+([0-9]+).*/\1/p'`
+		;;
+	ext2|ext3|ext4|ext4dev|udf|btrfs|reiser4|ocfs2|reiserfs)
+		def_blksz=`echo $MKFS_OPTIONS | sed -rn 's/.*-b ?+([0-9]+).*/\1/p'`
+		;;
+	jfs)
+		def_blksz=4096
+		;;
+	esac
 
-    [ -n "$def_blksz" ] && blocksize=$def_blksz
-    [ -z "$blocksize" ] && blocksize=4096
+	[ -n "$def_blksz" ] && blocksize=$def_blksz
+	[ -z "$blocksize" ] && blocksize=4096
 
+	local re='^[0-9]+$'
+	if ! [[ $fssize =~ $re ]] ; then
+		_notrun "error: _scratch_mkfs_sized: fs size \"$fssize\" not an integer."
+	fi
+	if ! [[ $blocksize =~ $re ]] ; then
+		_notrun "error: _scratch_mkfs_sized: block size \"$blocksize\" not an integer."
+	fi
 
-    local re='^[0-9]+$'
-    if ! [[ $fssize =~ $re ]] ; then
-        _notrun "error: _scratch_mkfs_sized: fs size \"$fssize\" not an integer."
-    fi
-    if ! [[ $blocksize =~ $re ]] ; then
-        _notrun "error: _scratch_mkfs_sized: block size \"$blocksize\" not an integer."
-    fi
-
-    local blocks=`expr $fssize / $blocksize`
-
-    if [ -b "$SCRATCH_DEV" ]; then
-	local devsize=`blockdev --getsize64 $SCRATCH_DEV`
-	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
-    fi
-
-    if [ "$FSTYP" = "xfs" ] && [ -b "$SCRATCH_RTDEV" ]; then
-	local rtdevsize=`blockdev --getsize64 $SCRATCH_RTDEV`
-	[ "$fssize" -gt "$rtdevsize" ] && _notrun "Scratch rt device too small"
-	rt_ops="-r size=$fssize"
-    fi
+	local blocks=`expr $fssize / $blocksize`
 
-    case $FSTYP in
-    xfs)
-	# don't override MKFS_OPTIONS that set a block size.
-	echo $MKFS_OPTIONS |egrep -q "b?size="
-	if [ $? -eq 0 ]; then
-		_scratch_mkfs_xfs -d size=$fssize $rt_ops
-	else
-		_scratch_mkfs_xfs -d size=$fssize $rt_ops -b size=$blocksize
-	fi
-	;;
-    ext2|ext3|ext4|ext4dev)
-	${MKFS_PROG}.$FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
-	;;
-    gfs2)
-	# mkfs.gfs2 doesn't automatically shrink journal files on small
-	# filesystems, so the journal files may end up being bigger than the
-	# filesystem, which will cause mkfs.gfs2 to fail.  Until that's fixed,
-	# shrink the journal size to at most one eigth of the filesystem and at
-	# least 8 MiB, the minimum size allowed.
-	local min_journal_size=8
-	local default_journal_size=128
-	if (( fssize/8 / (1024*1024) < default_journal_size )); then
-	    local journal_size=$(( fssize/8 / (1024*1024) ))
-	    (( journal_size >= min_journal_size )) || journal_size=$min_journal_size
-	    MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS"
+	if [ -b "$SCRATCH_DEV" ]; then
+		local devsize=`blockdev --getsize64 $SCRATCH_DEV`
+		[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
 	fi
-	${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks
-	;;
-    ocfs2)
-	yes | ${MKFS_PROG}.$FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
-	;;
-    udf)
-	$MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
-	;;
-    btrfs)
-	local mixed_opt=
-	# minimum size that's needed without the mixed option.
-	# Ref: btrfs-prog: btrfs_min_dev_size()
-	# Non mixed mode is also the default option.
-	(( fssize < $((256 * 1024 *1024)) )) && mixed_opt='--mixed'
-	$MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV
-	;;
-    jfs)
-	${MKFS_PROG}.$FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks
-	;;
-    reiserfs)
-	${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
-	;;
-    reiser4)
-	# mkfs.resier4 requires size in KB as input for creating filesystem
-	$MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \
-			   `expr $fssize / 1024`
-	;;
-    f2fs)
-	# mkfs.f2fs requires # of sectors as an input for the size
-	local sector_size=`blockdev --getss $SCRATCH_DEV`
-	$MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size`
-	;;
-    tmpfs)
-	local free_mem=`_free_memory_bytes`
-	if [ "$free_mem" -lt "$fssize" ] ; then
-	   _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes"
+
+	if [ "$FSTYP" = "xfs" ] && [ -b "$SCRATCH_RTDEV" ]; then
+		local rtdevsize=`blockdev --getsize64 $SCRATCH_RTDEV`
+		[ "$fssize" -gt "$rtdevsize" ] && _notrun "Scratch rt device too small"
+		rt_ops="-r size=$fssize"
 	fi
-	export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS"
-	;;
-    *)
-	_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized"
-	;;
-    esac
+
+	case $FSTYP in
+	xfs)
+		# don't override MKFS_OPTIONS that set a block size.
+		echo $MKFS_OPTIONS |egrep -q "b?size="
+		if [ $? -eq 0 ]; then
+			_scratch_mkfs_xfs -d size=$fssize $rt_ops
+		else
+			_scratch_mkfs_xfs -d size=$fssize $rt_ops -b size=$blocksize
+		fi
+		;;
+	ext2|ext3|ext4|ext4dev)
+		${MKFS_PROG}.$FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		;;
+	gfs2)
+		# mkfs.gfs2 doesn't automatically shrink journal files on small
+		# filesystems, so the journal files may end up being bigger than the
+		# filesystem, which will cause mkfs.gfs2 to fail.  Until that's fixed,
+		# shrink the journal size to at most one eigth of the filesystem and at
+		# least 8 MiB, the minimum size allowed.
+		local min_journal_size=8
+		local default_journal_size=128
+		if (( fssize/8 / (1024*1024) < default_journal_size )); then
+			local journal_size=$(( fssize/8 / (1024*1024) ))
+			(( journal_size >= min_journal_size )) || journal_size=$min_journal_size
+			MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS"
+		fi
+		${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks
+		;;
+	ocfs2)
+		yes | ${MKFS_PROG}.$FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		;;
+	udf)
+		$MKFS_UDF_PROG $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		;;
+	btrfs)
+		local mixed_opt=
+		# minimum size that's needed without the mixed option.
+		# Ref: btrfs-prog: btrfs_min_dev_size()
+		# Non mixed mode is also the default option.
+		(( fssize < $((256 * 1024 *1024)) )) && mixed_opt='--mixed'
+		$MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV
+		;;
+	jfs)
+		${MKFS_PROG}.$FSTYP $MKFS_OPTIONS $SCRATCH_DEV $blocks
+		;;
+	reiserfs)
+		${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
+		;;
+	reiser4)
+		# mkfs.resier4 requires size in KB as input for creating filesystem
+		$MKFS_REISER4_PROG $MKFS_OPTIONS -y -b $blocksize $SCRATCH_DEV \
+				   `expr $fssize / 1024`
+		;;
+	f2fs)
+		# mkfs.f2fs requires # of sectors as an input for the size
+		local sector_size=`blockdev --getss $SCRATCH_DEV`
+		$MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size`
+		;;
+	tmpfs)
+		local free_mem=`_free_memory_bytes`
+		if [ "$free_mem" -lt "$fssize" ] ; then
+		   _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes"
+		fi
+		export MOUNT_OPTIONS="-o size=$fssize $TMPFS_MOUNT_OPTIONS"
+		;;
+	*)
+		_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized"
+		;;
+	esac
 }
 
 # Emulate an N-data-disk stripe w/ various stripe units

Re: [PATCH 27/24] common/rc: fix indentation in _scratch_mkfs_sized

[Christoph Hellwig]

On Thu, Sep 17, 2020 at 07:11:09PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Fix the weird indentation in _scratch_mkfs_sized.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 25/24] common: kill _supported_os

[Christoph Hellwig]

On Thu, Sep 17, 2020 at 07:09:36PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> fstests only supports Linux, so get rid of this unnecessary predicate.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>