diff mbox series

[blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect

Message ID 20230628124343.2900339-1-shinichiro.kawasaki@wdc.com (mailing list archive)
State New, archived
Headers show
Series [blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect | expand

Commit Message

Shin'ichiro Kawasaki June 28, 2023, 12:43 p.m. UTC 
From: Max Gurtovoy <mgurtovoy@nvidia.com>

After the kernel commit ae8bd606e09b ("nvme-fabrics: prevent overriding
of existing host"), 'nvme discover' and 'nvme connect' commands fail
when pair of hostid and hostnqn is not provide. This caused failure of
many test cases in the nvme group with kernel messages "nvme_fabrics:
found same hostid XXX but different hostnqn YYY".

To avoid the failure, specify valid hostnqn and hostid to the nvme
commands always. Prepare def_hostnqn and def_hostid even when
/etc/nvme/hostnqn or /etc/nvme/hostid is not available. Using these
values, add --hostnqn and --hostid options to the nvme commands in
_nvme_discover() and _nvme_connect_subsys().

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Link: https://lore.kernel.org/linux-nvme/CAHj4cs_qUWzetD0203EKbBLNv3KF=qgTLsWLeHN3PY7UE6mzmw@mail.gmail.com/
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/nvme/rc | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

Comments

Max Gurtovoy June 28, 2023, 1:11 p.m. UTC | #1 
Hi Shin'ichiro,

On 28/06/2023 15:43, Shin'ichiro Kawasaki wrote:
> From: Max Gurtovoy <mgurtovoy@nvidia.com>

please set yourself as the author. You deserve the credit :)

> 
> After the kernel commit ae8bd606e09b ("nvme-fabrics: prevent overriding
> of existing host"), 'nvme discover' and 'nvme connect' commands fail

"... 'nvme discover' and 'nvme connect' commands will fail in case 
hostid and hostnqn don't maintain 1:1 mapping in the system. This caused 
..."

> when pair of hostid and hostnqn is not provide. This caused failure of
> many test cases in the nvme group with kernel messages "nvme_fabrics:
> found same hostid XXX but different hostnqn YYY".
> 
> To avoid the failure, specify valid hostnqn and hostid to the nvme
> commands always. Prepare def_hostnqn and def_hostid even when
> /etc/nvme/hostnqn or /etc/nvme/hostid is not available. Using these
> values, add --hostnqn and --hostid options to the nvme commands in
> _nvme_discover() and _nvme_connect_subsys().
> 
> Reported-by: Yi Zhang <yi.zhang@redhat.com>

Yi,
can we get your Tested-by here please ?

> Link: https://lore.kernel.org/linux-nvme/CAHj4cs_qUWzetD0203EKbBLNv3KF=qgTLsWLeHN3PY7UE6mzmw@mail.gmail.com/
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>   tests/nvme/rc | 29 +++++++++++++++++++++--------
>   1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 191f3e2..1c2c2fa 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -14,8 +14,23 @@ def_remote_wwnn="0x10001100aa000001"
>   def_remote_wwpn="0x20001100aa000001"
>   def_local_wwnn="0x10001100aa000002"
>   def_local_wwpn="0x20001100aa000002"
> -def_hostnqn="$(cat /etc/nvme/hostnqn 2> /dev/null)"
> -def_hostid="$(cat /etc/nvme/hostid 2> /dev/null)"
> +
> +if [ -f "/etc/nvme/hostid" ]; then
> +	def_hostid="$(cat /etc/nvme/hostid 2> /dev/null)"
> +else
> +	def_hostid="$(uuidgen)"
> +fi
> +if [ -z "$def_hostid" ] ; then
> +	def_hostid="0f01fb42-9f7f-4856-b0b3-51e60b8de349"
> +fi
> +
> +if [ -f "/etc/nvme/hostnqn" ]; then
> +	def_hostnqn="$(cat /etc/nvme/hostnqn 2> /dev/null)"
> +fi
> +if [ -z "$def_hostnqn" ] ; then
> +	def_hostnqn="nqn.2014-08.org.nvmexpress:uuid:${def_hostid}"
> +fi
> +
>   nvme_trtype=${nvme_trtype:-"loop"}
>   nvme_img_size=${nvme_img_size:-"1G"}
>   nvme_num_iter=${nvme_num_iter:-"1000"}
> @@ -442,12 +457,8 @@ _nvme_connect_subsys() {
>   	elif [[ "${trtype}" != "loop" ]]; then
>   		ARGS+=(-a "${traddr}" -s "${trsvcid}")
>   	fi
> -	if [[ "${hostnqn}" != "$def_hostnqn" ]]; then
> -		ARGS+=(--hostnqn="${hostnqn}")
> -	fi
> -	if [[ "${hostid}" != "$def_hostid" ]]; then
> -		ARGS+=(--hostid="${hostid}")
> -	fi
> +	ARGS+=(--hostnqn="${hostnqn}")
> +	ARGS+=(--hostid="${hostid}")
>   	if [[ -n "${hostkey}" ]]; then
>   		ARGS+=(--dhchap-secret="${hostkey}")
>   	fi
> @@ -483,6 +494,8 @@ _nvme_discover() {
>   	local trsvcid="${3:-$def_trsvcid}"
>   
>   	ARGS=(-t "${trtype}")
> +	ARGS+=(--hostnqn="${def_hostnqn}")
> +	ARGS+=(--hostid="${def_hostid}")
>   	if [[ "${trtype}" = "fc" ]]; then
>   		ARGS+=(-a "${traddr}" -w "${host_traddr}")
>   	elif [[ "${trtype}" != "loop" ]]; then
Chaitanya Kulkarni June 28, 2023, 10:23 p.m. UTC | #2 
Shinichiro/Max,

On 6/28/2023 5:43 AM, Shin'ichiro Kawasaki wrote:
> From: Max Gurtovoy <mgurtovoy@nvidia.com>
> 
> After the kernel commit ae8bd606e09b ("nvme-fabrics: prevent overriding
> of existing host"), 'nvme discover' and 'nvme connect' commands fail
> when pair of hostid and hostnqn is not provide. This caused failure of
> many test cases in the nvme group with kernel messages "nvme_fabrics:
> found same hostid XXX but different hostnqn YYY".
> 
> To avoid the failure, specify valid hostnqn and hostid to the nvme
> commands always. Prepare def_hostnqn and def_hostid even when
> /etc/nvme/hostnqn or /etc/nvme/hostid is not available. Using these
> values, add --hostnqn and --hostid options to the nvme commands in
> _nvme_discover() and _nvme_connect_subsys().
> 
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Link: https://lore.kernel.org/linux-nvme/CAHj4cs_qUWzetD0203EKbBLNv3KF=qgTLsWLeHN3PY7UE6mzmw@mail.gmail.com/
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Thanks for fixing this quickly.

I've tested this patch on my setup where there was no errors.

With this patch all the testecases are passing on my non-broken setup,
but please wait for Yi's Tested-by tag before applying the patch.

with that :-

Tested-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck
Yi Zhang June 29, 2023, 12:03 a.m. UTC | #3 
I've verified the fix on my environment, thanks for the fix.
Tested-by: Yi Zhang <yi.zhang@redhat.com>

On Wed, Jun 28, 2023 at 8:44 PM Shin'ichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> From: Max Gurtovoy <mgurtovoy@nvidia.com>
>
> After the kernel commit ae8bd606e09b ("nvme-fabrics: prevent overriding
> of existing host"), 'nvme discover' and 'nvme connect' commands fail
> when pair of hostid and hostnqn is not provide. This caused failure of
> many test cases in the nvme group with kernel messages "nvme_fabrics:
> found same hostid XXX but different hostnqn YYY".
>
> To avoid the failure, specify valid hostnqn and hostid to the nvme
> commands always. Prepare def_hostnqn and def_hostid even when
> /etc/nvme/hostnqn or /etc/nvme/hostid is not available. Using these
> values, add --hostnqn and --hostid options to the nvme commands in
> _nvme_discover() and _nvme_connect_subsys().
>
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Link: https://lore.kernel.org/linux-nvme/CAHj4cs_qUWzetD0203EKbBLNv3KF=qgTLsWLeHN3PY7UE6mzmw@mail.gmail.com/
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  tests/nvme/rc | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 191f3e2..1c2c2fa 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -14,8 +14,23 @@ def_remote_wwnn="0x10001100aa000001"
>  def_remote_wwpn="0x20001100aa000001"
>  def_local_wwnn="0x10001100aa000002"
>  def_local_wwpn="0x20001100aa000002"
> -def_hostnqn="$(cat /etc/nvme/hostnqn 2> /dev/null)"
> -def_hostid="$(cat /etc/nvme/hostid 2> /dev/null)"
> +
> +if [ -f "/etc/nvme/hostid" ]; then
> +       def_hostid="$(cat /etc/nvme/hostid 2> /dev/null)"
> +else
> +       def_hostid="$(uuidgen)"
> +fi
> +if [ -z "$def_hostid" ] ; then
> +       def_hostid="0f01fb42-9f7f-4856-b0b3-51e60b8de349"
> +fi
> +
> +if [ -f "/etc/nvme/hostnqn" ]; then
> +       def_hostnqn="$(cat /etc/nvme/hostnqn 2> /dev/null)"
> +fi
> +if [ -z "$def_hostnqn" ] ; then
> +       def_hostnqn="nqn.2014-08.org.nvmexpress:uuid:${def_hostid}"
> +fi
> +
>  nvme_trtype=${nvme_trtype:-"loop"}
>  nvme_img_size=${nvme_img_size:-"1G"}
>  nvme_num_iter=${nvme_num_iter:-"1000"}
> @@ -442,12 +457,8 @@ _nvme_connect_subsys() {
>         elif [[ "${trtype}" != "loop" ]]; then
>                 ARGS+=(-a "${traddr}" -s "${trsvcid}")
>         fi
> -       if [[ "${hostnqn}" != "$def_hostnqn" ]]; then
> -               ARGS+=(--hostnqn="${hostnqn}")
> -       fi
> -       if [[ "${hostid}" != "$def_hostid" ]]; then
> -               ARGS+=(--hostid="${hostid}")
> -       fi
> +       ARGS+=(--hostnqn="${hostnqn}")
> +       ARGS+=(--hostid="${hostid}")
>         if [[ -n "${hostkey}" ]]; then
>                 ARGS+=(--dhchap-secret="${hostkey}")
>         fi
> @@ -483,6 +494,8 @@ _nvme_discover() {
>         local trsvcid="${3:-$def_trsvcid}"
>
>         ARGS=(-t "${trtype}")
> +       ARGS+=(--hostnqn="${def_hostnqn}")
> +       ARGS+=(--hostid="${def_hostid}")
>         if [[ "${trtype}" = "fc" ]]; then
>                 ARGS+=(-a "${traddr}" -w "${host_traddr}")
>         elif [[ "${trtype}" != "loop" ]]; then
> --
> 2.40.1
>
Shin'ichiro Kawasaki June 29, 2023, 12:56 a.m. UTC | #4 
On Jun 28, 2023 / 21:43, Shin'ichiro Kawasaki wrote:
> From: Max Gurtovoy <mgurtovoy@nvidia.com>
> 
> After the kernel commit ae8bd606e09b ("nvme-fabrics: prevent overriding
> of existing host"), 'nvme discover' and 'nvme connect' commands fail
> when pair of hostid and hostnqn is not provide. This caused failure of
> many test cases in the nvme group with kernel messages "nvme_fabrics:
> found same hostid XXX but different hostnqn YYY".
> 
> To avoid the failure, specify valid hostnqn and hostid to the nvme
> commands always. Prepare def_hostnqn and def_hostid even when
> /etc/nvme/hostnqn or /etc/nvme/hostid is not available. Using these
> values, add --hostnqn and --hostid options to the nvme commands in
> _nvme_discover() and _nvme_connect_subsys().

Thanks for the reviews and tests. I've applied it with changes Max suggested.
diff mbox series

Patch

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 191f3e2..1c2c2fa 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -14,8 +14,23 @@  def_remote_wwnn="0x10001100aa000001"
 def_remote_wwpn="0x20001100aa000001"
 def_local_wwnn="0x10001100aa000002"
 def_local_wwpn="0x20001100aa000002"
-def_hostnqn="$(cat /etc/nvme/hostnqn 2> /dev/null)"
-def_hostid="$(cat /etc/nvme/hostid 2> /dev/null)"
+
+if [ -f "/etc/nvme/hostid" ]; then
+	def_hostid="$(cat /etc/nvme/hostid 2> /dev/null)"
+else
+	def_hostid="$(uuidgen)"
+fi
+if [ -z "$def_hostid" ] ; then
+	def_hostid="0f01fb42-9f7f-4856-b0b3-51e60b8de349"
+fi
+
+if [ -f "/etc/nvme/hostnqn" ]; then
+	def_hostnqn="$(cat /etc/nvme/hostnqn 2> /dev/null)"
+fi
+if [ -z "$def_hostnqn" ] ; then
+	def_hostnqn="nqn.2014-08.org.nvmexpress:uuid:${def_hostid}"
+fi
+
 nvme_trtype=${nvme_trtype:-"loop"}
 nvme_img_size=${nvme_img_size:-"1G"}
 nvme_num_iter=${nvme_num_iter:-"1000"}
@@ -442,12 +457,8 @@  _nvme_connect_subsys() {
 	elif [[ "${trtype}" != "loop" ]]; then
 		ARGS+=(-a "${traddr}" -s "${trsvcid}")
 	fi
-	if [[ "${hostnqn}" != "$def_hostnqn" ]]; then
-		ARGS+=(--hostnqn="${hostnqn}")
-	fi
-	if [[ "${hostid}" != "$def_hostid" ]]; then
-		ARGS+=(--hostid="${hostid}")
-	fi
+	ARGS+=(--hostnqn="${hostnqn}")
+	ARGS+=(--hostid="${hostid}")
 	if [[ -n "${hostkey}" ]]; then
 		ARGS+=(--dhchap-secret="${hostkey}")
 	fi
@@ -483,6 +494,8 @@  _nvme_discover() {
 	local trsvcid="${3:-$def_trsvcid}"
 
 	ARGS=(-t "${trtype}")
+	ARGS+=(--hostnqn="${def_hostnqn}")
+	ARGS+=(--hostid="${def_hostid}")
 	if [[ "${trtype}" = "fc" ]]; then
 		ARGS+=(-a "${traddr}" -w "${host_traddr}")
 	elif [[ "${trtype}" != "loop" ]]; then