diff mbox series

[blktests] nvme/rc: fix rdma driver check

Message ID 20231005080242.292291-1-shinichiro.kawasaki@wdc.com (mailing list archive)
State New, archived
Headers show
Series [blktests] nvme/rc: fix rdma driver check | expand

Commit Message

Shin'ichiro Kawasaki Oct. 5, 2023, 8:02 a.m. UTC
Since the commit 4824ac3f5c4a ("Skip tests based on SKIP_REASON, not
return value"), blktests no longer checks return values of _have_foo
helpers. Instead, it checks if _have_foo helpers set SKIP_REASON, which
was renamed to SKIP_REASONS later, to judge test case skip. If two
_have_foo helpers are chained with ||, the skip check does not work as
expected since one of the helper may set SKIP_REASONS even when the
other does not set. Such chain with || is done in _nvme_requires() to
check rdma drivers.

To fix the check, do not chain the helper functions with || operator.
Instead, refer $use_rxe to call only the required function.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/nvme/rc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Bart Van Assche Oct. 5, 2023, 6:02 p.m. UTC | #1
On 10/5/23 01:02, Shin'ichiro Kawasaki wrote:
> Since the commit 4824ac3f5c4a ("Skip tests based on SKIP_REASON, not
> return value"), blktests no longer checks return values of _have_foo
> helpers. Instead, it checks if _have_foo helpers set SKIP_REASON, which
> was renamed to SKIP_REASONS later, to judge test case skip. If two
> _have_foo helpers are chained with ||, the skip check does not work as
> expected since one of the helper may set SKIP_REASONS even when the
> other does not set. Such chain with || is done in _nvme_requires() to
> check rdma drivers.
> 
> To fix the check, do not chain the helper functions with || operator.
> Instead, refer $use_rxe to call only the required function.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>   tests/nvme/rc | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 1ec9eb6..bac2db7 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -44,7 +44,11 @@ _nvme_requires() {
>   		_have_driver nvmet-rdma
>   		_have_configfs
>   		_have_program rdma
> -		_have_driver rdma_rxe || _have_driver siw
> +		if [ -n "$use_rxe" ]; then
> +			_have_driver rdma_rxe
> +		else
> +			_have_driver siw
> +		fi
>   		;;
>   	fc)
>   		_have_driver nvme-fc

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Shin'ichiro Kawasaki Oct. 12, 2023, 5:11 a.m. UTC | #2
On Oct 05, 2023 / 11:02, Bart Van Assche wrote:
> 
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Thanks for the review. I've applied the patch.
diff mbox series

Patch

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 1ec9eb6..bac2db7 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -44,7 +44,11 @@  _nvme_requires() {
 		_have_driver nvmet-rdma
 		_have_configfs
 		_have_program rdma
-		_have_driver rdma_rxe || _have_driver siw
+		if [ -n "$use_rxe" ]; then
+			_have_driver rdma_rxe
+		else
+			_have_driver siw
+		fi
 		;;
 	fc)
 		_have_driver nvme-fc