[PATCH] xfs/132: umount scratch device after finishing test

[PATCH] xfs/132: umount scratch device after finishing test

[Xiao Yang]

xfs/132 and xfs/133 running together got the following error:
------------------------------------------------------------
...
xfs/132 1s ... 1s
xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
...
QA output created by 133
-Format and mount
-Corrupt filesystem
-Remount, try to append
-Write did not succeed (ok).
+SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
+Already mounted result:
+/dev/sda11 /mnt/xfstests/scratch
...
------------------------------------------------------------

xfs/132 led to XFS shutdown due to the corrupted inode, but it didn't rectify XFS
by umount scratch device.  Subsequent xfs/133 got a wrong canonical mount point
path(i.e. SCRATCH_MNT='common/config: SCRATCH_MNT (/mnt/xfstests/scratch) is not a
directory') for $SCRATCH_DEV when calling _canonicalize_mountpoint in common/config,
and finally led to the failure of _check_mounted_on.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 tests/xfs/132 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/xfs/132 b/tests/xfs/132
index 6bb03e7..960096f 100755
--- a/tests/xfs/132
+++ b/tests/xfs/132
@@ -68,6 +68,8 @@ _scratch_mount
 # The corrupt inode should be tripped over during these initial file creates.
 touch $SCRATCH_MNT/file{0,1,2,3,4,5}{0,1,2,3,4,5} 2>&1 | _filter_scratch
 
+_scratch_unmount
+
 # success, all done
 status=0
 exit
-- 
1.8.3.1

Re: [PATCH] xfs/132: umount scratch device after finishing test

[Dave Chinner]

On Sat, May 19, 2018 at 12:32:24PM +0800, Xiao Yang wrote:
> xfs/132 and xfs/133 running together got the following error:
> ------------------------------------------------------------
> ...
> xfs/132 1s ... 1s
> xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
> ...
> QA output created by 133
> -Format and mount
> -Corrupt filesystem
> -Remount, try to append
> -Write did not succeed (ok).
> +SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
> +Already mounted result:
> +/dev/sda11 /mnt/xfstests/scratch
> ...
> ------------------------------------------------------------
> 
> xfs/132 led to XFS shutdown due to the corrupted inode, but it didn't rectify XFS
> by umount scratch device.

AFAIK, we don't have to unmount the scratch device when a test
finishes - the test harness is supposed to do that and make sure
that it is in the correct state for the next test to run.

It seems that somewhere along the line this got broken. - I'm
guessing the fact that this test also says "don't check the scratch
device" the test harness is failing to unmount it because it's not
running _check_scratch_device. I'm guessing that it should at least
run _scratch_unmount....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs/132: umount scratch device after finishing test

[Xiao Yang]

On 2018/05/21 10:36, Dave Chinner wrote:
> On Sat, May 19, 2018 at 12:32:24PM +0800, Xiao Yang wrote:
>> xfs/132 and xfs/133 running together got the following error:
>> ------------------------------------------------------------
>> ...
>> xfs/132 1s ... 1s
>> xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
>> ...
>> QA output created by 133
>> -Format and mount
>> -Corrupt filesystem
>> -Remount, try to append
>> -Write did not succeed (ok).
>> +SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
>> +Already mounted result:
>> +/dev/sda11 /mnt/xfstests/scratch
>> ...
>> ------------------------------------------------------------
>>
>> xfs/132 led to XFS shutdown due to the corrupted inode, but it didn't rectify XFS
>> by umount scratch device.
> AFAIK, we don't have to unmount the scratch device when a test
> finishes - the test harness is supposed to do that and make sure
> that it is in the correct state for the next test to run.
>
> It seems that somewhere along the line this got broken. - I'm
> guessing the fact that this test also says "don't check the scratch
> device" the test harness is failing to unmount it because it's not
> running _check_scratch_device. I'm guessing that it should at least
> run _scratch_unmount....
Hi Dave,

Could we just make the test harness call _scratch_unmount after running every test, as below:
--------------------------------------------------------------
diff --git a/check b/check
index 96198ac..63ece67 100755
--- a/check
+++ b/check
@@ -815,6 +815,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
                 _make_testcase_report "$tc_status"
             fi
             seq="after_$seqnum"
+           _scratch_unmount 2>  /dev/null
         done
         sect_stop=`_wallclock`
         interrupt=false
@@ -823,7 +824,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
         echo

         _test_unmount 2>  /dev/null
-       _scratch_unmount 2>  /dev/null
  done

  interrupt=false
---------------------------------------------------------------

Thanks,
Xiao Yang

> Cheers,
>
> Dave.

Re: [PATCH] xfs/132: umount scratch device after finishing test

[Dave Chinner]

On Mon, May 21, 2018 at 03:45:52PM +0800, Xiao Yang wrote:
> On 2018/05/21 10:36, Dave Chinner wrote:
> >On Sat, May 19, 2018 at 12:32:24PM +0800, Xiao Yang wrote:
> >>xfs/132 and xfs/133 running together got the following error:
> >>------------------------------------------------------------
> >>...
> >>xfs/132 1s ... 1s
> >>xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
> >>...
> >>QA output created by 133
> >>-Format and mount
> >>-Corrupt filesystem
> >>-Remount, try to append
> >>-Write did not succeed (ok).
> >>+SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
> >>+Already mounted result:
> >>+/dev/sda11 /mnt/xfstests/scratch
> >>...
> >>------------------------------------------------------------
> >>
> >>xfs/132 led to XFS shutdown due to the corrupted inode, but it didn't rectify XFS
> >>by umount scratch device.
> >AFAIK, we don't have to unmount the scratch device when a test
> >finishes - the test harness is supposed to do that and make sure
> >that it is in the correct state for the next test to run.
> >
> >It seems that somewhere along the line this got broken. - I'm
> >guessing the fact that this test also says "don't check the scratch
> >device" the test harness is failing to unmount it because it's not
> >running _check_scratch_device. I'm guessing that it should at least
> >run _scratch_unmount....
> Hi Dave,
> 
> Could we just make the test harness call _scratch_unmount after running every test, as below:
> --------------------------------------------------------------
> diff --git a/check b/check
> index 96198ac..63ece67 100755
> --- a/check
> +++ b/check
> @@ -815,6 +815,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
>                 _make_testcase_report "$tc_status"
>             fi
>             seq="after_$seqnum"
> +           _scratch_unmount 2>  /dev/null
>         done
>         sect_stop=`_wallclock`
>         interrupt=false

Sort of. The problem is in _check_filesystem, though, where it does
nothing when _require_scratch_nocheck() is used. It needs to cycle
the mount in this case, and then the main loop doesn't need to be
touched...

> @@ -823,7 +824,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
>         echo
> 
>         _test_unmount 2>  /dev/null
> -       _scratch_unmount 2>  /dev/null
>  done

This needs to remain - this is the final unmount of the devices
after all tests run.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs/132: umount scratch device after finishing test

[Xiao Yang]

On 2018/05/22 6:46, Dave Chinner wrote:
> On Mon, May 21, 2018 at 03:45:52PM +0800, Xiao Yang wrote:
>> On 2018/05/21 10:36, Dave Chinner wrote:
>>> On Sat, May 19, 2018 at 12:32:24PM +0800, Xiao Yang wrote:
>>>> xfs/132 and xfs/133 running together got the following error:
>>>> ------------------------------------------------------------
>>>> ...
>>>> xfs/132 1s ... 1s
>>>> xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
>>>> ...
>>>> QA output created by 133
>>>> -Format and mount
>>>> -Corrupt filesystem
>>>> -Remount, try to append
>>>> -Write did not succeed (ok).
>>>> +SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
>>>> +Already mounted result:
>>>> +/dev/sda11 /mnt/xfstests/scratch
>>>> ...
>>>> ------------------------------------------------------------
>>>>
>>>> xfs/132 led to XFS shutdown due to the corrupted inode, but it didn't rectify XFS
>>>> by umount scratch device.
>>> AFAIK, we don't have to unmount the scratch device when a test
>>> finishes - the test harness is supposed to do that and make sure
>>> that it is in the correct state for the next test to run.
>>>
>>> It seems that somewhere along the line this got broken. - I'm
>>> guessing the fact that this test also says "don't check the scratch
>>> device" the test harness is failing to unmount it because it's not
>>> running _check_scratch_device. I'm guessing that it should at least
>>> run _scratch_unmount....
>> Hi Dave,
>>
>> Could we just make the test harness call _scratch_unmount after running every test, as below:
>> --------------------------------------------------------------
>> diff --git a/check b/check
>> index 96198ac..63ece67 100755
>> --- a/check
>> +++ b/check
>> @@ -815,6 +815,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
>>                  _make_testcase_report "$tc_status"
>>              fi
>>              seq="after_$seqnum"
>> +           _scratch_unmount 2>   /dev/null
>>          done
>>          sect_stop=`_wallclock`
>>          interrupt=false
> Sort of. The problem is in _check_filesystem, though, where it does
> nothing when _require_scratch_nocheck() is used. It needs to cycle
> the mount in this case, and then the main loop doesn't need to be
> touched...
Hi Dave,

What about cycling the mount when _require_scratch_nocheck() or _notrun is called?
--------------------------------------------------------------------
diff --git a/check b/check
index 96198ac..ea94524 100755
--- a/check
+++ b/check
@@ -489,10 +489,14 @@ _check_filesystems()
         if [ -f ${RESULT_DIR}/require_test ]; then
                 _check_test_fs || err=true
                 rm -f ${RESULT_DIR}/require_test*
+       else
+               _test_cycle_mount
         fi
         if [ -f ${RESULT_DIR}/require_scratch ]; then
                 _check_scratch_fs || err=true
                 rm -f ${RESULT_DIR}/require_scratch*
+       else
+               _scratch_cycle_mount
         fi
  }
