[bug report] most of blktests nvme/ failed on the latest linux tree

[Max Gurtovoy]

On 28/06/2023 13:34, Shinichiro Kawasaki wrote:
> On Jun 28, 2023 / 12:52, Max Gurtovoy wrote:
>> Hi Yi,
>>
>> On 28/06/2023 12:21, Max Gurtovoy wrote:
>>>
>>>
>>> On 28/06/2023 11:14, Sagi Grimberg wrote:
>>>>
>>>>>>
>>>>>>>> Yi,
>>>>>>>>
>>>>>>>> Do you have hostnqn and hostid files in your /etc/nvme directory?
>>>>>>>>
>>>>>>>
>>>>>>> No, only one discovery.conf there.
>>>>>>>
>>>>>>> # ls /etc/nvme/
>>>>>>> discovery.conf
>>>>>>
>>>>>> So the hostid is generated every time if it is not passed.
>>>>>> We should probably revert the patch and add it back when
>>>>>> blktests are passing.
>>>>>
>>>>> Seems like the patch is doing exactly what it should do - fix
>>>>> wrong behavior of users that override hostid.
>>>>> Can we fix the tests instead ?
>>>>
>>>> Right, I got confused between a provided host and the default host...
>>>>
>>>> I think we need to add check that /etc/nvme/[hostnqn,hostid] exist
>>>> in the test cases.
>>>
>>> Right.
>>> And if one of the files doesn't exist, generate the value.
>>>
>>> Should it go to tests/nvme/rc ?
>>
>> Can you please try adding the bellow un-tested code to blktests and re-run:
>>
>> [root at r-arch-stor03 blktests]# git diff
>> diff --git a/tests/nvme/rc b/tests/nvme/rc
>> index 191f3e2..88e6fa1 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"}
>>
> 
> I tried the patch above, and still see the errors. def_hostnqn and def_hostid
> were not passed to 'nvme discover' and 'nvme connect'. I added some more change
> as below patch and confirmed the nvme test group failures were avoided
> (nvme_trtype=loop).
> 
> 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

Looks good.
can you please send a patch with the above ?
you can add my
Signed-off-by: Max Gurtovoy <mgurtovoy at nvidia.com>