[PATCH blktests 09/15] nvme: check for controller device in _nvme_connect_subsys()

[Daniel Wagner]

On Thu, Oct 24, 2024 at 10:00:19AM GMT, Shin'ichiro Kawasaki wrote:
> From: Hannes Reinecke <hare at suse.de>
> 
> When calling 'nvme connect' we cannot assume that a namespace is
> always present (eg if connecting to a discovery service), so better
> check for the controller device to ensure that 'nvme connect' has
> succeeded.
> 
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> [Shin'ichiro: fixed _nvme_connect_subsys() return for no_wait case]
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
> ---
>  common/nvme | 37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/common/nvme b/common/nvme
> index 758df33..fa64ad4 100644
> --- a/common/nvme
> +++ b/common/nvme
> @@ -373,17 +373,20 @@ _nvme_connect_subsys() {
>  	if [[ -n "${ctrl_loss_tmo}" ]]; then
>  		ARGS+=(--ctrl-loss-tmo="${ctrl_loss_tmo}")
>  	fi
> +	ARGS+=(-o json)
> +	connect=$(nvme connect "${ARGS[@]}" 2> /dev/null)
>  
> -	nvme connect "${ARGS[@]}" 2> /dev/null | grep -v "connecting to device:"
> -
> -	# Wait until device file and uuid/wwid sysfs attributes get ready for
> -	# all namespaces.
> +	# Wait until device file and sysfs attributes get ready
>  	if [[ ${no_wait} = false ]]; then
> +		local ctrldev
> +
> +		ctrldev=$(echo "${connect}"  | sed -n 's/.*device.:.\(nvme[0-9]*\)./\1/p')
>  		udevadm settle
>  		for ((i = 0; i < 10; i++)); do
> -			_nvme_ns_ready "${subsysnqn}" && return
> +			_nvme_ctrl_ready "${ctrldev}" "${subsysnqn}" && return 0
>  			sleep .1
>  		done
> +		return 1
>  	fi
>  }
>  
> @@ -408,21 +411,17 @@ _nvme_disconnect_subsys() {
>  }
>  
>  
> -_nvme_ns_ready() {
> -	local subsysnqn="${1}"
> -	local ns_path ns_id dev
> -	local cfs_path="${NVMET_CFS}/subsystems/$subsysnqn"
> +_nvme_ctrl_ready() {
> +	local ctrldev="${1}"
> +	local subsysnqn="${2:-$def_subsysnqn}"
> +	local ctrlpath="/sys/class/nvme/${ctrldev}"
>  
> -	dev=$(_find_nvme_dev "$subsysnqn")
> -	for ns_path in "${cfs_path}/namespaces/"*; do
> -		ns_id=${ns_path##*/}
> -		if [[ ! -b /dev/${dev}n${ns_id} ||
> -			   ! -e /sys/block/${dev}n${ns_id}/uuid ||
> -			   ! -e /sys/block/${dev}n${ns_id}/wwid ]]; then
> -			return 1

IIRC, the uuid/wwid test were originally from nvme/004 and copied to the
other test. When we refactored we moved the uuid/wwid here. I suggest we
add this part back to nvme/004 so that we don't loose this test case.