[PATCH blktests] tests/nvme/031: fix connecting faiure

[Daniel Wagner]

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 at redhat.com>

Reviewed-by: Daniel Wagner <dwagner at suse.de>

> 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@suse.de/