diff mbox series

[blktests,v2,05/11] nvme/rc: introduce NVMET_TRTYPES

Message ID 20240416103207.2754778-6-shinichiro.kawasaki@wdc.com (mailing list archive)
State New
Headers show
Series support test case repeat by different conditions | expand

Commit Message

Shin'ichiro Kawasaki April 16, 2024, 10:32 a.m. UTC
Some of the test cases in nvme test group can be run under various nvme
target transport types. The configuration parameter nvme_trtype
specifies the transport to use. But this configuration method has two
drawbacks. Firstly, the blktests check script needs to be invoked
multiple times to cover multiple transport types. Secondly, the test
cases irrelevant to the transport types are executed exactly same
conditions in the multiple blktests runs.

To avoid the drawbacks, introduce the new configuration parameter
NVMET_TRTYPES. This parameter can take multiple transport types like:

    NVMET_TRTYPES="loop tcp"

Also introduce _nvmet_set_nvme_trtype() which can be called from the
set_conditions() hook of the transport type dependent test cases.
Blktests will repeat the test case as many as the number of elements in
NVMET_TRTYPES, and set nvme_trtype for each test case run.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 Documentation/running-tests.md | 11 ++++++++---
 tests/nvme/rc                  | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 4 deletions(-)

Comments

Sagi Grimberg April 18, 2024, 9:39 a.m. UTC | #1
On 16/04/2024 13:32, Shin'ichiro Kawasaki wrote:
> Some of the test cases in nvme test group can be run under various nvme
> target transport types. The configuration parameter nvme_trtype
> specifies the transport to use. But this configuration method has two
> drawbacks. Firstly, the blktests check script needs to be invoked
> multiple times to cover multiple transport types. Secondly, the test
> cases irrelevant to the transport types are executed exactly same
> conditions in the multiple blktests runs.
>
> To avoid the drawbacks, introduce the new configuration parameter
> NVMET_TRTYPES. This parameter can take multiple transport types like:
>
>      NVMET_TRTYPES="loop tcp"
>
> Also introduce _nvmet_set_nvme_trtype() which can be called from the
> set_conditions() hook of the transport type dependent test cases.
> Blktests will repeat the test case as many as the number of elements in
> NVMET_TRTYPES, and set nvme_trtype for each test case run.

It would be nicer to keep the same name and just have it accept an array.
Not a must though.

> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>   Documentation/running-tests.md | 11 ++++++++---
>   tests/nvme/rc                  | 33 ++++++++++++++++++++++++++++++++-
>   2 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md
> index ae80860..144acd1 100644
> --- a/Documentation/running-tests.md
> +++ b/Documentation/running-tests.md
> @@ -102,8 +102,13 @@ RUN_ZONED_TESTS=1
>   
>   The NVMe tests can be additionally parameterized via environment variables.
>   
> +- NVMET_TRTYPES: 'loop' (default), 'tcp', 'rdma' and 'fc'
> +  Set up NVME target backends with the specified transport. Multiple transports
> +  can be listed with separating spaces, e.g., "loop tcp rdma". In this case, the
> +  tests are repeated to cover all of the transports specified.
>   - nvme_trtype: 'loop' (default), 'tcp', 'rdma' and 'fc'
> -  Run the tests with the given transport.
> +  Run the tests with the given transport. This parameter is still usable but
> +  replaced with NVMET_TRTYPES. Use NVMET_TRTYPES instead.
>   - nvme_img_size: '1G' (default)
>     Run the tests with given image size in bytes. 'm', 'M', 'g'
>   	and 'G' postfix are supported.
> @@ -117,11 +122,11 @@ These tests will use the siw (soft-iWARP) driver by default. The rdma_rxe
>   
>   ```sh
>   To use the siw driver:
> -nvme_trtype=rdma ./check nvme/
> +NVMET_TRTYPES=rdma ./check nvme/
>   ./check srp/
>   
>   To use the rdma_rxe driver:
> -use_rxe=1 nvme_trtype=rdma ./check nvme/
> +use_rxe=1 NVMET_TRTYPES=rdma ./check nvme/
>   use_rxe=1 ./check srp/
>   ```
>   
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 1f5ff44..34ecdde 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -18,10 +18,41 @@ def_hostid="0f01fb42-9f7f-4856-b0b3-51e60b8de349"
>   def_hostnqn="nqn.2014-08.org.nvmexpress:uuid:${def_hostid}"
>   export def_subsysnqn="blktests-subsystem-1"
>   export def_subsys_uuid="91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> -nvme_trtype=${nvme_trtype:-"loop"}
>   nvme_img_size=${nvme_img_size:-"1G"}
>   nvme_num_iter=${nvme_num_iter:-"1000"}
>   
> +# Check consistency of NVMET_TRTYPES and nvme_trtype configurations.
> +# If neither is configured, set the default value.
> +first_call=${first_call:-1}
> +if ((first_call)); then
> +	if [[ -n $nvme_trtype ]]; then
> +		if [[ -n $NVMET_TRTYPES ]]; then
> +			echo "Both nvme_trtype and NVMET_TRTYPES are specified"
> +			exit 1
> +		fi
> +		NVMET_TRTYPES="$nvme_trtype"
> +	elif [[ -z $NVMET_TRTYPES ]]; then
> +		nvme_trtype="loop"
> +		NVMET_TRTYPES="$nvme_trtype"
> +	fi
> +	first_call=0
> +fi

