Re: [PATCH blktests v2 1/2] nvme: move helper functions to common/nvme

[Ofir Gal <ofir.gal@volumez.com>]

On 7/16/24 12:23, Ofir Gal wrote:
>>> -_nvme_disconnect_ctrl() {
>>> -    local ctrl="$1"
>>> -
>>> -    nvme disconnect --device "${ctrl}"
>>> -}
>>> -
>>>   _nvme_disconnect_subsys() {
>>>      local subsysnqn="$def_subsysnqn"
>>>  
>>> @@ -465,141 +206,6 @@ _nvme_disconnect_subsys() {
>>>          grep -o "disconnected.*"
>>>   }
>>>  
>>> -_nvme_connect_subsys() {
>>> -    local subsysnqn="$def_subsysnqn"
>>> -    local hostnqn="$def_hostnqn"
>>
>> It looks weird that _nvme_connect_subsys() is moved to common/nvme, but
>> _nvme_disconnect_subsys() stays in tests/nvme/rc. I think it's the better to
>> move _nvme_disconnect_subsys() also. Currently, md/001 does not use
>> _nvme_disconnect_subsys(). Isn't it the better to call it in
>> cleanup_nvme_over_tcp()?
>>
> I agree, I would move _nvme_disconnect_subsys() in v3.
> cleanup_nvme_over_tcp() use _nvme_disconnect_ctrl() to disconnect the
> controller by the controller name rather the subsys name. I can change
> it to _nvme_disconnect_subsys() if it's more appropriate.
>
I changed it to _nvme_disconnect_subsys() after using _find_nvme_ns() as
Daniel suggested, will be in v3.