* Re: [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect
2023-06-28 12:43 [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect Shin'ichiro Kawasaki
@ 2023-06-28 13:11 ` Max Gurtovoy
2023-06-28 22:23 ` Chaitanya Kulkarni
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Max Gurtovoy @ 2023-06-28 13:11 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, linux-block, linux-nvme
Cc: Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect
2023-06-28 12:43 [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect Shin'ichiro Kawasaki
2023-06-28 13:11 ` Max Gurtovoy
@ 2023-06-28 22:23 ` Chaitanya Kulkarni
2023-06-29 0:03 ` Yi Zhang
2023-06-29 0:56 ` Shinichiro Kawasaki
3 siblings, 0 replies; 5+ messages in thread
From: Chaitanya Kulkarni @ 2023-06-28 22:23 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, Max Gurtovoy
Cc: Yi Zhang, linux-block@vger.kernel.org, Sagi Grimberg,
Chaitanya Kulkarni, linux-nvme@lists.infradead.org
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect
2023-06-28 12:43 [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect Shin'ichiro Kawasaki
2023-06-28 13:11 ` Max Gurtovoy
2023-06-28 22:23 ` Chaitanya Kulkarni
@ 2023-06-29 0:03 ` Yi Zhang
2023-06-29 0:56 ` Shinichiro Kawasaki
3 siblings, 0 replies; 5+ messages in thread
From: Yi Zhang @ 2023-06-29 0:03 UTC (permalink / raw)
To: Shin'ichiro Kawasaki
Cc: linux-block, linux-nvme, Max Gurtovoy, Sagi Grimberg,
Chaitanya Kulkarni
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
>
--
Best Regards,
Yi Zhang
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect
2023-06-28 12:43 [PATCH blktests] nvme/rc: specify hostnqn to hostid to nvme discover and connect Shin'ichiro Kawasaki
` (2 preceding siblings ...)
2023-06-29 0:03 ` Yi Zhang
@ 2023-06-29 0:56 ` Shinichiro Kawasaki
3 siblings, 0 replies; 5+ messages in thread
From: Shinichiro Kawasaki @ 2023-06-29 0:56 UTC (permalink / raw)
To: linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
Max Gurtovoy
Cc: Yi Zhang, Sagi Grimberg, Chaitanya Kulkarni
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.
^ permalink raw reply [flat|nested] 5+ messages in thread