--------------------------------------------------------------------

>> @@ -823,7 +824,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
>>          echo
>>
>>          _test_unmount 2>   /dev/null
>> -       _scratch_unmount 2>   /dev/null
>>   done
> This needs to remain - this is the final unmount of the devices
> after all tests run.
Agreed.  remain it.

Thanks,
Xiao Yang

> Cheers,
>
> Dave.

Re: [PATCH] xfs/132: umount scratch device after finishing test

[Dave Chinner]

On Tue, May 22, 2018 at 12:06:30PM +0800, Xiao Yang wrote:
> On 2018/05/22 6:46, Dave Chinner wrote:
> >On Mon, May 21, 2018 at 03:45:52PM +0800, Xiao Yang wrote:
> >>On 2018/05/21 10:36, Dave Chinner wrote:
> >>>On Sat, May 19, 2018 at 12:32:24PM +0800, Xiao Yang wrote:
> >>>>xfs/132 and xfs/133 running together got the following error:
> >>>>------------------------------------------------------------
> >>>>...
> >>>>xfs/132 1s ... 1s
> >>>>xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
> >>>>...
> >>>>QA output created by 133
> >>>>-Format and mount
> >>>>-Corrupt filesystem
> >>>>-Remount, try to append
> >>>>-Write did not succeed (ok).
> >>>>+SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
> >>>>+Already mounted result:
> >>>>+/dev/sda11 /mnt/xfstests/scratch
> >>>>...
> >>>>------------------------------------------------------------
> >>>>
> >>>>xfs/132 led to XFS shutdown due to the corrupted inode, but it didn't rectify XFS
> >>>>by umount scratch device.
> >>>AFAIK, we don't have to unmount the scratch device when a test
> >>>finishes - the test harness is supposed to do that and make sure
> >>>that it is in the correct state for the next test to run.
> >>>
> >>>It seems that somewhere along the line this got broken. - I'm
> >>>guessing the fact that this test also says "don't check the scratch
> >>>device" the test harness is failing to unmount it because it's not
> >>>running _check_scratch_device. I'm guessing that it should at least
> >>>run _scratch_unmount....
> >>Hi Dave,
> >>
> >>Could we just make the test harness call _scratch_unmount after running every test, as below:
> >>--------------------------------------------------------------
> >>diff --git a/check b/check
> >>index 96198ac..63ece67 100755
> >>--- a/check
> >>+++ b/check
> >>@@ -815,6 +815,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
> >>                 _make_testcase_report "$tc_status"
> >>             fi
> >>             seq="after_$seqnum"
> >>+           _scratch_unmount 2>   /dev/null
> >>         done
> >>         sect_stop=`_wallclock`
> >>         interrupt=false
> >Sort of. The problem is in _check_filesystem, though, where it does
> >nothing when _require_scratch_nocheck() is used. It needs to cycle
> >the mount in this case, and then the main loop doesn't need to be
> >touched...
> Hi Dave,
> 
> What about cycling the mount when _require_scratch_nocheck() or _notrun is called?
> --------------------------------------------------------------------
> diff --git a/check b/check
> index 96198ac..ea94524 100755
> --- a/check
> +++ b/check
> @@ -489,10 +489,14 @@ _check_filesystems()
>         if [ -f ${RESULT_DIR}/require_test ]; then
>                 _check_test_fs || err=true
>                 rm -f ${RESULT_DIR}/require_test*
> +       else
> +               _test_cycle_mount
>         fi
>         if [ -f ${RESULT_DIR}/require_scratch ]; then
>                 _check_scratch_fs || err=true
>                 rm -f ${RESULT_DIR}/require_scratch*
> +       else
> +               _scratch_cycle_mount
>         fi
>  }
> --------------------------------------------------------------------

