[PATCH 1/2] xfs/521: use _try_scratch_mount instead of _scratch_mount

[PATCH 1/2] xfs/521: use _try_scratch_mount instead of _scratch_mount

[Yang Xu]

Here will check the return code of mount option. So update it
to use _try_scratch_mount.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 tests/xfs/521 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xfs/521 b/tests/xfs/521
index 3aaa1296..e6c417b8 100755
--- a/tests/xfs/521
+++ b/tests/xfs/521
@@ -49,7 +49,7 @@ echo "Format and mount 100m rt volume"
 export USE_EXTERNAL=yes
 export SCRATCH_RTDEV=$rtdev
 _scratch_mkfs -r size=100m > $seqres.full
-_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
+_try_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
 
 testdir=$SCRATCH_MNT/test-$seq
 mkdir $testdir
-- 
2.23.0

[PATCH 2/2] generic/402: use _try_scratch_mount instead of _scratch_mount

[Yang Xu]

Here will check the return code of mount option. So update it
to use _try_scratch_mount

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 tests/generic/402 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/402 b/tests/generic/402
index 7fc81864..4c940ac4 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -102,7 +102,7 @@ run_test()
 }
 
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
-_scratch_mount || _fail "scratch mount failed"
+_try_scratch_mount || _fail "scratch mount failed"
 
 _require_timestamp_range $SCRATCH_DEV
 
-- 
2.23.0

Re: [PATCH 1/2] xfs/521: use _try_scratch_mount instead of _scratch_mount

[Darrick J. Wong]

On Fri, Oct 16, 2020 at 05:31:23PM +0800, Yang Xu wrote:
> Here will check the return code of mount option. So update it
> to use _try_scratch_mount.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

Looks good to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  tests/xfs/521 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/521 b/tests/xfs/521
> index 3aaa1296..e6c417b8 100755
> --- a/tests/xfs/521
> +++ b/tests/xfs/521
> @@ -49,7 +49,7 @@ echo "Format and mount 100m rt volume"
>  export USE_EXTERNAL=yes
>  export SCRATCH_RTDEV=$rtdev
>  _scratch_mkfs -r size=100m > $seqres.full
> -_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
> +_try_scratch_mount || _notrun "Could not mount scratch with synthetic rt volume"
>  
>  testdir=$SCRATCH_MNT/test-$seq
>  mkdir $testdir
> -- 
> 2.23.0
> 
> 
> 

Re: [PATCH 2/2] generic/402: use _try_scratch_mount instead of _scratch_mount

[Darrick J. Wong]

On Fri, Oct 16, 2020 at 05:31:24PM +0800, Yang Xu wrote:
> Here will check the return code of mount option. So update it
> to use _try_scratch_mount
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>  tests/generic/402 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/402 b/tests/generic/402
> index 7fc81864..4c940ac4 100755
> --- a/tests/generic/402
> +++ b/tests/generic/402
> @@ -102,7 +102,7 @@ run_test()
>  }
>  
>  _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
> -_scratch_mount || _fail "scratch mount failed"
> +_try_scratch_mount || _fail "scratch mount failed"

Why not just drop the "|| _fail" part?  The message isn't much
more specific than the fail message that _scratch_mount emits.

--D

>  
>  _require_timestamp_range $SCRATCH_DEV
>  
> -- 
> 2.23.0
> 
> 
> 

Re: [PATCH 2/2] generic/402: use _try_scratch_mount instead of _scratch_mount

[Eryu Guan]

On Fri, Oct 16, 2020 at 08:33:21AM -0700, Darrick J. Wong wrote:
> On Fri, Oct 16, 2020 at 05:31:24PM +0800, Yang Xu wrote:
> > Here will check the return code of mount option. So update it
> > to use _try_scratch_mount
> > 
> > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> > ---
> >  tests/generic/402 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/generic/402 b/tests/generic/402
> > index 7fc81864..4c940ac4 100755
> > --- a/tests/generic/402
> > +++ b/tests/generic/402
> > @@ -102,7 +102,7 @@ run_test()
> >  }
> >  
> >  _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
> > -_scratch_mount || _fail "scratch mount failed"
> > +_try_scratch_mount || _fail "scratch mount failed"
> 
> Why not just drop the "|| _fail" part?  The message isn't much
> more specific than the fail message that _scratch_mount emits.

Agreed, using _scratch_mount is fine here.

Thanks,
Eryu

Re: [PATCH 2/2] generic/402: use _try_scratch_mount instead of _scratch_mount

[Yang Xu]

on 2020/10/18 18:10, Eryu Guan wrote:
> On Fri, Oct 16, 2020 at 08:33:21AM -0700, Darrick J. Wong wrote:
>> On Fri, Oct 16, 2020 at 05:31:24PM +0800, Yang Xu wrote:
>>> Here will check the return code of mount option. So update it
>>> to use _try_scratch_mount
>>>
>>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>>> ---
>>>   tests/generic/402 | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/generic/402 b/tests/generic/402
>>> index 7fc81864..4c940ac4 100755
>>> --- a/tests/generic/402
>>> +++ b/tests/generic/402
>>> @@ -102,7 +102,7 @@ run_test()
>>>   }
>>>
>>>   _scratch_mkfs&>>  $seqres.full 2>&1 || _fail "mkfs failed"
>>> -_scratch_mount || _fail "scratch mount failed"
>>> +_try_scratch_mount || _fail "scratch mount failed"
>>
>> Why not just drop the "|| _fail" part?  The message isn't much
>> more specific than the fail message that _scratch_mount emits.
>
> Agreed, using _scratch_mount is fine here.
Hi Darrick, Eryu
	Thanks for your review. Using _scratch_mount is better. I will send a 
v2 patch.

Best Regards
Yang Xu
	
>
> Thanks,
> Eryu
>
>
> .
>

[PATCH v2] generic/402: Drop useless fail message

[Yang Xu]

Here won't be "scratch mount failed" messages when mounting SCRATCH_DEV
failed, because _scratch_mount exits early by invoking the following_fail.

_scratch_mount
{
        _try_scratch_mount $* || _fail "mount failed"
}

The message is useless and redundant. So drop it.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 tests/generic/402 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/generic/402 b/tests/generic/402
index 7fc81864..ddaadf6d 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -102,7 +102,7 @@ run_test()
 }
 
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
-_scratch_mount || _fail "scratch mount failed"
+_scratch_mount
 
 _require_timestamp_range $SCRATCH_DEV
 
-- 
2.23.0

Re: [PATCH v2] generic/402: Drop useless fail message

[Darrick J. Wong]

On Mon, Oct 19, 2020 at 11:01:59AM +0800, Yang Xu wrote:
> Here won't be "scratch mount failed" messages when mounting SCRATCH_DEV
> failed, because _scratch_mount exits early by invoking the following_fail.
> 
> _scratch_mount
> {
>         _try_scratch_mount $* || _fail "mount failed"
> }
> 
> The message is useless and redundant. So drop it.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---

Looks good to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  tests/generic/402 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/402 b/tests/generic/402
> index 7fc81864..ddaadf6d 100755
> --- a/tests/generic/402
> +++ b/tests/generic/402
> @@ -102,7 +102,7 @@ run_test()
>  }
>  
>  _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
> -_scratch_mount || _fail "scratch mount failed"
> +_scratch_mount
>  
>  _require_timestamp_range $SCRATCH_DEV
>  
> -- 
> 2.23.0
> 
> 
>