[PATCH v2 3/7] nvme: make tests transport type agnostic

[Sagi Grimberg]

>> diff --git a/tests/nvme/002 b/tests/nvme/002
>> index 999e222705bf..8540623497c7 100755
>> --- a/tests/nvme/002
>> +++ b/tests/nvme/002
>> @@ -21,7 +21,7 @@ test() {
>>    
>>    	local iterations=1000
>>    	local port
>> -	port="$(_create_nvmet_port "loop")"
>> +	port="$(_create_nvmet_port ${nvme_trtype})"
> Is there a way to directly use nvme_trtype especially in rc ?
> if not disregard this comment.

I didn't want to do this, because a test can create multiple ports.
But maybe it could have a default value?

>> @@ -33,10 +33,10 @@ test() {
>>    		"91fdba0d-f87b-4c25-b80f-db7be1418b9e"
>>    	_add_nvmet_subsys_to_port "${port}" "blktests-subsystem-1"
>>    
>> -	_nvme_connect_subsys "loop" "blktests-subsystem-1"
>> +	_nvme_connect_subsys ${nvme_trtype} "blktests-subsystem-1"
>>    
>>    	local nvmedev
>> -	nvmedev="$(_find_nvme_loop_dev)"
>> +	nvmedev="$(_find_nvme_dev)"
>>    	cat "/sys/block/${nvmedev}n1/uuid"
>>    	cat "/sys/block/${nvmedev}n1/wwid"
> 
> Since we are touching nvmedev can we move above uuid and wwid to
> a wrapper something like _nvme_show_uuid_wwid ${nvmedev}n1 ?

Doesn't help the patch set cause, so it can be added incrementally.

> 
>>
>> @@ -36,12 +36,12 @@ test() {
>>    
>>    	loop_dev="$(losetup -f --show "$TMPDIR/img")"
>>    
>> -	port="$(_create_nvmet_port "loop")"
>> +	port="$(_create_nvmet_port ${nvme_trtype})"
>>    
>>    	for ((i = 0; i < iterations; i++)); do
>>    		_create_nvmet_subsystem "${subsys}$i" "${loop_dev}"
>>    		_add_nvmet_subsys_to_port "${port}" "${subsys}$i"
>> -		_nvme_connect_subsys "loop" "${subsys}$i"
>> +		_nvme_connect_subsys ${nvme_trtype} "${subsys}$i"
> Same here for nvme_trtype as first comment.
>>    		_nvme_disconnect_subsys "${subsys}$i" >> "${FULL}" 2>&1
>>    		_remove_nvmet_subsystem_from_port "${port}" "${subsys}$i"
>>    		_remove_nvmet_subsystem "${subsys}$i"
>> diff --git a/tests/nvme/rc b/tests/nvme/rc
>> index 6d57cf591300..191f0497416a 100644
>> --- a/tests/nvme/rc
>> +++ b/tests/nvme/rc
>> @@ -6,6 +6,9 @@
>>    
>>    . common/rc
>>    
>> +def_traddr="127.0.0.1"
>> +def_adrfam="ipv4"
>> +def_trsvcid="4420"
>>    nvme_trtype=${nvme_trtype:-"loop"}
>>    
>>    _nvme_requires() {
>> @@ -62,8 +65,8 @@ _cleanup_nvmet() {
>>    	for dev in /sys/class/nvme/nvme*; do
>>    		dev="$(basename "$dev")"
>>    		transport="$(cat "/sys/class/nvme/${dev}/transport")"
>> -		if [[ "$transport" == "loop" ]]; then
>> -			echo "WARNING: Test did not clean up loop device: ${dev}"
>> +		if [[ "$transport" == "${nvme_trtype}" ]]; then
>> +			echo "WARNING: Test did not clean up ${nvme_trtype} device: ${dev}"
>>    			_nvme_disconnect_ctrl "${dev}"
>>    		fi
>>    	done
>> @@ -87,14 +90,20 @@ _cleanup_nvmet() {
>>    	shopt -u nullglob
>>    	trap SIGINT
>>    
>> -	modprobe -r nvme-loop 2>/dev/null
>> +	modprobe -r nvme-${nvme_trtype} 2>/dev/null
>> +	if [[ "${nvme_trtype}" != "loop" ]]; then
>> +		modprobe -r nvmet-${nvme_trtype} 2>/dev/null
> This is not from your patch but I'd keep the error message it has
> turned out to be useful for me when debugging refcount problem
> especially unload and load scenario.

Again, I'd like to avoid doing things that are outside the scope
of what this is trying to achieve because it is not a small change.

We can add it incrementally.