I would just unmount them. _require_test will mount the test device
if it's unmounted, and _require_scratch unmounts it if it is
mounted. Hence there is no real need to mount it again here,
especially as trying to mount a corrupt scratch device can throw
errors...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH v2] check: Make the test harness unmount fs when finishing a test

[Xiao Yang]

Finishing xfs/132 left a shutdown scratch fs and the test harness didn't
unmount the fs(because we told it not to check the fs) so the test harness
called by subsequent xfs/133 tried to "test -d $SCRATCH_MNT" and received
the IO error from the dead fs.

i.e. Running xfs/132 and xfs/133 together got the following error:
------------------------------------------------------------
...
xfs/132 1s ... 1s
xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
...
QA output created by 133
-Format and mount
-Corrupt filesystem
-Remount, try to append
-Write did not succeed (ok).
+SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
+Already mounted result:
+/dev/sda11 /mnt/xfstests/scratch
...
------------------------------------------------------------

Even if we don't check fs, the test harness is supposed to unmount fs and
return an initial state before running the next test.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 check | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/check b/check
index 96198ac..a0d5364 100755
--- a/check
+++ b/check
@@ -489,10 +489,14 @@ _check_filesystems()
 	if [ -f ${RESULT_DIR}/require_test ]; then
 		_check_test_fs || err=true
 		rm -f ${RESULT_DIR}/require_test*
