On Fri, Sep 08, 2023 at 11:26:31AM +0000, Shinichiro Kawasaki wrote:
> On Sep 07, 2023 / 11:44, Yi Zhang wrote:
> > allow_any_host was disabled during _create_nvmet_subsystem, call
> > _create_nvmet_host before connecting to allow the host to connect.
> >
> > [76096.420586] nvmet: adding nsid 1 to subsystem blktests-subsystem-0
> > [76096.440595] nvmet_tcp: enabling port 0 (127.0.0.1:4420)
> > [76096.491344] nvmet: connect by host nqn.2014-08.org.nvmexpress:uuid:0f01fb42-9f7f-4856-b0b3-51e60b8de349 for subsystem blktests-subsystem-0 not allowed
> > [76096.505049] nvme nvme2: Connect for subsystem blktests-subsystem-0 is not allowed, hostnqn: nqn.2014-08.org.nvmexpress:uuid:0f01fb42-9f7f-4856-b0b3-51e60b8de349
> > [76096.519609] nvme nvme2: failed to connect queue: 0 ret=16772
> >
> > Signed-off-by: Yi Zhang <yi.zhang@xxxxxxxxxx>
Reviewed-by: Daniel Wagner <dwagner@xxxxxxx>
> Thanks for the catching this. I looked back the past changes and found that the
> commit c32b233b7dd6 ("nvme/rc: Add helper for adding/removing to allow list")
> triggered the connection failure. So, I think a Fixes tag with this commit is
> required (I can add when this patch is applied).
>
> Even after the commit, the test case still passes. That's why I did not notice
> the connection failure. I think _nvme_connect_subsys() should check exit status
> of "nvme connect" command and print an error message on failure. This will help
> to catch similar connection failures in future.
I was running into a similiar problem for (not yet existing) nvme/050
test case [1]:
nvmf_wait_for_state() {
local def_state_timeout=5
local subsys_name="$1"
local state="$2"
local timeout="${3:-$def_state_timeout}"
local nvmedev
local state_file
local start_time
local end_time
nvmedev=$(_find_nvme_dev "${subsys_name}")
state_file="/sys/class/nvme-fabrics/ctl/${nvmedev}/state"
start_time=$(date +%s)
while ! grep -q "${state}" "${state_file}"; do
sleep 1
end_time=$(date +%s)
if (( end_time - start_time > timeout )); then
echo "expected state \"${state}\" not " \
"reached within ${timeout} seconds"
return 1
fi
done
return 0
}
_nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" \
--hostnqn "${def_hostnqn}" \
--reconnect-delay 1 \
--ctrl-loss-tmo 10
nvmf_wait_for_state "${def_subsysnqn}" "live"
nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
We could make this a bit more generic and move it into the connect
helper. What do you think?
[1] https://lore.kernel.org/linux-nvme/20230621155825.20146-2-dwagner@xxxxxxx/
