On Fri, Aug 18, 2023 at 01:40:56PM +0900, Shin'ichiro Kawasaki wrote: > The helper function _nvme_connect_subsys() creates a nvme device. It may > take some time after the function call until the device gets ready for > I/O. So it is expected that the test cases call _find_nvme_dev() after > _nvme_connect_subsys() before I/O. _find_nvme_dev() returns the path of > the created device, and it also waits for uuid and wwid sysfs attributes > of the created device get ready. This wait works as the wait for the > device I/O readiness. > > However, this wait by _find_nvme_dev() has two problems. The first > problem is missing call of _find_nvme_dev(). The test case nvme/047 > calls _nvme_connect_subsys() twice, but _find_nvme_dev() is called only > for the first _nvme_connect_subsys() call. This causes too early I/O to > the device with tcp transport [1]. Fix this by moving the wait for the > device readiness from _find_nvme_dev() to _nvme_connect_subsys(). Also > add --no-wait option to _nvme_connect_subsys(). It allows to skip the > wait in _nvmet_passthru_target_connect() which has its own wait for > device readiness. > > The second problem is wrong paths for the sysfs attributes. The paths > do not include namespace index, so the check for the attributes always > fail. Still _find_nvme_dev() does 1 second wait and allows the device > get ready for I/O in most cases, but this is not intended behavior. > Fix this by checking sysfs paths with the namespace index. Get list of > namespace indices for the sub-system and do the check for all indices. > > On top of the checks for sysfs attributes, add 'udevadm settle' and a > check for the created device file. These ensures that the create device > is ready for I/O. > > [1] https://lore.kernel.org/linux-block/CAHj4cs9GNohGUjohNw93jrr8JGNcRYC-ienAZz+sa7az1RK77w@xxxxxxxxxxxxxx/ > > Fixes: c766fccf3aff ("Make the NVMe tests more reliable") > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> just a minor nitpick but feel free to add: Reviewed-by: Daniel Wagner <dwagner@xxxxxxx> > diff --git a/tests/nvme/rc b/tests/nvme/rc > index 0b964e9..92eac06 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -428,6 +428,8 @@ _nvme_connect_subsys() { > local keep_alive_tmo="" > local reconnect_delay="" > local ctrl_loss_tmo="" > + local no_wait="" I suggest to use the default 'no_wait=false' here > + local i > > while [[ $# -gt 0 ]]; do > case $1 in > @@ -483,6 +485,10 @@ _nvme_connect_subsys() { > ctrl_loss_tmo="$2" > shift 2 > ;; > + --no-wait) > + no_wait=true > + shift 1 > + ;; > *) > positional_args+=("$1") > shift > @@ -532,6 +538,33 @@ _nvme_connect_subsys() { > fi > > nvme connect "${ARGS[@]}" 2> /dev/null > + > + # Wait until device file and uuid/wwid sysfs attributes get ready for > + # all namespaces. > + if [[ -z ${no_wait} ]]; then and do a if [[ "${no_wait}" = true ]] ; then instead using a testing for an empty string.
