[PATCHv2 0/2] blktests: test ANA base support

[PATCHv2 0/2] blktests: test ANA base support

[Hannes Reinecke]

Hi all,

here's a small patchset for testing ANA base support.
The test itself requires the ANA patchset from Christoph currently queued
in nvme-4.19.

Changes to v1:
- Include reviews from Chaitanya
- Match on namespace UUID when figuring out which NS to use
- Drop sysfs names from the output to allow the testcase to run
  on machines with other NVMe devices

Hannes Reinecke (2):
  blktests: enable ANA support
  blktests: add test for ANA state transition

 tests/nvme/018     | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/018.out |  18 +++++++
 tests/nvme/rc      |  89 +++++++++++++++++++++++++++++--
 3 files changed, 257 insertions(+), 3 deletions(-)
 create mode 100755 tests/nvme/018
 create mode 100644 tests/nvme/018.out

-- 
2.12.3

[PATCH 1/2] blktests: enable ANA support

[Hannes Reinecke]

Update functions to support ANA.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 tests/nvme/rc | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 86 insertions(+), 3 deletions(-)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index fb5dbdf..ecbd9b9 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -27,7 +27,7 @@ group_device_requires() {
 	_test_dev_is_nvme
 }
 
-NVMET_CFS="/sys/kernel/config/nvmet/"
+NVMET_CFS="/sys/kernel/config/nvmet"
 
 _test_dev_is_nvme() {
 	if ! readlink -f "$TEST_DEV_SYSFS/device" | grep -q nvme; then
@@ -49,6 +49,7 @@ _create_nvmet_port() {
 
 	mkdir "${NVMET_CFS}/ports/${port}"
 	echo "${trtype}" > "${NVMET_CFS}/ports/${port}/addr_trtype"
+	echo "${port}" > "${NVMET_CFS}/ports/${port}/addr_traddr"
 
 	echo "${port}"
 }
@@ -58,6 +59,62 @@ _remove_nvmet_port() {
 	rmdir "${NVMET_CFS}/ports/${port}"
 }
 
+_create_nvmet_anagroup() {
+	local port="$1"
+	local port_cfs="${NVMET_CFS}/ports/${port}"
+	local grpid
+
+	for ((grpid = 1; ; grpid++)); do
+		if [[ ! -e "${port_cfs}/ana_groups/${grpid}" ]]; then
+			break
+		fi
+	done
+
+	mkdir "${port_cfs}/ana_groups/${grpid}"
+
+	echo "${grpid}"
+}
+
+_remove_nvmet_anagroup() {
+	local port="$1"
+	local grpid="$2"
+	local ana_cfs="${NVMET_CFS}/ports/${port}/ana_groups/${grpid}"
+
+	rmdir "${ana_cfs}"
+}
+
+_set_nvmet_anagroup_state() {
+	local port="$1"
+	local grpid="$2"
+	local state="$3"
+	local ana_cfs="${NVMET_CFS}/ports/${port}/ana_groups/${grpid}"
+
+	echo "${state}" > "${ana_cfs}/ana_state"
+}
+
+_display_ana_state() {
+	local nvmet_subsystem="$1"
+	local grpid state cntlid
+
+	for nvme in /sys/class/nvme/* ; do
+		[ -f ${nvme}/subsysnqn ] || continue
+		subsys="$(cat "${nvme}/subsysnqn")"
+		if [ "${subsys}" != "${nvmet_subsystem}" ] ; then
+			continue
+		fi
+		cntlid="$(cat "${nvme}/cntlid")"
+		for c in ${nvme}/nvme* ; do
+			if [ ! -d ${c} ] ; then
+				echo "${cntlid}: ANA disabled"
+				continue
+			fi
+			grpid="$(cat "${c}/ana_grpid")"
+			state="$(cat "${c}/ana_state")"
+			echo "${cntlid}: grpid ${grpid} state ${state}"
+		done
+	done
+}
+
 _create_nvmet_ns() {
 	local nvmet_subsystem="$1"
 	local nsid="$2"
@@ -65,14 +122,22 @@ _create_nvmet_ns() {
 	local uuid="00000000-0000-0000-0000-000000000000"
 	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
 	local ns_path="${subsys_path}/namespaces/${nsid}"
+	local ana_grpid
 
-	if [[ $# -eq 4 ]]; then
+	if [[ $# -ge 4 ]]; then
 		uuid="$4"
 	fi
 
+	if [[ $# -eq 5 ]]; then
+		ana_grpid="$5"
+	fi
+
 	mkdir "${ns_path}"
 	printf "%s" "${blkdev}" > "${ns_path}/device_path"
 	printf "%s" "${uuid}" > "${ns_path}/device_uuid"
+	if [ -n "${ana_grpid}" ] ; then
+		printf "%s" "${ana_grpid}" > "${ns_path}/ana_grpid"
+	fi
 	printf 1 > "${ns_path}/enable"
 }
 
@@ -80,11 +145,12 @@ _create_nvmet_subsystem() {
 	local nvmet_subsystem="$1"
 	local blkdev="$2"
 	local uuid=$3
+	local ana_grpid=$4
 	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
 
 	mkdir -p "${cfs_path}"
 	echo 1 > "${cfs_path}/attr_allow_any_host"
-	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
+	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}" "${ana_grpid}"
 }
 
 _remove_nvmet_ns() {
@@ -128,6 +194,23 @@ _find_nvme_loop_dev() {
 		transport="$(cat "/sys/class/nvme/${dev}/transport")"
 		if [[ "$transport" == "loop" ]]; then
 			echo "$dev"
+			break
+		fi
+	done
+}
+
+_find_nvme_ns() {
+	local uuid=$1
+	local dev
+	local hidden
+
+	for dev in /sys/block/nvme*; do
+		hidden="$(cat "${dev}/hidden")"
+		[[ "$hidden" -eq 1 ]] && continue
+		u="$(cat "${dev}/uuid")"
+		if [[ "$uuid" = "$u" ]] ; then
+			echo "$(basename "${dev}")"
+			break
 		fi
 	done
 }
-- 
2.12.3

[PATCH 1/2] blktests: enable ANA support

[Chaitanya Kulkarni]

I was able to run this test and others for NVMe category.

Looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>


From: Hannes Reinecke <hare@suse.de>
Sent: Thursday, August 2, 2018 3:34 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Jens Axboe; Sagi Grimberg; linux-nvme at lists.infradead.org; Chaitanya Kulkarni; Hannes Reinecke; Hannes Reinecke
Subject: [PATCH 1/2] blktests: enable ANA support
? 
 
Update functions to support ANA.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
?tests/nvme/rc | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
?1 file changed, 86 insertions(+), 3 deletions(-)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index fb5dbdf..ecbd9b9 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -27,7 +27,7 @@ group_device_requires() {
???????? _test_dev_is_nvme
?}
?
-NVMET_CFS="/sys/kernel/config/nvmet/"
+NVMET_CFS="/sys/kernel/config/nvmet"
?
?_test_dev_is_nvme() {
???????? if ! readlink -f "$TEST_DEV_SYSFS/device" | grep -q nvme; then
@@ -49,6 +49,7 @@ _create_nvmet_port() {
?
???????? mkdir "${NVMET_CFS}/ports/${port}"
???????? echo "${trtype}" > "${NVMET_CFS}/ports/${port}/addr_trtype"
+?????? echo "${port}" > "${NVMET_CFS}/ports/${port}/addr_traddr"
?
???????? echo "${port}"
?}
@@ -58,6 +59,62 @@ _remove_nvmet_port() {
???????? rmdir "${NVMET_CFS}/ports/${port}"
?}
?
+_create_nvmet_anagroup() {
+?????? local port="$1"
+?????? local port_cfs="${NVMET_CFS}/ports/${port}"
+?????? local grpid
+
+?????? for ((grpid = 1; ; grpid++)); do
+?????????????? if [[ ! -e "${port_cfs}/ana_groups/${grpid}" ]]; then
+?????????????????????? break
+?????????????? fi
+?????? done
+
+?????? mkdir "${port_cfs}/ana_groups/${grpid}"
+
+?????? echo "${grpid}"
+}
+
+_remove_nvmet_anagroup() {
+?????? local port="$1"
+?????? local grpid="$2"
+?????? local ana_cfs="${NVMET_CFS}/ports/${port}/ana_groups/${grpid}"
+
+?????? rmdir "${ana_cfs}"
+}
+
+_set_nvmet_anagroup_state() {
+?????? local port="$1"
+?????? local grpid="$2"
+?????? local state="$3"
+?????? local ana_cfs="${NVMET_CFS}/ports/${port}/ana_groups/${grpid}"
+
+?????? echo "${state}" > "${ana_cfs}/ana_state"
+}
+
+_display_ana_state() {
+?????? local nvmet_subsystem="$1"
+?????? local grpid state cntlid
+
+?????? for nvme in /sys/class/nvme/* ; do
+?????????????? [ -f ${nvme}/subsysnqn ] || continue
+?????????????? subsys="$(cat "${nvme}/subsysnqn")"
+?????????????? if [ "${subsys}" != "${nvmet_subsystem}" ] ; then
+?????????????????????? continue
+?????????????? fi
+?????????????? cntlid="$(cat "${nvme}/cntlid")"
+?????????????? for c in ${nvme}/nvme* ; do
+?????????????????????? if [ ! -d ${c} ] ; then
+?????????????????????????????? echo "${cntlid}: ANA disabled"
+?????????????????????????????? continue
+?????????????????????? fi
+?????????????????????? grpid="$(cat "${c}/ana_grpid")"
+?????????????????????? state="$(cat "${c}/ana_state")"
+?????????????????????? echo "${cntlid}: grpid ${grpid} state ${state}"
+?????????????? done
+?????? done
+}
+
?_create_nvmet_ns() {
???????? local nvmet_subsystem="$1"
???????? local nsid="$2"
@@ -65,14 +122,22 @@ _create_nvmet_ns() {
???????? local uuid="00000000-0000-0000-0000-000000000000"
???????? local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
???????? local ns_path="${subsys_path}/namespaces/${nsid}"
+?????? local ana_grpid
?
-?????? if [[ $# -eq 4 ]]; then
+?????? if [[ $# -ge 4 ]]; then
???????????????? uuid="$4"
???????? fi
?
+?????? if [[ $# -eq 5 ]]; then
+?????????????? ana_grpid="$5"
+?????? fi
+
???????? mkdir "${ns_path}"
???????? printf "%s" "${blkdev}" > "${ns_path}/device_path"
???????? printf "%s" "${uuid}" > "${ns_path}/device_uuid"
+?????? if [ -n "${ana_grpid}" ] ; then
+?????????????? printf "%s" "${ana_grpid}" > "${ns_path}/ana_grpid"
+?????? fi
???????? printf 1 > "${ns_path}/enable"
?}
?
@@ -80,11 +145,12 @@ _create_nvmet_subsystem() {
???????? local nvmet_subsystem="$1"
???????? local blkdev="$2"
???????? local uuid=$3
+?????? local ana_grpid=$4
???????? local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
?
???????? mkdir -p "${cfs_path}"
???????? echo 1 > "${cfs_path}/attr_allow_any_host"
-?????? _create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
+?????? _create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}" "${ana_grpid}"
?}
?
?_remove_nvmet_ns() {
@@ -128,6 +194,23 @@ _find_nvme_loop_dev() {
???????????????? transport="$(cat "/sys/class/nvme/${dev}/transport")"
???????????????? if [[ "$transport" == "loop" ]]; then
???????????????????????? echo "$dev"
+?????????????????????? break
+?????????????? fi
+?????? done
+}
+
+_find_nvme_ns() {
+?????? local uuid=$1
+?????? local dev
+?????? local hidden
+
+?????? for dev in /sys/block/nvme*; do
+?????????????? hidden="$(cat "${dev}/hidden")"
+?????????????? [[ "$hidden" -eq 1 ]] && continue
+?????????????? u="$(cat "${dev}/uuid")"
+?????????????? if [[ "$uuid" = "$u" ]] ; then
+?????????????????????? echo "$(basename "${dev}")"
+?????????????????????? break
???????????????? fi
???????? done
?}
-- 
2.12.3

    

[PATCH 2/2] blktests: add test for ANA state transition

[Hannes Reinecke]

Add a simple test for ANA state transition handling.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 tests/nvme/018     | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/018.out |  18 +++++++
 2 files changed, 171 insertions(+)
 create mode 100755 tests/nvme/018
 create mode 100644 tests/nvme/018.out

diff --git a/tests/nvme/018 b/tests/nvme/018
new file mode 100755
index 0000000..b7f9e00
--- /dev/null
+++ b/tests/nvme/018
@@ -0,0 +1,153 @@
+#!/bin/bash
+#
+# Regression test for ANA base support
+#
+# Copyright (C) 2018 Hannes Reinecke
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. tests/nvme/rc
+
+DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
+QUICK=1
+
+switch_nvmet_anagroup() {
+	local port1="$1"
+	local port2="$2"
+	local mode="$3"
+
+	echo "ANA state ${mode}"
+
+	if [ "${mode}" = "change" ] ; then
+		_set_nvmet_anagroup_state "${port1}" "1" "change"
+		_set_nvmet_anagroup_state "${port1}" "2" "change"
+		_set_nvmet_anagroup_state "${port2}" "1" "change"
+		_set_nvmet_anagroup_state "${port2}" "2" "change"
+	elif [ "${mode}" = "failover" ] ; then
+		_set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
+		_set_nvmet_anagroup_state "${port1}" "2" "optimized"
+		_set_nvmet_anagroup_state "${port2}" "1" "optimized"
+		_set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
+	else
+		_set_nvmet_anagroup_state "${port1}" "1" "optimized"
+		_set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
+		_set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
+		_set_nvmet_anagroup_state "${port2}" "2" "optimized"
+	fi
+}
+
+requires() {
+	_have_program nvme && _have_module nvme-loop && _have_module loop && \
+		_have_configfs && _have_fio
+}
+
+test() {
+	local mount_dir="/mnt/blktests"
+	local subsys="blktests-subsystem-1"
+	local port1
+	local port2
+	local loop_dev1
+	local loop_dev2
+	local nvmedev
+
+	echo "Running ${TEST_NAME}"
+
+	modprobe nvmet
+	modprobe nvme-loop
+
+	mkdir -p "${mount_dir}" > /dev/null 2>&1
+
+	port1="$(_create_nvmet_port "loop")"
+	_create_nvmet_anagroup "${port1}" > /dev/null
+
+	port2="$(_create_nvmet_port "loop")"
+	ag="$(_create_nvmet_anagroup "${port2}")"
+
+	truncate -s 1G "$TMPDIR/img1"
+
+	loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
+
+	_create_nvmet_subsystem "${subsys}" "${loop_dev1}" \
+		"91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
+
+	truncate -s 1G "$TMPDIR/img2"
+
+	loop_dev2="$(losetup -f --show "$TMPDIR/img2")"
+
+	_create_nvmet_ns "${subsys}" "2" "${loop_dev2}" \
+		"9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
+
+	_add_nvmet_subsys_to_port "${port1}" "${subsys}"
+	_add_nvmet_subsys_to_port "${port2}" "${subsys}"
+
+	switch_nvmet_anagroup "${port1}" "${port2}" failback
+
+	nvme connect -t loop -a "${port1}" -n "${subsys}"
+	nvme connect -t loop -a "${port2}" -n "${subsys}"
+
+	sleep 1
+
+	_display_ana_state "${subsys}"
+
+	nvmedev="$(_find_nvme_ns 91fdba0d-f87b-4c25-b80f-db7be1418b9e)"
+
+	mkfs.xfs -f /dev/"${nvmedev}" > /dev/null 2>&1
+
+	mount /dev/"${nvmedev}" "${mount_dir}"
+
+	_run_fio_rand_io --size=63m --directory="${mount_dir}" --runtime=20s --time_based &
+	trap "kill $!" EXIT
+
+	sleep 10
+
+	switch_nvmet_anagroup "${port1}" "${port2}" "change"
+
+	# Insert a delay to allow the AEN to be processed
+	sleep 1
+
+	_display_ana_state "${subsys}"
+
+	sleep 6
+
+	switch_nvmet_anagroup "${port1}" "${port2}" "failover"
+
+	# Insert a delay to allow the AEN to be processed
+	sleep 1
+
+	_display_ana_state "${subsys}"
+
+	wait
+	trap - EXIT
+
+	umount "${mount_dir}" > /dev/null 2>&1
+
+	nvme disconnect -n "${subsys}"
+
+	_remove_nvmet_subsystem_from_port "${port1}" "${subsys}"
+	_remove_nvmet_subsystem_from_port "${port2}" "${subsys}"
+	_remove_nvmet_ns "${subsys}" "2"
+	_remove_nvmet_subsystem "${subsys}"
+	_remove_nvmet_anagroup "${port1}" "${ag}"
+	_remove_nvmet_port "${port1}"
+	_remove_nvmet_anagroup "${port2}" "${ag}"
+	_remove_nvmet_port "${port2}"
+	losetup -d "${loop_dev2}"
+	rm "$TMPDIR/img2"
+	losetup -d "${loop_dev1}"
+	rm "$TMPDIR/img1"
+
+	modprobe -r nvme-loop
+	modprobe -r nvmet
+	echo "Test complete"
+}
diff --git a/tests/nvme/018.out b/tests/nvme/018.out
new file mode 100644
index 0000000..77d52d2
--- /dev/null
+++ b/tests/nvme/018.out
@@ -0,0 +1,18 @@
+Running nvme/018
+ANA state failback
+1: grpid 1 state optimized
+1: grpid 2 state inaccessible
+2: grpid 1 state inaccessible
+2: grpid 2 state optimized
+ANA state change
+1: grpid 1 state change
+1: grpid 2 state change
+2: grpid 1 state change
+2: grpid 2 state change
+ANA state failover
+1: grpid 1 state inaccessible
+1: grpid 2 state optimized
+2: grpid 1 state optimized
+2: grpid 2 state inaccessible
+NQN:blktests-subsystem-1 disconnected 2 controller(s)
+Test complete
-- 
2.12.3

[PATCH 2/2] blktests: add test for ANA state transition

[Chaitanya Kulkarni]

Apart from one minor comment (which can be fixed at the time of applying the patch) looks good to me.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

From: Hannes Reinecke <hare@suse.de>
Sent: Thursday, August 2, 2018 3:34 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Jens Axboe; Sagi Grimberg; linux-nvme at lists.infradead.org; Chaitanya Kulkarni; Hannes Reinecke; Hannes Reinecke
Subject: [PATCH 2/2] blktests: add test for ANA state transition
? 
 
Add a simple test for ANA state transition handling.

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
?tests/nvme/018???? | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++
?tests/nvme/018.out |? 18 +++++++
?2 files changed, 171 insertions(+)
?create mode 100755 tests/nvme/018
?create mode 100644 tests/nvme/018.out

diff --git a/tests/nvme/018 b/tests/nvme/018
new file mode 100755
index 0000000..b7f9e00
--- /dev/null
+++ b/tests/nvme/018
@@ -0,0 +1,153 @@
+#!/bin/bash
+#
+# Regression test for ANA base support
+#
+# Copyright (C) 2018 Hannes Reinecke
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.? If not, see <http://www.gnu.org/licenses/>.
+
+. tests/nvme/rc
+
+DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
+QUICK=1
+
+switch_nvmet_anagroup() {
+?????? local port1="$1"
+?????? local port2="$2"
+?????? local mode="$3"
+
+?????? echo "ANA state ${mode}"
+
+?????? if [ "${mode}" = "change" ] ; then
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "change"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "change"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "change"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "change"
+?????? elif [ "${mode}" = "failover" ] ; then
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
+?????? else
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "optimized"
+?????? fi
+}
+
+requires() {
+?????? _have_program nvme && _have_module nvme-loop && _have_module loop && \
+?????????????? _have_configfs && _have_fio
+}
+
[CK] We need to add "_have_program mkfs.xfs" and "have_module nvmet" in requires().

+test() {
+?????? local mount_dir="/mnt/blktests"
+?????? local subsys="blktests-subsystem-1"
+?????? local port1
+?????? local port2
+?????? local loop_dev1
+?????? local loop_dev2
+?????? local nvmedev
+
+?????? echo "Running ${TEST_NAME}"
+
+?????? modprobe nvmet
+?????? modprobe nvme-loop
+
+?????? mkdir -p "${mount_dir}" > /dev/null 2>&1
+
+?????? port1="$(_create_nvmet_port "loop")"
+?????? _create_nvmet_anagroup "${port1}" > /dev/null
+
+?????? port2="$(_create_nvmet_port "loop")"
+?????? ag="$(_create_nvmet_anagroup "${port2}")"
+
+?????? truncate -s 1G "$TMPDIR/img1"
+
+?????? loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
+
+?????? _create_nvmet_subsystem "${subsys}" "${loop_dev1}" \
+?????????????? "91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
+
+?????? truncate -s 1G "$TMPDIR/img2"
+
+?????? loop_dev2="$(losetup -f --show "$TMPDIR/img2")"
+
+?????? _create_nvmet_ns "${subsys}" "2" "${loop_dev2}" \
+?????????????? "9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
+
+?????? _add_nvmet_subsys_to_port "${port1}" "${subsys}"
+?????? _add_nvmet_subsys_to_port "${port2}" "${subsys}"
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" failback
+
+?????? nvme connect -t loop -a "${port1}" -n "${subsys}"
+?????? nvme connect -t loop -a "${port2}" -n "${subsys}"
+
+?????? sleep 1
+
+?????? _display_ana_state "${subsys}"
+
+?????? nvmedev="$(_find_nvme_ns 91fdba0d-f87b-4c25-b80f-db7be1418b9e)"
+
+?????? mkfs.xfs -f /dev/"${nvmedev}" > /dev/null 2>&1
+
+?????? mount /dev/"${nvmedev}" "${mount_dir}"
+
+?????? _run_fio_rand_io --size=63m --directory="${mount_dir}" --runtime=20s --time_based &
+?????? trap "kill $!" EXIT
+
+?????? sleep 10
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "change"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state "${subsys}"
+
+?????? sleep 6
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "failover"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state "${subsys}"
+
+?????? wait
+?????? trap - EXIT
+
+?????? umount "${mount_dir}" > /dev/null 2>&1
+
+?????? nvme disconnect -n "${subsys}"
+
+?????? _remove_nvmet_subsystem_from_port "${port1}" "${subsys}"
+?????? _remove_nvmet_subsystem_from_port "${port2}" "${subsys}"
+?????? _remove_nvmet_ns "${subsys}" "2"
+?????? _remove_nvmet_subsystem "${subsys}"
+?????? _remove_nvmet_anagroup "${port1}" "${ag}"
+?????? _remove_nvmet_port "${port1}"
+?????? _remove_nvmet_anagroup "${port2}" "${ag}"
+?????? _remove_nvmet_port "${port2}"
+?????? losetup -d "${loop_dev2}"
+?????? rm "$TMPDIR/img2"
+?????? losetup -d "${loop_dev1}"
+?????? rm "$TMPDIR/img1"
+
+?????? modprobe -r nvme-loop
+?????? modprobe -r nvmet
+?????? echo "Test complete"
+}
diff --git a/tests/nvme/018.out b/tests/nvme/018.out
new file mode 100644
index 0000000..77d52d2
--- /dev/null
+++ b/tests/nvme/018.out
@@ -0,0 +1,18 @@
+Running nvme/018
+ANA state failback
+1: grpid 1 state optimized
+1: grpid 2 state inaccessible
+2: grpid 1 state inaccessible
+2: grpid 2 state optimized
+ANA state change
+1: grpid 1 state change
+1: grpid 2 state change
+2: grpid 1 state change
+2: grpid 2 state change
+ANA state failover
+1: grpid 1 state inaccessible
+1: grpid 2 state optimized
+2: grpid 1 state optimized
+2: grpid 2 state inaccessible
+NQN:blktests-subsystem-1 disconnected 2 controller(s)
+Test complete
-- 
2.12.3

    

[PATCH 2/2] blktests: add test for ANA state transition

[Omar Sandoval]

On Thu, Aug 02, 2018@12:34:55PM +0200, Hannes Reinecke wrote:
> Add a simple test for ANA state transition handling.
> 
> Signed-off-by: Hannes Reinecke <hare at suse.com>
> ---
>  tests/nvme/018     | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/018.out |  18 +++++++
>  2 files changed, 171 insertions(+)
>  create mode 100755 tests/nvme/018
>  create mode 100644 tests/nvme/018.out
> 
> diff --git a/tests/nvme/018 b/tests/nvme/018
> new file mode 100755
> index 0000000..b7f9e00
> --- /dev/null
> +++ b/tests/nvme/018
> @@ -0,0 +1,153 @@
> +#!/bin/bash
> +#
> +# Regression test for ANA base support
> +#
> +# Copyright (C) 2018 Hannes Reinecke
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
> +QUICK=1
> +
> +switch_nvmet_anagroup() {
> +	local port1="$1"
> +	local port2="$2"
> +	local mode="$3"
> +
> +	echo "ANA state ${mode}"
> +
> +	if [ "${mode}" = "change" ] ; then
> +		_set_nvmet_anagroup_state "${port1}" "1" "change"
> +		_set_nvmet_anagroup_state "${port1}" "2" "change"
> +		_set_nvmet_anagroup_state "${port2}" "1" "change"
> +		_set_nvmet_anagroup_state "${port2}" "2" "change"
> +	elif [ "${mode}" = "failover" ] ; then
> +		_set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
> +		_set_nvmet_anagroup_state "${port1}" "2" "optimized"
> +		_set_nvmet_anagroup_state "${port2}" "1" "optimized"
> +		_set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
> +	else
> +		_set_nvmet_anagroup_state "${port1}" "1" "optimized"
> +		_set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
> +		_set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
> +		_set_nvmet_anagroup_state "${port2}" "2" "optimized"
> +	fi
> +}
> +
> +requires() {
> +	_have_program nvme && _have_module nvme-loop && _have_module loop && \
> +		_have_configfs && _have_fio

Sorry for the delay. This needs _have_program mkfs.xfs. Is there any way
to detect if the kernel supports ANA? This fails miserably on 4.18,
obviously.

Thanks!

[PATCH 2/2] blktests: add test for ANA state transition

[Chaitanya Kulkarni]

One way to avoid this is if a testcase is validating optional commands or controller feature then
it should execute id-ctrl or id-ns, based on the output check for the appropriate bits
before proceeding.


On 8/14/18, 2:56 PM, "Omar Sandoval" <osandov@osandov.com> wrote:

    On Thu, Aug 02, 2018@12:34:55PM +0200, Hannes Reinecke wrote:
    > Add a simple test for ANA state transition handling.
    > 
    > Signed-off-by: Hannes Reinecke <hare at suse.com>
    > ---
    >  tests/nvme/018     | 153 +++++++++++++++++++++++++++++++++++++++++++++++++++++
    >  tests/nvme/018.out |  18 +++++++
    >  2 files changed, 171 insertions(+)
    >  create mode 100755 tests/nvme/018
    >  create mode 100644 tests/nvme/018.out
    > 
    > diff --git a/tests/nvme/018 b/tests/nvme/018
    > new file mode 100755
    > index 0000000..b7f9e00
    > --- /dev/null
    > +++ b/tests/nvme/018
    > @@ -0,0 +1,153 @@
    > +#!/bin/bash
    > +#
    > +# Regression test for ANA base support
    > +#
    > +# Copyright (C) 2018 Hannes Reinecke
    > +#
    > +# This program is free software: you can redistribute it and/or modify
    > +# it under the terms of the GNU General Public License as published by
    > +# the Free Software Foundation, either version 3 of the License, or
    > +# (at your option) any later version.
    > +#
    > +# This program is distributed in the hope that it will be useful,
    > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
    > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    > +# GNU General Public License for more details.
    > +#
    > +# You should have received a copy of the GNU General Public License
    > +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
    > +
    > +. tests/nvme/rc
    > +
    > +DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
    > +QUICK=1
    > +
    > +switch_nvmet_anagroup() {
    > +	local port1="$1"
    > +	local port2="$2"
    > +	local mode="$3"
    > +
    > +	echo "ANA state ${mode}"
    > +
    > +	if [ "${mode}" = "change" ] ; then
    > +		_set_nvmet_anagroup_state "${port1}" "1" "change"
    > +		_set_nvmet_anagroup_state "${port1}" "2" "change"
    > +		_set_nvmet_anagroup_state "${port2}" "1" "change"
    > +		_set_nvmet_anagroup_state "${port2}" "2" "change"
    > +	elif [ "${mode}" = "failover" ] ; then
    > +		_set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
    > +		_set_nvmet_anagroup_state "${port1}" "2" "optimized"
    > +		_set_nvmet_anagroup_state "${port2}" "1" "optimized"
    > +		_set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
    > +	else
    > +		_set_nvmet_anagroup_state "${port1}" "1" "optimized"
    > +		_set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
    > +		_set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
    > +		_set_nvmet_anagroup_state "${port2}" "2" "optimized"
    > +	fi
    > +}
    > +
    > +requires() {
    > +	_have_program nvme && _have_module nvme-loop && _have_module loop && \
    > +		_have_configfs && _have_fio
    
    Sorry for the delay. This needs _have_program mkfs.xfs. Is there any way
    to detect if the kernel supports ANA? This fails miserably on 4.18,
    obviously.
    
    Thanks!
    

[PATCH 0/2] blktests: test ANA base support

[Hannes Reinecke]

Hi all,

here's a small patchset for testing ANA base support.
The test itself requires the ANA patchset from Christoph, plus
the fixes I've sent to the mailing list earlier.

Hannes Reinecke (2):
  blktests: enable ANA support
  blktests: add test for ANA state transition

 tests/nvme/014     | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/014.out |  17 ++++++
 tests/nvme/rc      |  49 ++++++++++++++++-
 3 files changed, 221 insertions(+), 3 deletions(-)
 create mode 100755 tests/nvme/014
 create mode 100644 tests/nvme/014.out

-- 
2.12.3

[PATCH 2/2] blktests: add test for ANA state transition

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 tests/nvme/014     | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/014.out |  17 ++++++
 2 files changed, 175 insertions(+)
 create mode 100755 tests/nvme/014
 create mode 100644 tests/nvme/014.out

diff --git a/tests/nvme/014 b/tests/nvme/014
new file mode 100755
index 0000000..4b57229
--- /dev/null
+++ b/tests/nvme/014
@@ -0,0 +1,158 @@
+#!/bin/bash
+#
+# Regression test for ANA base support
+#
+# Copyright (C) 2018 Hannes Reinecke
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. tests/nvme/rc
+
+DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
+QUICK=1
+
+switch_nvmet_anagroup() {
+	local port1="$1"
+	local port2="$2"
+	local mode="$3"
+
+	echo "ANA state ${mode}"
+
+	if [ "${mode}" = "change" ] ; then
+		_set_nvmet_anagroup_state "${port1}" "1" "change"
+		_set_nvmet_anagroup_state "${port1}" "2" "change"
+		_set_nvmet_anagroup_state "${port2}" "1" "change"
+		_set_nvmet_anagroup_state "${port2}" "2" "change"
+	elif [ "${mode}" = "failover" ] ; then
+		_set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
+		_set_nvmet_anagroup_state "${port1}" "2" "optimized"
+		_set_nvmet_anagroup_state "${port2}" "1" "optimized"
+		_set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
+	else
+		_set_nvmet_anagroup_state "${port1}" "1" "optimized"
+		_set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
+		_set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
+		_set_nvmet_anagroup_state "${port2}" "2" "optimized"
+	fi
+}
+
+_display_ana_state() {
+	local grpid state
+	for nvme in /sys/class/nvme/* ; do
+		for c in ${nvme}/nvme* ; do
+			if [ ! -d ${c} ] ; then
+				echo "${nvme##*/}: ANA disabled"
+				continue
+			fi
+			grpid="$(cat "${c}/ana_grpid")"
+			state="$(cat "${c}/ana_state")"
+			echo "${c##*/}: grpid ${grpid} state ${state}"
+		done
+	done
+}
+
+_switch_ana_states() {
+	local port1=$1
+	local port2=$2
+
+}
+
+requires() {
+	_have_program nvme && _have_module nvme-loop && _have_module loop && \
+		_have_configfs && _have_fio
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	modprobe nvmet
+	modprobe nvme-loop
+
+	local port1
+	port1="$(_create_nvmet_port "loop")"
+	ag1="$(_create_nvmet_anagroup "${port1}")"
+
+	local port2
+	port2="$(_create_nvmet_port "loop")"
+	ag2="$(_create_nvmet_anagroup "${port2}")"
+
+	truncate -s 1G "$TMPDIR/img1"
+
+	local loop_dev1
+	loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
+
+	_create_nvmet_subsystem "blktests-subsystem-1" "${loop_dev1}" \
+		"91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
+
+	truncate -s 1G "$TMPDIR/img2"
+
+	local loop_dev2
+	loop_dev2="$(losetup -f --show "$TMPDIR/img2")"
+
+	_create_nvmet_ns "blktests-subsystem-1" "2" "${loop_dev2}" \
+		"9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
+
+	_add_nvmet_subsys_to_port "${port1}" "blktests-subsystem-1"
+	_add_nvmet_subsys_to_port "${port2}" "blktests-subsystem-1"
+
+	switch_nvmet_anagroup "${port1}" "${port2}" failback
+
+	nvme connect -t loop -a "${port1}" -n blktests-subsystem-1
+	nvme connect -t loop -a "${port2}" -n blktests-subsystem-1
+
+	_display_ana_state
+
+	_run_fio_rand_io --size=256m --filename="/dev/nvme0n1" &
+	trap "kill $!" EXIT
+
+	sleep 10
+
+	switch_nvmet_anagroup "${port1}" "${port2}" "change"
+
+	# Insert a delay to allow the AEN to be processed
+	sleep 1
+
+	_display_ana_state
+
+	sleep 6
+
+	switch_nvmet_anagroup "${port1}" "${port2}" "failover"
+
+	# Insert a delay to allow the AEN to be processed
+	sleep 1
+
+	_display_ana_state
+
+	wait
+	trap - EXIT
+
+	nvme disconnect -n blktests-subsystem-1
+
+	_remove_nvmet_subsystem_from_port "${port1}" "blktests-subsystem-1"
+	_remove_nvmet_subsystem_from_port "${port2}" "blktests-subsystem-1"
+	_remove_nvmet_ns "blktests-subsystem-1" "2"
+	_remove_nvmet_subsystem "blktests-subsystem-1"
+	_remove_nvmet_anagroup "${port1}" "2"
+	_remove_nvmet_port "${port1}"
+	_remove_nvmet_anagroup "${port2}" "2"
+	_remove_nvmet_port "${port2}"
+	losetup -d "$loop_dev2"
+	rm "$TMPDIR/img2"
+	losetup -d "$loop_dev1"
+	rm "$TMPDIR/img1"
+
+	modprobe -r nvme-loop
+	modprobe -r nvmet
+	echo "Test complete"
+}
diff --git a/tests/nvme/014.out b/tests/nvme/014.out
new file mode 100644
index 0000000..0a6ed35
--- /dev/null
+++ b/tests/nvme/014.out
@@ -0,0 +1,17 @@
+Running nvme/016
+nvme0c1n1: grpid 1 state optimized
+nvme0c1n2: grpid 2 state inaccessible
+nvme0c2n1: grpid 1 state inaccessible
+nvme0c2n2: grpid 2 state optimized
+ANA state transition
+nvme0c1n1: grpid 1 state change
+nvme0c1n2: grpid 2 state change
+nvme0c2n1: grpid 1 state change
+nvme0c2n2: grpid 2 state change
+ANA state failover
+nvme0c1n1: grpid 1 state inaccessible
+nvme0c1n2: grpid 2 state optimized
+nvme0c2n1: grpid 1 state optimized
+nvme0c2n2: grpid 2 state inaccessible
+NQN:blktests-subsystem-1 disconnected 2 controller(s)
+Test complete
-- 
2.12.3

[PATCH 2/2] blktests: add test for ANA state transition

[Johannes Thumshirn]

On Tue, Jul 17, 2018@03:31:18PM +0200, Hannes Reinecke wrote:
> +requires() {
> +	_have_program nvme && _have_module nvme-loop && _have_module loop && \
> +		_have_configfs && _have_fio
> +}

this needs '_have_module_param nvme-core multipath' as well.


-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 2/2] blktests: add test for ANA state transition

[Chaitanya Kulkarni]

From: linux-block-owner@vger.kernel.org <linux-block-owner@vger.kernel.org> on behalf of Hannes Reinecke <hare@suse.de>
Sent: Tuesday, July 17, 2018 6:31 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org; Hannes Reinecke; Hannes Reinecke
Subject: [PATCH 2/2] blktests: add test for ANA state transition
? 
 
Signed-off-by: Hannes Reinecke <hare at suse.com>
---
?tests/nvme/014???? | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
?tests/nvme/014.out |? 17 ++++++
?2 files changed, 175 insertions(+)
?create mode 100755 tests/nvme/014
?create mode 100644 tests/nvme/014.out

diff --git a/tests/nvme/014 b/tests/nvme/014
new file mode 100755
index 0000000..4b57229
--- /dev/null
+++ b/tests/nvme/014
@@ -0,0 +1,158 @@
+#!/bin/bash
+#
+# Regression test for ANA base support
+#
+# Copyright (C) 2018 Hannes Reinecke
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.? If not, see <http://www.gnu.org/licenses/>.


Licenses - GNU Project - Free Software Foundation
www.gnu.org
Published software should be free software.To make it free software, you need to release it under a free software license. We normally use the GNU General Public License (GNU GPL), specifying version 3 or any later version, but occasionally we use other free software licenses.

+
+. tests/nvme/rc
+
+DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
+QUICK=1
+
+switch_nvmet_anagroup() {
+?????? local port1="$1"
+?????? local port2="$2"
+?????? local mode="$3"
+
+?????? echo "ANA state ${mode}"
+
+?????? if [ "${mode}" = "change" ] ; then
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "change"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "change"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "change"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "change"
+?????? elif [ "${mode}" = "failover" ] ; then
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
+?????? else
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "optimized"
+?????? fi
+}
+
+_display_ana_state() {
+?????? local grpid state
[CK] Newliine here ?
+?????? for nvme in /sys/class/nvme/* ; do
+?????????????? for c in ${nvme}/nvme* ; do
+?????????????????????? if [ ! -d ${c} ] ; then
+?????????????????????????????? echo "${nvme##*/}: ANA disabled"
+?????????????????????????????? continue
+?????????????????????? fi
+?????????????????????? grpid="$(cat "${c}/ana_grpid")"
+?????????????????????? state="$(cat "${c}/ana_state")"
+?????????????????????? echo "${c##*/}: grpid ${grpid} state ${state}"
+?????????????? done
+?????? done
+}

