[PATCH blktests V2 0/3] nvme: add cntlid and model testcases

[PATCH blktests V2 0/3] nvme: add cntlid and model testcases

[Chaitanya Kulkarni]

Hi Omar,

This is a small patch-series which adds two new testcases for
setting up controller IDs and model from configfs.

I've tested these testcases with and without newly added attribute.
If they still fail on your system, I'll creata a new vm and test it
on nvme-5.5 branch.

Regards,
Chaitanya

Changes from V1:-

1. Reorg series into three patches.
2. Fix shellcheck warnings (and shellcheck on my machine :P)
3. Fix test description.
4. For model related testcases declare global variable model and use 
        nvme list | grep "${nvmedev}n1" | grep -o "$model"
   instead of 
        nvme list | grep ${nvmedev}n1 | grep -q test~model.

Chaitanya Kulkarni (3):
  nvme: allow target to set cntlid min/max & model
  nvme: test target cntlid min cntlid max
  nvme: test target model attribute

 tests/nvme/033     | 61 +++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/033.out |  4 +++
 tests/nvme/034     | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/034.out |  3 +++
 tests/nvme/rc      | 25 ++++++++++++++++++
 5 files changed, 157 insertions(+)
 create mode 100755 tests/nvme/033
 create mode 100644 tests/nvme/033.out
 create mode 100755 tests/nvme/034
 create mode 100644 tests/nvme/034.out

 Here is the test result with and without cntlid and model attributes :-
Shellcheck :-
# /bin/shellcheck -x tests/nvme/rc tests/nvme/033 tests/nvme/034 
# echo $?
0
# for i in 033 034; do ./check tests/nvme/${i} ; done 
nvme/033 (Test NVMeOF target cntlid[min|max] attributes)     [not run]
    attr_cntlid_[min|max] not found
nvme/034 (Test NVMeOF target model attribute)                [not run]
    attr_model not found

# for i in 033 034; do ./check tests/nvme/${i} ; done 
nvme/033 (Test NVMeOF target cntlid[min|max] attributes)     [passed]
    runtime  1.706s  ...  1.513s
nvme/034 (Test NVMeOF target model attribute)                [passed]
    runtime  1.672s  ...  1.510s

-- 
2.22.1

[PATCH blktests V2 1/3] nvme: allow target to set cntlid min/max & model

[Chaitanya Kulkarni]

