[PATCH blktests v2 05/13] nvme/{rc,002,016,017,030}: pass port ID to genctr helper functions

[Hannes Reinecke]

On 10/30/24 07:51, Shin'ichiro Kawasaki wrote:
> Commit "nvme: sanitize transport parameter in _create_nvmet_port()"
> introduced wwnn and wwpn calculation based on port ID for fc transport.
> This means that --traddr and --host-traddr options for "nvme discover"
> command should reflect the port ID. This change was missing for the
> genctr, or Generation Counter, related helper functions and it made
> the test case nvme/030 fail.
> 
> To avoid the failure, pass the port ID to the genctr related helper
> functions and reflect the port ID for fc transport.
> 
> Reviewed-by: Daniel Wagner <dwagner at suse.de>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
> ---
>   tests/nvme/002 |  3 ++-
>   tests/nvme/016 |  3 ++-
>   tests/nvme/017 |  3 ++-
>   tests/nvme/030 | 13 ++++++++-----
>   tests/nvme/rc  | 18 +++++++++++-------
>   5 files changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/nvme/002 b/tests/nvme/002
> index e5b751d..b87570a 100755
> --- a/tests/nvme/002
> +++ b/tests/nvme/002
> @@ -39,7 +39,8 @@ test() {
>   		_add_nvmet_subsys_to_port "${port}" "blktests-subsystem-$i"
>   	done
>   
> -	genctr=$(_check_genctr "${genctr}" "adding a subsystem to a port")
> +	genctr=$(_check_genctr "${genctr}" "$port" \
> +			       "adding a subsystem to a port")
>   
>   	for ((i = iterations - 1; i >= 0; i--)); do
>   		_remove_nvmet_subsystem_from_port "${port}" "blktests-subsystem-$i"
> diff --git a/tests/nvme/016 b/tests/nvme/016
> index 8ab9d22..13d2c46 100755
> --- a/tests/nvme/016
> +++ b/tests/nvme/016
> @@ -41,7 +41,8 @@ test() {
>   	port="$(_create_nvmet_port)"
>   	_add_nvmet_subsys_to_port "$port" "${def_subsysnqn}"
>   
> -	genctr=$(_check_genctr "${genctr}" "adding a subsystem to a port")
> +	genctr=$(_check_genctr "${genctr}" "$port" \
> +			       "adding a subsystem to a port")
>   
>   	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
>   	_remove_nvmet_port "${port}"
> diff --git a/tests/nvme/017 b/tests/nvme/017
> index dd302fc..10ee38c 100755
> --- a/tests/nvme/017
> +++ b/tests/nvme/017
> @@ -42,7 +42,8 @@ test() {
>   	port="$(_create_nvmet_port)"
>   	_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
>   
> -	genctr=$(_check_genctr "${genctr}" "adding a subsystem to a port")
> +	genctr=$(_check_genctr "${genctr}" "$port" \
> +			       "adding a subsystem to a port")
>   
>   	_remove_nvmet_subsystem_from_port "${port}" "${def_subsysnqn}"
>   	_remove_nvmet_port "${port}"
> diff --git a/tests/nvme/030 b/tests/nvme/030
> index 1ba1274..596e411 100755
> --- a/tests/nvme/030
> +++ b/tests/nvme/030
> @@ -35,26 +35,29 @@ test() {
>   	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
>   	_create_nvmet_host "${subsys}1" "${def_hostnqn}"
>   
> -	genctr=$(_discovery_genctr)
> +	genctr=$(_discovery_genctr "$port")
>   
>   	_create_nvmet_subsystem --subsysnqn "${subsys}2" \
>   				--blkdev "$(losetup -f)"
>   	_add_nvmet_subsys_to_port "${port}" "${subsys}2"
>   
> -	genctr=$(_check_genctr "${genctr}" "adding a subsystem to a port")
> +	genctr=$(_check_genctr "${genctr}" "$port" \
> +			       "adding a subsystem to a port")
>   
>   	_add_nvmet_allow_hosts "${subsys}2" "${def_hostnqn}"
>   
> -	genctr=$(_check_genctr "${genctr}" "adding host to allow_hosts")
> +	genctr=$(_check_genctr "${genctr}" "$port" "adding host to allow_hosts")
>   
>   	_remove_nvmet_allow_hosts "${subsys}2" "${def_hostnqn}"
>   
> -	genctr=$(_check_genctr "${genctr}" "removing host from allow_hosts")
> +	genctr=$(_check_genctr "${genctr}" "$port" \
> +			       "removing host from allow_hosts")
>   
>   	_remove_nvmet_subsystem_from_port "${port}" "${subsys}2"
>   	_remove_nvmet_subsystem "${subsys}2"
>   
> -	genctr=$(_check_genctr "${genctr}" "removing a subsystem from a port")
> +	genctr=$(_check_genctr "${genctr}" "$port" \
> +			       "removing a subsystem from a port")
>   
>   	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
>   	_remove_nvmet_subsystem "${subsys}1"
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 5b4629d..64a31e0 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -222,15 +222,16 @@ _nvme_calc_rand_io_size() {
>   
>   _nvme_discover() {
>   	local trtype="$1"
> -	local traddr="${2:-$def_traddr}"
> -	local host_traddr="${3:-$def_host_traddr}"
> -	local trsvcid="${3:-$def_trsvcid}"
> +	local port="$2"
> +	local traddr="$def_traddr"
> +	local trsvcid="$def_trsvcid"
>   
>   	ARGS=(--transport "${trtype}")
>   	ARGS+=(--hostnqn="${def_hostnqn}")
>   	ARGS+=(--hostid="${def_hostid}")
>   	if [[ "${trtype}" = "fc" ]]; then
> -		ARGS+=(--traddr "${traddr}" --host-traddr "${host_traddr}")
> +		ARGS+=(--traddr "$(_fc_traddr "$port")")
> +		ARGS+=(--host-traddr "$(_fc_host_traddr "$port")")
>   	elif [[ "${trtype}" != "loop" ]]; then
>   		ARGS+=(--traddr "${traddr}" --trsvcid "${trsvcid}")
>   	fi
> @@ -463,16 +464,19 @@ _nvmet_passthru_target_cleanup() {
>   }
>   
>   _discovery_genctr() {
> -	_nvme_discover "${nvme_trtype}" |
> +	local port=${1}
> +
> +	_nvme_discover "${nvme_trtype}" "${port}" |
>   		sed -n -e 's/^.*Generation counter \([0-9]\+\).*$/\1/p'
>   }
>   
>   _check_genctr() {
>   	local last=$1
> -	local msg=$2
> +	local port=$2
> +	local msg=$3
>   	local genctr
>   
> -	genctr=$(_discovery_genctr)
> +	genctr=$(_discovery_genctr "$port")
>   	if (( "${genctr}" <= "${last}" )); then
>   		echo "Generation counter not incremented when ${msg} (${genctr} <= ${last})"
>   	fi
See my comments for the previous patch regarding local wwpn/wwnn setup.
Otherwise looks fine.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare at suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich