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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: linux-block-owner@xxxxxxxxxxxxxxx <linux-block-owner@xxxxxxxxxxxxxxx> on behalf of Hannes Reinecke <hare@xxxxxxx>
Sent: Tuesday, July 17, 2018 6:31 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme@xxxxxxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx; Hannes Reinecke; Hannes Reinecke
Subject: [PATCH 2/2] blktests: add test for ANA state transition
  
 
Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
---
 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

    



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux