[PATCH] common/rc: check error case and fail the test

[PATCH] common/rc: check error case and fail the test

[Naohiro Aota]

If we place /var/lib/xfstests on a read-only filesystem, commands in
_link_out_file_named() fail to modify the files. However, they won't fail
the test. As a result, the test case fails mysteriously with only "no
qualified output" printed.

Fix it by checking the error case.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 common/rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 76a7e7740318..4e9bd34f7772 100644
--- a/common/rc
+++ b/common/rc
@@ -3431,8 +3431,8 @@ _link_out_file_named()
 		}
 		print $result
 		' <$seqfull.cfg)
-	rm -f $1
-	ln -fs $(basename $1).$suffix $1
+	rm -f $1 || _fail "_link_out_file_named: failed to remove existing output file"
+	ln -fs $(basename $1).$suffix $1 || _fail "_link_out_file_named: failed to setup output file"
 }
 
 _link_out_file()
-- 
2.42.0

Re: [PATCH] common/rc: check error case and fail the test

[Darrick J. Wong]

On Wed, Sep 27, 2023 at 03:11:00PM +0900, Naohiro Aota wrote:
> If we place /var/lib/xfstests on a read-only filesystem, commands in
> _link_out_file_named() fail to modify the files. However, they won't fail
> the test. As a result, the test case fails mysteriously with only "no
> qualified output" printed.
> 
> Fix it by checking the error case.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  common/rc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 76a7e7740318..4e9bd34f7772 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3431,8 +3431,8 @@ _link_out_file_named()
>  		}
>  		print $result
>  		' <$seqfull.cfg)
> -	rm -f $1
> -	ln -fs $(basename $1).$suffix $1
> +	rm -f $1 || _fail "_link_out_file_named: failed to remove existing output file"
> +	ln -fs $(basename $1).$suffix $1 || _fail "_link_out_file_named: failed to setup output file"

For the second _fail, you could improve the diagnostic output by
specifying /which/ output file couldn't be linked, e.g.

	ln -fs $(basename $1).$suffix $1 || \
		_fail "$(basename $1).$suffix: could not setup output file"

Since there's always the possibility of Weird Things happening, like
project quota ENOSPC/EXDEV failures, ro mounts, and other strange things
that might be a consequence of the file the test picked.

With that changed,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  }
>  
>  _link_out_file()
> -- 
> 2.42.0
>