It would be nice to have the string check be done at first so you don't
get any typo-related error later during the execution.
Shin'ichiro Kawasaki April 22, 2024, 11:35 a.m. UTC | #2
Hi, Sagi, thanks for the comments.

On Apr 18, 2024 / 12:39, Sagi Grimberg wrote:
> 
> 
> On 16/04/2024 13:32, Shin'ichiro Kawasaki wrote:
> > Some of the test cases in nvme test group can be run under various nvme
> > target transport types. The configuration parameter nvme_trtype
> > specifies the transport to use. But this configuration method has two
> > drawbacks. Firstly, the blktests check script needs to be invoked
> > multiple times to cover multiple transport types. Secondly, the test
> > cases irrelevant to the transport types are executed exactly same
> > conditions in the multiple blktests runs.
> > 
> > To avoid the drawbacks, introduce the new configuration parameter
> > NVMET_TRTYPES. This parameter can take multiple transport types like:
> > 
> >      NVMET_TRTYPES="loop tcp"
> > 
> > Also introduce _nvmet_set_nvme_trtype() which can be called from the
> > set_conditions() hook of the transport type dependent test cases.
> > Blktests will repeat the test case as many as the number of elements in
> > NVMET_TRTYPES, and set nvme_trtype for each test case run.
> 
> It would be nicer to keep the same name and just have it accept an array.
> Not a must though.

Okay, actually this patch makes nvme_trtype to work same as NVMET_TRTYPES.
I will update the commit message and the description in running-tests.md
to clarify that nvme_trtype accepts multiple transports.

Though the existing nvme_trtype and the new NVMET_TRTYPES work same, I think
this is a good chance to take a step to replace the lowercase parameters to
uppercase ones. Will keep those two and describe that NVMET_TRTYPES is
recommended.

> 
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> > ---
> >   Documentation/running-tests.md | 11 ++++++++---
> >   tests/nvme/rc                  | 33 ++++++++++++++++++++++++++++++++-
> >   2 files changed, 40 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md
> > index ae80860..144acd1 100644
> > --- a/Documentation/running-tests.md
> > +++ b/Documentation/running-tests.md
> > @@ -102,8 +102,13 @@ RUN_ZONED_TESTS=1
> >   The NVMe tests can be additionally parameterized via environment variables.
> > +- NVMET_TRTYPES: 'loop' (default), 'tcp', 'rdma' and 'fc'
> > +  Set up NVME target backends with the specified transport. Multiple transports
> > +  can be listed with separating spaces, e.g., "loop tcp rdma". In this case, the
> > +  tests are repeated to cover all of the transports specified.
> >   - nvme_trtype: 'loop' (default), 'tcp', 'rdma' and 'fc'
> > -  Run the tests with the given transport.
> > +  Run the tests with the given transport. This parameter is still usable but
> > +  replaced with NVMET_TRTYPES. Use NVMET_TRTYPES instead.
> >   - nvme_img_size: '1G' (default)
> >     Run the tests with given image size in bytes. 'm', 'M', 'g'
> >   	and 'G' postfix are supported.
> > @@ -117,11 +122,11 @@ These tests will use the siw (soft-iWARP) driver by default. The rdma_rxe
> >   ```sh
> >   To use the siw driver:
> > -nvme_trtype=rdma ./check nvme/
> > +NVMET_TRTYPES=rdma ./check nvme/
> >   ./check srp/
> >   To use the rdma_rxe driver:
> > -use_rxe=1 nvme_trtype=rdma ./check nvme/
> > +use_rxe=1 NVMET_TRTYPES=rdma ./check nvme/
> >   use_rxe=1 ./check srp/
> >   ```
> > diff --git a/tests/nvme/rc b/tests/nvme/rc
> > index 1f5ff44..34ecdde 100644
> > --- a/tests/nvme/rc
> > +++ b/tests/nvme/rc
> > @@ -18,10 +18,41 @@ def_hostid="0f01fb42-9f7f-4856-b0b3-51e60b8de349"
> >   def_hostnqn="nqn.2014-08.org.nvmexpress:uuid:${def_hostid}"
> >   export def_subsysnqn="blktests-subsystem-1"
> >   export def_subsys_uuid="91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> > -nvme_trtype=${nvme_trtype:-"loop"}
> >   nvme_img_size=${nvme_img_size:-"1G"}
> >   nvme_num_iter=${nvme_num_iter:-"1000"}
> > +# Check consistency of NVMET_TRTYPES and nvme_trtype configurations.
> > +# If neither is configured, set the default value.
> > +first_call=${first_call:-1}
> > +if ((first_call)); then
> > +	if [[ -n $nvme_trtype ]]; then
> > +		if [[ -n $NVMET_TRTYPES ]]; then
> > +			echo "Both nvme_trtype and NVMET_TRTYPES are specified"
> > +			exit 1
> > +		fi
> > +		NVMET_TRTYPES="$nvme_trtype"
> > +	elif [[ -z $NVMET_TRTYPES ]]; then
> > +		nvme_trtype="loop"
> > +		NVMET_TRTYPES="$nvme_trtype"
> > +	fi
> > +	first_call=0
> > +fi
> 
> It would be nice to have the string check be done at first so you don't
> get any typo-related error later during the execution.

Agreed, will add a check helper function for it.
diff mbox series

Patch

diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md
index ae80860..144acd1 100644
--- a/Documentation/running-tests.md
+++ b/Documentation/running-tests.md
@@ -102,8 +102,13 @@  RUN_ZONED_TESTS=1
 
 The NVMe tests can be additionally parameterized via environment variables.
 
+- NVMET_TRTYPES: 'loop' (default), 'tcp', 'rdma' and 'fc'
+  Set up NVME target backends with the specified transport. Multiple transports
+  can be listed with separating spaces, e.g., "loop tcp rdma". In this case, the
+  tests are repeated to cover all of the transports specified.
 - nvme_trtype: 'loop' (default), 'tcp', 'rdma' and 'fc'
-  Run the tests with the given transport.
+  Run the tests with the given transport. This parameter is still usable but
+  replaced with NVMET_TRTYPES. Use NVMET_TRTYPES instead.
 - nvme_img_size: '1G' (default)
   Run the tests with given image size in bytes. 'm', 'M', 'g'
 	and 'G' postfix are supported.
@@ -117,11 +122,11 @@  These tests will use the siw (soft-iWARP) driver by default. The rdma_rxe
 
 ```sh
 To use the siw driver:
-nvme_trtype=rdma ./check nvme/
+NVMET_TRTYPES=rdma ./check nvme/
 ./check srp/
 
 To use the rdma_rxe driver:
-use_rxe=1 nvme_trtype=rdma ./check nvme/
+use_rxe=1 NVMET_TRTYPES=rdma ./check nvme/
 use_rxe=1 ./check srp/
 ```
 
diff --git a/tests/nvme/rc b/tests/nvme/rc
index 1f5ff44..34ecdde 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -18,10 +18,41 @@  def_hostid="0f01fb42-9f7f-4856-b0b3-51e60b8de349"
 def_hostnqn="nqn.2014-08.org.nvmexpress:uuid:${def_hostid}"
 export def_subsysnqn="blktests-subsystem-1"
 export def_subsys_uuid="91fdba0d-f87b-4c25-b80f-db7be1418b9e"
-nvme_trtype=${nvme_trtype:-"loop"}
 nvme_img_size=${nvme_img_size:-"1G"}
 nvme_num_iter=${nvme_num_iter:-"1000"}
 
+# Check consistency of NVMET_TRTYPES and nvme_trtype configurations.
+# If neither is configured, set the default value.
+first_call=${first_call:-1}
+if ((first_call)); then
+	if [[ -n $nvme_trtype ]]; then
+		if [[ -n $NVMET_TRTYPES ]]; then
+			echo "Both nvme_trtype and NVMET_TRTYPES are specified"
+			exit 1
+		fi
+		NVMET_TRTYPES="$nvme_trtype"
+	elif [[ -z $NVMET_TRTYPES ]]; then
+		nvme_trtype="loop"
+		NVMET_TRTYPES="$nvme_trtype"
+	fi
+	first_call=0
+fi
+
+_set_nvme_trtype() {
+	local index=$1
+	local -a types
+
+	read -r -a types <<< "$NVMET_TRTYPES"
+
+	if [[ -z $index ]]; then
+		echo ${#types[@]}
+		return
+	fi
+
+	nvme_trtype=${types[index]}
+	COND_DESC="nvmet tr=${nvme_trtype}"
+}
+
 # TMPDIR can not be referred out of test() or test_device() context. Instead of
 # global variable def_flie_path, use this getter function.
 _nvme_def_file_path() {