+	else
+		_test_unmount
 	fi
 	if [ -f ${RESULT_DIR}/require_scratch ]; then
 		_check_scratch_fs || err=true
 		rm -f ${RESULT_DIR}/require_scratch*
+	else
+		_scratch_unmount
 	fi
 }
 
-- 
1.8.3.1

[PATCH v3] check: Make the test harness unmount fs when finishing a test

[Xiao Yang]

Finishing xfs/132 left a shutdown scratch fs and the test harness didn't
unmount the fs(because we told it not to check the fs) so the test harness
called by subsequent xfs/133 tried to "test -d $SCRATCH_MNT" and received
the IO error from the dead fs.

i.e. Running xfs/132 and xfs/133 together got the following error:
------------------------------------------------------------
...
xfs/132 1s ... 1s
xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
...
QA output created by 133
-Format and mount
-Corrupt filesystem
-Remount, try to append
-Write did not succeed (ok).
+SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
+Already mounted result:
+/dev/sda11 /mnt/xfstests/scratch
...
------------------------------------------------------------

Even if we don't check fs, the test harness is supposed to unmount fs and
return an initial state before running the next test.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 check | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/check b/check
index 96198ac..f6fb352 100755
--- a/check
+++ b/check
@@ -489,10 +489,14 @@ _check_filesystems()
 	if [ -f ${RESULT_DIR}/require_test ]; then
 		_check_test_fs || err=true
 		rm -f ${RESULT_DIR}/require_test*