This patch updates helper function create_nvmet_subsystem() to handle
newly introduced model, cntlid_min and cntlid_max attributes.
Also, this adds SKIP reason when attributes are not found in the
configfs.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 tests/nvme/rc | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 40f0413..e4b57cb 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -121,11 +121,36 @@ _create_nvmet_subsystem() {
 	local nvmet_subsystem="$1"
 	local blkdev="$2"
 	local uuid=$3
+	local cntlid_min=$4
+	local cntlid_max=$5
+	local model=$6
 	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
 
 	mkdir -p "${cfs_path}"
 	echo 1 > "${cfs_path}/attr_allow_any_host"
+
+	if [ $# -eq 5 ]; then
+		if [ -f "${cfs_path}"/attr_cntlid_min ]; then
+			echo "${cntlid_min}" > "${cfs_path}"/attr_cntlid_min
+			echo "${cntlid_max}" > "${cfs_path}"/attr_cntlid_max
+		else
+			SKIP_REASON="attr_cntlid_[min|max] not found"
+			rmdir "${cfs_path}"
+			return 1
+		fi
+	fi
+	if [ $# -eq 6 ]; then
+		if [ -f "${cfs_path}"/attr_model ]; then
+			echo "${model}" > "${cfs_path}"/attr_model
+		else
+			SKIP_REASON="attr_model not found"
+			rmdir "${cfs_path}"
+			return 1
+		fi
+	fi
 	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
+
+	return 0
 }
 
 _remove_nvmet_ns() {
-- 
2.22.1

[PATCH blktests V2 2/3] nvme: test target cntlid min cntlid max

[Chaitanya Kulkarni]

The new testcases exercises newly added cntlid [min|max] attributes
for NVMeOF target.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 tests/nvme/033     | 61 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/033.out |  4 +++
 2 files changed, 65 insertions(+)
 create mode 100755 tests/nvme/033
 create mode 100644 tests/nvme/033.out

diff --git a/tests/nvme/033 b/tests/nvme/033
new file mode 100755
index 0000000..49f2fa1
--- /dev/null
+++ b/tests/nvme/033
@@ -0,0 +1,61 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2017-2018 Western Digital Corporation or its affiliates.
+#
+# Test NVMeOF target cntlid[min|max] attributes.
+
+. tests/nvme/rc
+
+DESCRIPTION="Test NVMeOF target cntlid[min|max] attributes"
+QUICK=1
+
+PORT=""
+NVMEDEV=""
+LOOP_DEV=""
+FILE_PATH="$TMPDIR/img"
+SUBSYS_NAME="blktests-subsystem-1"
+
+_have_cid_min_max()
+{
+	local cid_min=14
+	local cid_max=15
+
+	_setup_nvmet
+	truncate -s 1G "${FILE_PATH}"
+	LOOP_DEV="$(losetup -f --show "${FILE_PATH}")"
+
+	# we can only know skip reason when we create a subsys
+	 _create_nvmet_subsystem "${SUBSYS_NAME}" "${LOOP_DEV}" \
+		"91fdba0d-f87b-4c25-b80f-db7be1418b9e" ${cid_min} ${cid_max}
+}
+
+requires() {
+	_have_program nvme && _have_modules loop nvme-loop nvmet && \
+		_have_configfs && _have_cid_min_max
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	PORT="$(_create_nvmet_port "loop")"
+	_add_nvmet_subsys_to_port "${PORT}" "${SUBSYS_NAME}"
+
+	nvme connect -t loop -n "${SUBSYS_NAME}"
+
+	udevadm settle
+
+	NVMEDEV="$(_find_nvme_loop_dev)"
+	nvme id-ctrl /dev/"${NVMEDEV}"n1 | grep cntlid | tr -s ' ' ' '
+
+	nvme disconnect -n "${SUBSYS_NAME}"
+
+	_remove_nvmet_subsystem_from_port "${PORT}" "${SUBSYS_NAME}"
+	_remove_nvmet_subsystem "${SUBSYS_NAME}"
+	_remove_nvmet_port "${PORT}"
+
+	losetup -d "${LOOP_DEV}"
+
+	rm "${FILE_PATH}"
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/033.out b/tests/nvme/033.out
new file mode 100644
index 0000000..b1b0d15
--- /dev/null
+++ b/tests/nvme/033.out
@@ -0,0 +1,4 @@
+Running nvme/033
+cntlid : e
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.22.1

[PATCH blktests V2 3/3] nvme: test target model attribute

[Chaitanya Kulkarni]

The new testcases exercises newly added model attribute for
NVMeOF target.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 tests/nvme/034     | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/034.out |  3 +++
 2 files changed, 67 insertions(+)
 create mode 100755 tests/nvme/034
 create mode 100644 tests/nvme/034.out

diff --git a/tests/nvme/034 b/tests/nvme/034
new file mode 100755
index 0000000..0e51a62
--- /dev/null
+++ b/tests/nvme/034
@@ -0,0 +1,64 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2017-2018 Western Digital Corporation or its affiliates.
+#
+# Test NVMeOF target model attributes.
+
+. tests/nvme/rc
+
+DESCRIPTION="Test NVMeOF target model attribute"
+QUICK=1
+
+PORT=""
+NVMEDEV=""
+LOOP_DEV=""
+MODEL="test~model"
+FILE_PATH="$TMPDIR/img"
+SUBSYS_NAME="blktests-subsystem-1"
+
+_have_model()
+
+{
+	_setup_nvmet
+	truncate -s 1G "${FILE_PATH}"
+	LOOP_DEV="$(losetup -f --show "${FILE_PATH}")"
+
+	# we can only know skip reason when we create a subsys
+	 _create_nvmet_subsystem "${SUBSYS_NAME}" "${LOOP_DEV}" \
+		"91fdba0d-f87b-4c25-b80f-db7be1418b9e" 14 15 \
+		${MODEL}
+}
+
+requires() {
+	_have_program nvme && _have_modules loop nvme-loop nvmet && \
+		_have_configfs && _have_model
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	PORT="$(_create_nvmet_port "loop")"
+	_add_nvmet_subsys_to_port "${PORT}" "${SUBSYS_NAME}"
+
+	nvme connect -t loop -n "${SUBSYS_NAME}"
+
+	udevadm settle
+
+	NVMEDEV="$(_find_nvme_loop_dev)"
+	nvme list | grep "${NVMEDEV}"n1 | grep -q "${MODEL}"
+	result=$?
+
+	nvme disconnect -n "${SUBSYS_NAME}"
+
+	_remove_nvmet_subsystem_from_port "${PORT}" "${SUBSYS_NAME}"
+	_remove_nvmet_subsystem "${SUBSYS_NAME}"
+	_remove_nvmet_port "${PORT}"
+
+	losetup -d "${LOOP_DEV}"
+
+	rm "${FILE_PATH}"
+
+	if [ ${result} -eq 0 ]; then
+		echo "Test complete"
+	fi
+}
diff --git a/tests/nvme/034.out b/tests/nvme/034.out
new file mode 100644
index 0000000..0a7bd2f
--- /dev/null
+++ b/tests/nvme/034.out
@@ -0,0 +1,3 @@
+Running nvme/034
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.22.1

Re: [PATCH blktests V2 2/3] nvme: test target cntlid min cntlid max

[Omar Sandoval]

On Fri, Feb 14, 2020 at 05:38:30PM -0800, Chaitanya Kulkarni wrote:
> The new testcases exercises newly added cntlid [min|max] attributes
> for NVMeOF target.
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  tests/nvme/033     | 61 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/033.out |  4 +++
>  2 files changed, 65 insertions(+)
>  create mode 100755 tests/nvme/033
>  create mode 100644 tests/nvme/033.out
> 
> diff --git a/tests/nvme/033 b/tests/nvme/033
> new file mode 100755
> index 0000000..49f2fa1
> --- /dev/null
> +++ b/tests/nvme/033
> @@ -0,0 +1,61 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2017-2018 Western Digital Corporation or its affiliates.
> +#
> +# Test NVMeOF target cntlid[min|max] attributes.
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Test NVMeOF target cntlid[min|max] attributes"
> +QUICK=1
> +
> +PORT=""
> +NVMEDEV=""
> +LOOP_DEV=""
> +FILE_PATH="$TMPDIR/img"
> +SUBSYS_NAME="blktests-subsystem-1"
> +
> +_have_cid_min_max()
> +{
> +	local cid_min=14
> +	local cid_max=15
> +
> +	_setup_nvmet
> +	truncate -s 1G "${FILE_PATH}"
> +	LOOP_DEV="$(losetup -f --show "${FILE_PATH}")"
> +
> +	# we can only know skip reason when we create a subsys
> +	 _create_nvmet_subsystem "${SUBSYS_NAME}" "${LOOP_DEV}" \
> +		"91fdba0d-f87b-4c25-b80f-db7be1418b9e" ${cid_min} ${cid_max}
> +}

Sorry, I wasn't ignoring these patches, they just made me realize that
we really do need a way to skip a test from the test function itself, so
I wanted to implement that first. Could you try rebasing on my skip-test
branch (https://github.com/osandov/blktests/tree/skip-test) and
reworking this so you don't have to split the setup between requires()
and test()?

> +
> +requires() {
> +	_have_program nvme && _have_modules loop nvme-loop nvmet && \
> +		_have_configfs && _have_cid_min_max
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	PORT="$(_create_nvmet_port "loop")"
> +	_add_nvmet_subsys_to_port "${PORT}" "${SUBSYS_NAME}"
> +
> +	nvme connect -t loop -n "${SUBSYS_NAME}"
> +
> +	udevadm settle
> +
> +	NVMEDEV="$(_find_nvme_loop_dev)"
> +	nvme id-ctrl /dev/"${NVMEDEV}"n1 | grep cntlid | tr -s ' ' ' '
> +
> +	nvme disconnect -n "${SUBSYS_NAME}"
> +
> +	_remove_nvmet_subsystem_from_port "${PORT}" "${SUBSYS_NAME}"
> +	_remove_nvmet_subsystem "${SUBSYS_NAME}"
> +	_remove_nvmet_port "${PORT}"
> +
> +	losetup -d "${LOOP_DEV}"
> +
> +	rm "${FILE_PATH}"
> +
> +	echo "Test complete"
> +}
> diff --git a/tests/nvme/033.out b/tests/nvme/033.out
> new file mode 100644
> index 0000000..b1b0d15
> --- /dev/null
> +++ b/tests/nvme/033.out
> @@ -0,0 +1,4 @@
> +Running nvme/033
> +cntlid : e
> +NQN:blktests-subsystem-1 disconnected 1 controller(s)
> +Test complete
> -- 
> 2.22.1
> 

Re: [PATCH blktests V2 1/3] nvme: allow target to set cntlid min/max & model

[Omar Sandoval]

On Fri, Feb 14, 2020 at 05:38:29PM -0800, Chaitanya Kulkarni wrote:
> This patch updates helper function create_nvmet_subsystem() to handle
> newly introduced model, cntlid_min and cntlid_max attributes.
> Also, this adds SKIP reason when attributes are not found in the
> configfs.
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  tests/nvme/rc | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 40f0413..e4b57cb 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -121,11 +121,36 @@ _create_nvmet_subsystem() {
>  	local nvmet_subsystem="$1"
>  	local blkdev="$2"
>  	local uuid=$3
> +	local cntlid_min=$4
> +	local cntlid_max=$5
> +	local model=$6
>  	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
>  
>  	mkdir -p "${cfs_path}"
>  	echo 1 > "${cfs_path}/attr_allow_any_host"
> +
> +	if [ $# -eq 5 ]; then

I still don't like that we ignore this if we also pass the model.
Instead, just make this

if [[ -n $cntlid ]]

Then the caller can pass an empty argument if they want it ignored.

> +		if [ -f "${cfs_path}"/attr_cntlid_min ]; then
> +			echo "${cntlid_min}" > "${cfs_path}"/attr_cntlid_min
> +			echo "${cntlid_max}" > "${cfs_path}"/attr_cntlid_max
> +		else
> +			SKIP_REASON="attr_cntlid_[min|max] not found"
> +			rmdir "${cfs_path}"
> +			return 1
> +		fi
> +	fi

Re: [PATCH blktests V2 2/3] nvme: test target cntlid min cntlid max

[Chaitanya Kulkarni]

On 03/04/2020 05:30 PM, Omar Sandoval wrote:
>> +	 _create_nvmet_subsystem "${SUBSYS_NAME}" "${LOOP_DEV}" \
>> >+		"91fdba0d-f87b-4c25-b80f-db7be1418b9e" ${cid_min} ${cid_max}
>> >+}
> Sorry, I wasn't ignoring these patches, they just made me realize that
> we really do need a way to skip a test from the test function itself, so
> I wanted to implement that first. Could you try rebasing on my skip-test
> branch (https://github.com/osandov/blktests/tree/skip-test) and
> reworking this so you don't have to split the setup between requires()
> and test()?
>

Sure, thanks for adding this, will send an updated series.

>> >