[PATCH v2 3/7] nvme: make tests transport type agnostic

Chaitanya Kulkarni Chaitanya.Kulkarni at wdc.com
Thu Aug 6 23:09:27 EDT 2020


On 8/6/20 12:15, Sagi Grimberg wrote:
> Pass in nvme_trtype to common routines that can
> support multiple transport types.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   tests/nvme/002 |  4 ++--
>   tests/nvme/003 |  4 ++--
>   tests/nvme/004 |  6 +++---
>   tests/nvme/005 |  6 +++---
>   tests/nvme/006 |  2 +-
>   tests/nvme/007 |  2 +-
>   tests/nvme/008 |  6 +++---
>   tests/nvme/009 |  6 +++---
>   tests/nvme/010 |  6 +++---
>   tests/nvme/011 |  6 +++---
>   tests/nvme/012 |  6 +++---
>   tests/nvme/013 |  6 +++---
>   tests/nvme/014 |  6 +++---
>   tests/nvme/015 |  6 +++---
>   tests/nvme/016 |  2 +-
>   tests/nvme/017 |  2 +-
>   tests/nvme/018 |  6 +++---
>   tests/nvme/019 |  6 +++---
>   tests/nvme/020 |  6 +++---
>   tests/nvme/021 |  6 +++---
>   tests/nvme/022 |  6 +++---
>   tests/nvme/023 |  6 +++---
>   tests/nvme/024 |  6 +++---
>   tests/nvme/025 |  6 +++---
>   tests/nvme/026 |  6 +++---
>   tests/nvme/027 |  6 +++---
>   tests/nvme/028 |  8 ++++----
>   tests/nvme/029 |  6 +++---
>   tests/nvme/030 |  2 +-
>   tests/nvme/031 |  4 ++--
>   tests/nvme/rc  | 39 ++++++++++++++++++++++++++++++++-------
>   31 files changed, 110 insertions(+), 85 deletions(-)
> 
> diff --git a/tests/nvme/002 b/tests/nvme/002
> index 999e222705bf..8540623497c7 100755
> --- a/tests/nvme/002
> +++ b/tests/nvme/002
> @@ -21,7 +21,7 @@ test() {
>   
>   	local iterations=1000
>   	local port
> -	port="$(_create_nvmet_port "loop")"
> +	port="$(_create_nvmet_port ${nvme_trtype})"
Is there a way to directly use nvme_trtype especially in rc ?
if not disregard this comment.
>   
>   	local loop_dev
>   	loop_dev="$(losetup -f)"
> @@ -31,7 +31,7 @@ test() {
>   		_add_nvmet_subsys_to_port "${port}" "blktests-subsystem-$i"
>   	done
>   
> -	_nvme_discover "loop" | _filter_discovery
> +	_nvme_discover "${nvme_trtype}" | _filter_discovery
Same here for nvme_trtype.
>   
>   	for ((i = iterations - 1; i >= 0; i--)); do
>   		_remove_nvmet_subsystem_from_port "${port}" "blktests-subsystem-$i"
> diff --git a/tests/nvme/003 b/tests/nvme/003
> index 6ea6a23b7942..68f823011d7d 100755
> --- a/tests/nvme/003
> +++ b/tests/nvme/003
> @@ -21,7 +21,7 @@ test() {
>   	_setup_nvmet
>   
>   	local port
> -	port="$(_create_nvmet_port "loop")"
> +	port="$(_create_nvmet_port "${nvme_trtype}")"
>   
>   	local loop_dev
>   	loop_dev="$(losetup -f)"
> @@ -29,7 +29,7 @@ test() {
>   	_create_nvmet_subsystem "blktests-subsystem-1" "${loop_dev}"
>   	_add_nvmet_subsys_to_port "${port}" "blktests-subsystem-1"
>   
> -	_nvme_connect_subsys "loop" "nqn.2014-08.org.nvmexpress.discovery"
> +	_nvme_connect_subsys "${nvme_trtype}" "nqn.2014-08.org.nvmexpress.discovery"
>   
Same here for nvme_trtype.
>   	# This is ugly but checking for the absence of error messages is ...
>   	sleep 10
> diff --git a/tests/nvme/004 b/tests/nvme/004

> diff --git a/tests/nvme/018 b/tests/nvme/018
> index 4863274cc525..43340d3c4d25 100755
> --- a/tests/nvme/018
> +++ b/tests/nvme/018
> @@ -29,12 +29,12 @@ test() {
>   
>   	_create_nvmet_subsystem "${subsys_name}" "${file_path}" \
>   		 "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> -	port="$(_create_nvmet_port "loop")"
> +	port="$(_create_nvmet_port ${nvme_trtype})"
>   	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
>   
> -	_nvme_connect_subsys "loop" "${subsys_name}"
> +	_nvme_connect_subsys ${nvme_trtype} "${subsys_name}"
>   
> -	nvmedev="$(_find_nvme_loop_dev)"
> +	nvmedev="$(_find_nvme_dev)"
>   	cat "/sys/block/${nvmedev}n1/uuid"
>   	cat "/sys/block/${nvmedev}n1/wwid"
>   
> diff --git a/tests/nvme/019 b/tests/nvme/019
> index 19c5b4755387..98d82ae21b42 100755
> --- a/tests/nvme/019
> +++ b/tests/nvme/019
> @@ -33,12 +33,12 @@ test() {
>   
>   	_create_nvmet_subsystem "${subsys_name}" "${loop_dev}" \
>   		"91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> -	port="$(_create_nvmet_port "loop")"
> +	port="$(_create_nvmet_port ${nvme_trtype})"> diff --git a/tests/nvme/004 b/tests/nvme/004
> index 7ea539fda55e..af434674beaa 100755
> --- a/tests/nvme/004
> +++ b/tests/nvme/004
> @@ -22,7 +22,7 @@ test() {
>   	_setup_nvmet
>   
>   	local port
> -	port="$(_create_nvmet_port "loop")"
> +	port="$(_create_nvmet_port ${nvme_trtype})"
>   
>   	truncate -s 1G "$TMPDIR/img"
>   
> @@ -33,10 +33,10 @@ test() {
>   		"91fdba0d-f87b-4c25-b80f-db7be1418b9e"
>   	_add_nvmet_subsys_to_port "${port}" "blktests-subsystem-1"
>   
> -	_nvme_connect_subsys "loop" "blktests-subsystem-1"
> +	_nvme_connect_subsys ${nvme_trtype} "blktests-subsystem-1"
>   
>   	local nvmedev
> -	nvmedev="$(_find_nvme_loop_dev)"
> +	nvmedev="$(_find_nvme_dev)"
>   	cat "/sys/block/${nvmedev}n1/uuid"
>   	cat "/sys/block/${nvmedev}n1/wwid"

Since we are touching nvmedev can we move above uuid and wwid to
a wrapper something like _nvme_show_uuid_wwid ${nvmedev}n1 ?

>
> @@ -36,12 +36,12 @@ test() {
>   
>   	loop_dev="$(losetup -f --show "$TMPDIR/img")"
>   
> -	port="$(_create_nvmet_port "loop")"
> +	port="$(_create_nvmet_port ${nvme_trtype})"
>   
>   	for ((i = 0; i < iterations; i++)); do
>   		_create_nvmet_subsystem "${subsys}$i" "${loop_dev}"
>   		_add_nvmet_subsys_to_port "${port}" "${subsys}$i"
> -		_nvme_connect_subsys "loop" "${subsys}$i"
> +		_nvme_connect_subsys ${nvme_trtype} "${subsys}$i"
Same here for nvme_trtype as first comment.
>   		_nvme_disconnect_subsys "${subsys}$i" >> "${FULL}" 2>&1
>   		_remove_nvmet_subsystem_from_port "${port}" "${subsys}$i"
>   		_remove_nvmet_subsystem "${subsys}$i"
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 6d57cf591300..191f0497416a 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -6,6 +6,9 @@
>   
>   . common/rc
>   
> +def_traddr="127.0.0.1"
> +def_adrfam="ipv4"
> +def_trsvcid="4420"
>   nvme_trtype=${nvme_trtype:-"loop"}
>   
>   _nvme_requires() {
> @@ -62,8 +65,8 @@ _cleanup_nvmet() {
>   	for dev in /sys/class/nvme/nvme*; do
>   		dev="$(basename "$dev")"
>   		transport="$(cat "/sys/class/nvme/${dev}/transport")"
> -		if [[ "$transport" == "loop" ]]; then
> -			echo "WARNING: Test did not clean up loop device: ${dev}"
> +		if [[ "$transport" == "${nvme_trtype}" ]]; then
> +			echo "WARNING: Test did not clean up ${nvme_trtype} device: ${dev}"
>   			_nvme_disconnect_ctrl "${dev}"
>   		fi
>   	done
> @@ -87,14 +90,20 @@ _cleanup_nvmet() {
>   	shopt -u nullglob
>   	trap SIGINT
>   
> -	modprobe -r nvme-loop 2>/dev/null
> +	modprobe -r nvme-${nvme_trtype} 2>/dev/null
> +	if [[ "${nvme_trtype}" != "loop" ]]; then
> +		modprobe -r nvmet-${nvme_trtype} 2>/dev/null
This is not from your patch but I'd keep the error message it has
turned out to be useful for me when debugging refcount problem 
especially unload and load scenario.

> +	fi
>   	modprobe -r nvmet 2>/dev/null
>   }
>   
>   _setup_nvmet() {
>   	_register_test_cleanup _cleanup_nvmet
>   	modprobe nvmet
> -	modprobe nvme-loop
> +	if [[ "${nvme_trtype}" != "loop" ]]; then
> +		modprobe nvmet-${nvme_trtype}
> +	fi
> +	modprobe nvme-${nvme_trtype}
>   }
>   
>   _nvme_disconnect_ctrl() {
> @@ -112,20 +121,33 @@ _nvme_disconnect_subsys() {
>   _nvme_connect_subsys() {
>   	local trtype="$1"
>   	local subsysnqn="$2"
> +	local traddr="${3:-$def_traddr}"
> +	local trsvcid="${4:-$def_trsvcid}"
>   
>   	cmd="nvme connect -t ${trtype} -n ${subsysnqn}"
> +	if [[ "${trtype}" != "loop" ]]; then
> +		cmd=$cmd" -a ${traddr} -s ${trsvcid}"
> +	fi
>   	eval $cmd
>   }
>   
>   _nvme_discover() {
>   	local trtype="$1"
> +	local traddr="${2:-$def_traddr}"
> +	local trsvcid="${3:-$def_trsvcid}"
>   
>   	cmd="nvme discover -t ${trtype}"
> +	if [[ "${trtype}" != "loop" ]]; then
> +		cmd=$cmd" -a ${traddr} -s ${trsvcid}"
> +	fi
>   	eval $cmd
>   }
>   
>   _create_nvmet_port() {
>   	local trtype="$1"
> +	local traddr="${2:-$def_traddr}"
> +	local adrfam="${3:-$def_adrfam}"
> +	local trsvcid="${4:-$def_trsvcid}"
>   
>   	local port
>   	for ((port = 0; ; port++)); do
> @@ -136,6 +158,9 @@ _create_nvmet_port() {
>   
>   	mkdir "${NVMET_CFS}/ports/${port}"
>   	echo "${trtype}" > "${NVMET_CFS}/ports/${port}/addr_trtype"
> +	echo "${traddr}" > "${NVMET_CFS}/ports/${port}/addr_traddr"
> +	echo "${adrfam}" > "${NVMET_CFS}/ports/${port}/addr_adrfam"
> +	echo "${trsvcid}" > "${NVMET_CFS}/ports/${port}/addr_trsvcid"

Do we need argument count check before we apply these to configfs ?
>   
>   	echo "${port}"
>   }
> @@ -207,13 +232,13 @@ _remove_nvmet_subsystem_from_port() {
>   	rm "${NVMET_CFS}/ports/${port}/subsystems/${nvmet_subsystem}"
>   }
>   
> -_find_nvme_loop_dev() {
> +_find_nvme_dev() {
>   	local dev
>   	local transport
>   	for dev in /sys/class/nvme/nvme*; do
>   		dev="$(basename "$dev")"
>   		transport="$(cat "/sys/class/nvme/${dev}/transport")"
> -		if [[ "$transport" == "loop" ]]; then
> +		if [[ "$transport" == "${nvme_trtype}" ]]; then
>   			echo "$dev"
>   			for ((i = 0; i < 10; i++)); do
>   				if [[ -e /sys/block/$dev/uuid &&
> @@ -233,6 +258,6 @@ _filter_discovery() {
>   }
>   
>   _discovery_genctr() {
> -	_nvme_discover "loop" |
> +	_nvme_discover "${nvme_trtype}" |
>   		sed -n -e 's/^.*Generation counter \([0-9]\+\).*$/\1/p'
>   }
> 




More information about the Linux-nvme mailing list