From: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To: Daniel Wagner <dwagner@suse.de>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
Chaitanya Kulkarni <chaitanyak@nvidia.com>
Subject: Re: [PATCH blktests v2 1/3] nvme/rc: Parse optional arguments in _nvme_connect_subsys()
Date: Thu, 23 Mar 2023 10:45:02 +0000 [thread overview]
Message-ID: <20230323104501.yo7psenh26jjqry4@shindev> (raw)
In-Reply-To: <20230322101648.31514-2-dwagner@suse.de>
On Mar 22, 2023 / 11:16, Daniel Wagner wrote:
> Extend the nvme_connect_subsys() function to parse optional arguments.
> This avoids that all test have to pass in always all arguments.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
> tests/nvme/041 | 7 ++++--
> tests/nvme/042 | 10 +++++---
> tests/nvme/043 | 10 +++++---
> tests/nvme/044 | 23 +++++++++++------
> tests/nvme/045 | 6 +++--
> tests/nvme/rc | 68 +++++++++++++++++++++++++++++++++++++++++++-------
> 6 files changed, 95 insertions(+), 29 deletions(-)
>
> diff --git a/tests/nvme/041 b/tests/nvme/041
> index 8ffcf13a500a..03e2dab25918 100755
> --- a/tests/nvme/041
> +++ b/tests/nvme/041
> @@ -55,14 +55,17 @@ test() {
> # Test unauthenticated connection (should fail)
> echo "Test unauthenticated connection (should fail)"
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}"
>
> _nvme_disconnect_subsys "${subsys_name}"
>
> # Test authenticated connection
> echo "Test authenticated connection"
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" "${hostkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}"
>
> udevadm settle
>
> diff --git a/tests/nvme/042 b/tests/nvme/042
> index d581bce4a9ee..4ad726f72f5a 100755
> --- a/tests/nvme/042
> +++ b/tests/nvme/042
> @@ -58,8 +58,9 @@ test() {
> _set_nvmet_hostkey "${hostnqn}" "${hostkey}"
>
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}"
> udevadm settle
>
> _nvme_disconnect_subsys "${subsys_name}"
> @@ -75,8 +76,9 @@ test() {
> _set_nvmet_hostkey "${hostnqn}" "${hostkey}"
>
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}"
>
> udevadm settle
>
> diff --git a/tests/nvme/043 b/tests/nvme/043
> index c8ce292ba2e7..c031cecf34a5 100755
> --- a/tests/nvme/043
> +++ b/tests/nvme/043
> @@ -56,8 +56,9 @@ test() {
> _set_nvmet_hash "${hostnqn}" "${hash}"
>
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}"
>
> udevadm settle
>
> @@ -71,8 +72,9 @@ test() {
> _set_nvmet_dhgroup "${hostnqn}" "${dhgroup}"
>
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}"
>
> udevadm settle
>
> diff --git a/tests/nvme/044 b/tests/nvme/044
> index c19a9c026711..f2406ecadf7d 100755
> --- a/tests/nvme/044
> +++ b/tests/nvme/044
> @@ -66,8 +66,9 @@ test() {
> # Step 1: Connect with host authentication only
> echo "Test host authentication"
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}"
>
> udevadm settle
>
> @@ -77,8 +78,10 @@ test() {
> # and invalid ctrl authentication
> echo "Test invalid ctrl authentication (should fail)"
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}" "${hostkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}" \
> + --dhchap-ctrl-secret "${hostkey}"
>
> udevadm settle
>
> @@ -88,8 +91,10 @@ test() {
> # and valid ctrl authentication
> echo "Test valid ctrl authentication"
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}" "${ctrlkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}" \
> + --dhchap-ctrl-secret "${ctrlkey}"
>
> udevadm settle
>
> @@ -100,8 +105,10 @@ test() {
> echo "Test invalid ctrl key (should fail)"
> invkey="DHHC-1:00:Jc/My1o0qtLCWRp+sHhAVafdfaS7YQOMYhk9zSmlatobqB8C:"
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}" "${invkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}" \
> + --dhchap-ctrl-secret "${invkey}"
>
> udevadm settle
>
> diff --git a/tests/nvme/045 b/tests/nvme/045
> index a0e6e93ed3c7..612e5f168e3c 100755
> --- a/tests/nvme/045
> +++ b/tests/nvme/045
> @@ -65,8 +65,10 @@ test() {
> _set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
>
> _nvme_connect_subsys "${nvme_trtype}" "${subsys_name}" \
> - "" "" "${hostnqn}" "${hostid}" \
> - "${hostkey}" "${ctrlkey}"
> + --hostnqn "${hostnqn}" \
> + --hostid "${hostid}" \
> + --dhchap-secret "${hostkey}" \
> + --dhchap-ctrl-secret "${ctrlkey}"
>
> udevadm settle
>
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 210a82aea384..1145fed2d14c 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -316,15 +316,65 @@ _nvme_disconnect_subsys() {
> }
>
> _nvme_connect_subsys() {
> - local trtype="$1"
> - local subsysnqn="$2"
> - local traddr="${3:-$def_traddr}"
> - local host_traddr="${4:-$def_host_traddr}"
> - local trsvcid="${4:-$def_trsvcid}"
> - local hostnqn="${5:-$def_hostnqn}"
> - local hostid="${6:-$def_hostid}"
> - local hostkey="${7}"
> - local ctrlkey="${8}"
> + local positional_args=()
> + local trtype=""
> + local subsysnqn=""
> + local traddr="$def_traddr"
> + local host_traddr="$def_host_traddr"
> + local trsvcid="$def_trsvcid"
> + local hostnqn="$def_hostnqn"
> + local hostid="$def_hostid"
> + local hostkey=""
> + local ctrlkey=""
> +
> + while [[ $# -gt 0 ]]; do
> + case $1 in
> + -a|--traddr)
> + traddr="$2"
> + shift
> + shift
This patch looks good other than the type you found.
One nit: the two lines above can be single line with "shift 2". Same comment
for below and the 2nd patch.
> + ;;
> + -w|--host-traddr)
> + host_traddr="$2"
> + shift
> + shift
> + ;;
> + -s|--trsvcid)
> + trsvcid="$2"
> + shift
> + shift
> + ;;
> + -n|--hostnqn)
> + hostnqn="$2"
> + shift
> + shift
> + ;;
> + -I|--hostid)
> + hostid="$2"
> + shift
> + shift
> + ;;
> + -S|--dhchap-secret)
> + hostkey="$2"
> + shift
> + shift
> + ;;
> + -C|--dhchap-ctrl-sectret)
> + ctrlkey="$2"
> + shift
> + shift
> + ;;
> + *)
> + positional_args+=("$1")
> + shift
> + ;;
> + esac
> + done
> +
> + set -- "${positional_args[@]}"
> +
> + trtype="$1"
> + subsysnqn="$2"
>
> ARGS=(-t "${trtype}" -n "${subsysnqn}")
> if [[ "${trtype}" == "fc" ]] ; then
> --
> 2.40.0
>
--
Shin'ichiro Kawasaki
next prev parent reply other threads:[~2023-03-23 10:45 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-22 10:16 [PATCH blktests v2 0/3] Test different queue counts Daniel Wagner
2023-03-22 10:16 ` [PATCH blktests v2 1/3] nvme/rc: Parse optional arguments in _nvme_connect_subsys() Daniel Wagner
2023-03-22 11:08 ` Daniel Wagner
2023-03-23 10:45 ` Shinichiro Kawasaki [this message]
2023-03-22 10:16 ` [PATCH blktests v2 2/3] nvme/rc: Add nr queue parser arguments Daniel Wagner
2023-03-23 10:46 ` Shinichiro Kawasaki
2023-03-22 10:16 ` [PATCH blktests v2 3/3] nvme/047: Test different queue counts Daniel Wagner
2023-03-23 10:55 ` Shinichiro Kawasaki
2023-03-23 11:06 ` [PATCH blktests v2 0/3] " Shinichiro Kawasaki
2023-03-27 15:41 ` Daniel Wagner
2023-03-28 8:45 ` Shinichiro Kawasaki
2023-03-28 18:20 ` Chaitanya Kulkarni
2023-03-29 0:57 ` Shinichiro Kawasaki
2023-03-28 18:35 ` Keith Busch
2023-03-29 3:30 ` Chaitanya Kulkarni
2023-03-29 6:25 ` Daniel Wagner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230323104501.yo7psenh26jjqry4@shindev \
--to=shinichiro.kawasaki@wdc.com \
--cc=chaitanyak@nvidia.com \
--cc=dwagner@suse.de \
--cc=linux-block@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox