On 8/7/20 10:19 AM, Sagi Grimberg wrote:
>
>>> 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.
Okay.
>
>>
>>>
>>> @@ -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.
That make sense, we can do it incrementally.
>