I think we need to move above functions to the  ${BLKTESTS_HOME}/tests/nvme/rc.
+
+_switch_ana_states() {
+?????? local port1=$1
+?????? local port2=$2
+
[CK] Please remove the extra line.
+}
[CK] I was not able to find a caller for above function, I'm I missing something ?
+
+requires() {
+?????? _have_program nvme && _have_module nvme-loop && _have_module loop && \
+?????????????? _have_configfs && _have_fio
[CK] Missing nvmet module from the above list.
+}

Can we split following test function into small routines, it will be easier to review and
maintain?

+
+test() {
+?????? echo "Running ${TEST_NAME}"
+
+?????? modprobe nvmet
+?????? modprobe nvme-loop
+
+?????? local port1
+?????? port1="$(_create_nvmet_port "loop")"
[CK] Can we initialize variables at the time of declaration or after declaration of all the
variables ?
+?????? ag1="$(_create_nvmet_anagroup "${port1}")"
[CK] Not sure if we need ag1 variable.

+
+?????? local port2
[CK] Can we plese declare all the variable at the top please see tests/nvme/006-013
to maintain uniform style ?
+?????? port2="$(_create_nvmet_port "loop")"
+?????? ag2="$(_create_nvmet_anagroup "${port2}")"
+
+?????? truncate -s 1G "$TMPDIR/img1"
+
+?????? local loop_dev1
+?????? loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
+
+?????? _create_nvmet_subsystem "blktests-subsystem-1" "${loop_dev1}" \
+?????????????? "91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
+
+?????? truncate -s 1G "$TMPDIR/img2"
+
[CK] Do we really need img2 ? is it possible to use the img1 as we are not doing data verification ?
+?????? local loop_dev2
+?????? loop_dev2="$(losetup -f --show "$TMPDIR/img2")"
+
+?????? _create_nvmet_ns "blktests-subsystem-1" "2" "${loop_dev2}" \
+?????????????? "9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
+
+?????? _add_nvmet_subsys_to_port "${port1}" "blktests-subsystem-1"
+?????? _add_nvmet_subsys_to_port "${port2}" "blktests-subsystem-1"
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" failback
+
+?????? nvme connect -t loop -a "${port1}" -n blktests-subsystem-1
+?????? nvme connect -t loop -a "${port2}" -n blktests-subsystem-1
+
+?????? _display_ana_state
+
+?????? _run_fio_rand_io --size=256m --filename="/dev/nvme0n1" &
[CK] Hardcoded value for ns, on my machine I have PCIe ns with "/dev/nvme0n1",
can we please make sure this is the right block device for the namespace that we just
configured ?
+?????? trap "kill $!" EXIT
+
+?????? sleep 10
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "change"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state
+
+?????? sleep 6
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "failover"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state
+
+?????? wait
+?????? trap - EXIT
+
+?????? nvme disconnect -n blktests-subsystem-1
+
+?????? _remove_nvmet_subsystem_from_port "${port1}" "blktests-subsystem-1"
+?????? _remove_nvmet_subsystem_from_port "${port2}" "blktests-subsystem-1"
+?????? _remove_nvmet_ns "blktests-subsystem-1" "2"
+?????? _remove_nvmet_subsystem "blktests-subsystem-1"
+?????? _remove_nvmet_anagroup "${port1}" "2"
+?????? _remove_nvmet_port "${port1}"
+?????? _remove_nvmet_anagroup "${port2}" "2"
[CK] Plase make the groupid local varible instead of hard coding "2".
+?????? _remove_nvmet_port "${port2}"
+?????? losetup -d "$loop_dev2"
+?????? rm "$TMPDIR/img2"
+?????? losetup -d "$loop_dev1"
+?????? rm "$TMPDIR/img1"
+
+?????? modprobe -r nvme-loop
+?????? modprobe -r nvmet
+?????? echo "Test complete"
+}
diff --git a/tests/nvme/014.out b/tests/nvme/014.out
new file mode 100644
index 0000000..0a6ed35
--- /dev/null
+++ b/tests/nvme/014.out
@@ -0,0 +1,17 @@
+Running nvme/016
+nvme0c1n1: grpid 1 state optimized
+nvme0c1n2: grpid 2 state inaccessible
+nvme0c2n1: grpid 1 state inaccessible
+nvme0c2n2: grpid 2 state optimized
+ANA state transition
+nvme0c1n1: grpid 1 state change
+nvme0c1n2: grpid 2 state change
+nvme0c2n1: grpid 1 state change
+nvme0c2n2: grpid 2 state change
+ANA state failover
+nvme0c1n1: grpid 1 state inaccessible
+nvme0c1n2: grpid 2 state optimized
+nvme0c2n1: grpid 1 state optimized
+nvme0c2n2: grpid 2 state inaccessible
+NQN:blktests-subsystem-1 disconnected 2 controller(s)
+Test complete
-- 
2.12.3

    