+	else
+		_test_unmount 2> /dev/null
 	fi
 	if [ -f ${RESULT_DIR}/require_scratch ]; then
 		_check_scratch_fs || err=true
 		rm -f ${RESULT_DIR}/require_scratch*
+	else
+		_scratch_unmount 2> /dev/null
 	fi
 }
 
-- 
1.8.3.1

Re: [PATCH v2] check: Make the test harness unmount fs when finishing a test

[Xiao Yang]

On 2018/05/22 14:03, Xiao Yang wrote:
> Finishing xfs/132 left a shutdown scratch fs and the test harness didn't
> unmount the fs(because we told it not to check the fs) so the test harness
> called by subsequent xfs/133 tried to "test -d $SCRATCH_MNT" and received
> the IO error from the dead fs.
>
> i.e. Running xfs/132 and xfs/133 together got the following error:
> ------------------------------------------------------------
> ...
> xfs/132 1s ... 1s
> xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
> ...
> QA output created by 133
> -Format and mount
> -Corrupt filesystem
> -Remount, try to append
> -Write did not succeed (ok).
> +SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
> +Already mounted result:
> +/dev/sda11 /mnt/xfstests/scratch
> ...
> ------------------------------------------------------------
>
> Even if we don't check fs, the test harness is supposed to unmount fs and
> return an initial state before running the next test.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  check | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/check b/check
> index 96198ac..a0d5364 100755
> --- a/check
> +++ b/check
> @@ -489,10 +489,14 @@ _check_filesystems()
>  	if [ -f ${RESULT_DIR}/require_test ]; then
>  		_check_test_fs || err=true
>  		rm -f ${RESULT_DIR}/require_test*
> +	else
> +		_test_unmount
>  	fi
>  	if [ -f ${RESULT_DIR}/require_scratch ]; then
>  		_check_scratch_fs || err=true
>  		rm -f ${RESULT_DIR}/require_scratch*
> +	else
> +		_scratch_unmount
Hi,

Sorry, some tests have unmounted them inside, so drop the stderr from
umount.
The v3 patch is coming. :-(

Thanks,
Xiao Yang
>  	fi
>  }
>  

Re: [PATCH v3] check: Make the test harness unmount fs when finishing a test

[Dave Chinner]

On Tue, May 22, 2018 at 05:49:59PM +0800, Xiao Yang wrote:
> Finishing xfs/132 left a shutdown scratch fs and the test harness didn't
> unmount the fs(because we told it not to check the fs) so the test harness
> called by subsequent xfs/133 tried to "test -d $SCRATCH_MNT" and received
> the IO error from the dead fs.
> 
> i.e. Running xfs/132 and xfs/133 together got the following error:
> ------------------------------------------------------------
> ...
> xfs/132 1s ... 1s
> xfs/133 1s ... [failed, exit status 1] - output mismatch (see /var/lib/xfstests/results//xfs/133.out.bad)
> ...
> QA output created by 133
> -Format and mount
> -Corrupt filesystem
> -Remount, try to append
> -Write did not succeed (ok).
> +SCRATCH_DEV=/dev/sda11 is mounted but not on SCRATCH_MNT=common/config: - aborting
> +Already mounted result:
> +/dev/sda11 /mnt/xfstests/scratch
> ...
> ------------------------------------------------------------
> 
> Even if we don't check fs, the test harness is supposed to unmount fs and
> return an initial state before running the next test.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  check | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/check b/check
> index 96198ac..f6fb352 100755
> --- a/check
> +++ b/check
> @@ -489,10 +489,14 @@ _check_filesystems()
>  	if [ -f ${RESULT_DIR}/require_test ]; then
>  		_check_test_fs || err=true
>  		rm -f ${RESULT_DIR}/require_test*
> +	else
> +		_test_unmount 2> /dev/null
>  	fi
>  	if [ -f ${RESULT_DIR}/require_scratch ]; then
>  		_check_scratch_fs || err=true
>  		rm -f ${RESULT_DIR}/require_scratch*
> +	else
> +		_scratch_unmount 2> /dev/null
>  	fi
>  }

Looks good! Thanks for fixing this up, Xiao.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com