[PATCH 2/2] blktests: add test for ANA state transition

[Hannes Reinecke]

On 07/21/2018 11:29 PM, Chaitanya Kulkarni wrote:
> From: linux-block-owner at vger.kernel.org <linux-block-owner at vger.kernel.org> on behalf of Hannes Reinecke <hare at suse.de>
> Sent: Tuesday, July 17, 2018 6:31 AM
> To: Omar Sandoval
> Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org; Hannes Reinecke; Hannes Reinecke
> Subject: [PATCH 2/2] blktests: add test for ANA state transition
>    
>   
> Signed-off-by: Hannes Reinecke <hare at suse.com>
> ---
>  ?tests/nvme/014???? | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ?tests/nvme/014.out |? 17 ++++++
>  ?2 files changed, 175 insertions(+)
>  ?create mode 100755 tests/nvme/014
>  ?create mode 100644 tests/nvme/014.out
> 
> diff --git a/tests/nvme/014 b/tests/nvme/014
> new file mode 100755
> index 0000000..4b57229
> --- /dev/null
> +++ b/tests/nvme/014
> @@ -0,0 +1,158 @@
> +#!/bin/bash
> +#
> +# Regression test for ANA base support
> +#
> +# Copyright (C) 2018 Hannes Reinecke
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.? If not, see <http://www.gnu.org/licenses/>.
> 
> 
> Licenses - GNU Project - Free Software Foundation
> www.gnu.org
> Published software should be free software.To make it free software, you need to release it under
> a free software license. We normally use the GNU General Public License (GNU GPL), specifying version 3
> or any later version, but occasionally we use other free software licenses.
> 
I don't mind, I just copied it over from testcase 10...

> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
> +QUICK=1
> +
> +switch_nvmet_anagroup() {
> +?????? local port1="$1"
> +?????? local port2="$2"
> +?????? local mode="$3"
> +
> +?????? echo "ANA state ${mode}"
> +
> +?????? if [ "${mode}" = "change" ] ; then
> +?????????????? _set_nvmet_anagroup_state "${port1}" "1" "change"
> +?????????????? _set_nvmet_anagroup_state "${port1}" "2" "change"
> +?????????????? _set_nvmet_anagroup_state "${port2}" "1" "change"
> +?????????????? _set_nvmet_anagroup_state "${port2}" "2" "change"
> +?????? elif [ "${mode}" = "failover" ] ; then
> +?????????????? _set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
> +?????????????? _set_nvmet_anagroup_state "${port1}" "2" "optimized"
> +?????????????? _set_nvmet_anagroup_state "${port2}" "1" "optimized"
> +?????????????? _set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
> +?????? else
> +?????????????? _set_nvmet_anagroup_state "${port1}" "1" "optimized"
> +?????????????? _set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
> +?????????????? _set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
> +?????????????? _set_nvmet_anagroup_state "${port2}" "2" "optimized"
> +?????? fi
> +}
> +
> +_display_ana_state() {
> +?????? local grpid state
> [CK] Newliine here ?

Okay

> +?????? for nvme in /sys/class/nvme/* ; do
> +?????????????? for c in ${nvme}/nvme* ; do
> +?????????????????????? if [ ! -d ${c} ] ; then
> +?????????????????????????????? echo "${nvme##*/}: ANA disabled"
> +?????????????????????????????? continue
> +?????????????????????? fi
> +?????????????????????? grpid="$(cat "${c}/ana_grpid")"
> +?????????????????????? state="$(cat "${c}/ana_state")"
> +?????????????????????? echo "${c##*/}: grpid ${grpid} state ${state}"
> +?????????????? done
> +?????? done
> +}
> 
> I think we need to move above functions to the  ${BLKTESTS_HOME}/tests/nvme/rc.

Okay, can do.

> +
> +_switch_ana_states() {
> +?????? local port1=$1
> +?????? local port2=$2
> +
> [CK] Please remove the extra line.
> +}
> [CK] I was not able to find a caller for above function, I'm I missing something ?
> +
Yeah, that's an older function which got left over during refactoring.

> +requires() {
> +?????? _have_program nvme && _have_module nvme-loop && _have_module loop && \
> +?????????????? _have_configfs && _have_fio
> [CK] Missing nvmet module from the above list.
> +}
> 
> Can we split following test function into small routines, it will be easier to review and
> maintain?
> 
> +
> +test() {
> +?????? echo "Running ${TEST_NAME}"
> +
> +?????? modprobe nvmet
> +?????? modprobe nvme-loop
> +
> +?????? local port1
> +?????? port1="$(_create_nvmet_port "loop")"
> [CK] Can we initialize variables at the time of declaration or after declaration of all the
> variables ?

Sure. But we should do it consistently; the older tests also do not 
follow these rules...

> +?????? ag1="$(_create_nvmet_anagroup "${port1}")"
> [CK] Not sure if we need ag1 variable.
> 
Yeah, it's more for symmetry than anything else.

> +
> +?????? local port2
> [CK] Can we plese declare all the variable at the top please see tests/nvme/006-013
> to maintain uniform style ?

Sure. But see above; not all tests follow the same scheme.

> +?????? port2="$(_create_nvmet_port "loop")"
> +?????? ag2="$(_create_nvmet_anagroup "${port2}")"
> +
> +?????? truncate -s 1G "$TMPDIR/img1"
> +
> +?????? local loop_dev1
> +?????? loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
> +
> +?????? _create_nvmet_subsystem "blktests-subsystem-1" "${loop_dev1}" \
> +?????????????? "91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
> +
> +?????? truncate -s 1G "$TMPDIR/img2"
> +
> [CK] Do we really need img2 ? is it possible to use the img1 as we are not doing data verification ?

Yes. There's an issue with the current implementation which fails to 
signal a namespace state change for the _second_ namepsace.

(That's the one patch I've sent to the current ANA implementation from hch)

> +?????? local loop_dev2
> +?????? loop_dev2="$(losetup -f --show "$TMPDIR/img2")"
> +
> +?????? _create_nvmet_ns "blktests-subsystem-1" "2" "${loop_dev2}" \
> +?????????????? "9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
> +
> +?????? _add_nvmet_subsys_to_port "${port1}" "blktests-subsystem-1"
> +?????? _add_nvmet_subsys_to_port "${port2}" "blktests-subsystem-1"
> +
> +?????? switch_nvmet_anagroup "${port1}" "${port2}" failback
> +
> +?????? nvme connect -t loop -a "${port1}" -n blktests-subsystem-1
> +?????? nvme connect -t loop -a "${port2}" -n blktests-subsystem-1
> +
> +?????? _display_ana_state
> +
> +?????? _run_fio_rand_io --size=256m --filename="/dev/nvme0n1" &
> [CK] Hardcoded value for ns, on my machine I have PCIe ns with "/dev/nvme0n1",
> can we please make sure this is the right block device for the namespace that we just
> configured ?

Yeah, I know. Will see what I can do.

> +?????? trap "kill $!" EXIT
> +
> +?????? sleep 10
> +
> +?????? switch_nvmet_anagroup "${port1}" "${port2}" "change"
> +
> +?????? # Insert a delay to allow the AEN to be processed
> +?????? sleep 1
> +
> +?????? _display_ana_state
> +
> +?????? sleep 6
> +
> +?????? switch_nvmet_anagroup "${port1}" "${port2}" "failover"
> +
> +?????? # Insert a delay to allow the AEN to be processed
> +?????? sleep 1
> +
> +?????? _display_ana_state
> +
> +?????? wait
> +?????? trap - EXIT
> +
> +?????? nvme disconnect -n blktests-subsystem-1
> +
> +?????? _remove_nvmet_subsystem_from_port "${port1}" "blktests-subsystem-1"
> +?????? _remove_nvmet_subsystem_from_port "${port2}" "blktests-subsystem-1"
> +?????? _remove_nvmet_ns "blktests-subsystem-1" "2"
> +?????? _remove_nvmet_subsystem "blktests-subsystem-1"
> +?????? _remove_nvmet_anagroup "${port1}" "2"
> +?????? _remove_nvmet_port "${port1}"
> +?????? _remove_nvmet_anagroup "${port2}" "2"
> [CK] Plase make the groupid local varible instead of hard coding "2".

Hmm. Not easy. But I'll have a look.

Thanks for the review.

Cheers,

Hannes