Re: [PATCH blktests v2 05/11] nvme: Add common helpers for passthru tests

[Chaitanya Kulkarni]

On 9/30/20 12:01, Logan Gunthorpe wrote:
> Add some simple helpers to setup a passthru target that passes through
> to a nvme test device.
>
> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> ---
>  tests/nvme/rc | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index dfa57a299625..1ea23308a3f7 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -73,6 +73,17 @@ _require_nvme_trtype_is_fabrics() {
>  	return 0
>  }
>  
> +_test_dev_nvme_ctrl() {
> +	local dev
> +
> +	dev=$(cat "${TEST_DEV_SYSFS}/device/dev")
 can you initialize dev this at the time of declaration ?
> +	echo "/dev/char/${dev}"
> +}
> +
> +_test_dev_nvme_nsid() {
> +	cat "${TEST_DEV_SYSFS}/nsid"
> +}
> +
>  _cleanup_nvmet() {
>  	local dev
>  	local port
> @@ -257,6 +268,27 @@ _remove_nvmet_subsystem() {
>  	rmdir "${subsys_path}"
>  }
>  
> +_create_nvmet_passthru() {
> +	local nvmet_subsystem="$1"
> +	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
> +	local passthru_path="${subsys_path}/passthru"
> +
> +	mkdir -p "${subsys_path}"
> +	printf 1 > "${subsys_path}/attr_allow_any_host"
> +
> +	printf "%s" "$(_test_dev_nvme_ctrl)" > "${passthru_path}/device_path"
> +	printf 1 > "${passthru_path}/enable"

can you please echo in general and printf only when it is needed ?

I know existing code is a bit inconsistent I'll send a clenup to make it
uniform.

> +}
> +
> +_remove_nvmet_passhtru() {
> +	local nvmet_subsystem="$1"
> +	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
> +	local passthru_path="${subsys_path}/passthru"
> +
> +	printf 0 > "${passthru_path}/enable"
> +	rmdir "${subsys_path}"
> +}
> +
>  _add_nvmet_subsys_to_port() {
>  	local port="$1"
>  	local nvmet_subsystem="$2"
> @@ -292,6 +324,50 @@ _find_nvme_dev() {
>  	done
>  }
>  
> +_find_nvme_passthru_loop_dev() {
> +	local subsys=$1
> +	local nsid
> +	local dev
> +
> +	dev=$(_find_nvme_dev "${subsys}")
> +	nsid=$(_test_dev_nvme_nsid)
> +	echo "/dev/${dev}n${nsid}"
> +}
> +
> +_nvmet_passthru_target_setup() {
> +	local subsys_name=$1
> +
> +	_create_nvmet_passthru "${subsys_name}"
> +	port="$(_create_nvmet_port "loop")"
> +	_add_nvmet_subsys_to_port "${port}" "${subsys_name}"
> +
> +	echo "$port"
> +}
> +
> +_nvmet_passthru_target_connect() {
> +	local trtype=$1
> +	local subsys_name=$2
> +
> +	_nvme_connect_subsys "${trtype}" "${subsys_name}"
> +	nsdev=$(_find_nvme_passthru_loop_dev "${subsys_name}")
> +
> +	# The following tests can race with the creation
> +	# of the device so ensure the block device exists
> +	# before continuing
> +	while [ ! -b "${nsdev}" ]; do sleep 1; done
> +
> +	echo "${nsdev}"
> +}
> +
> +_nvmet_passthru_target_cleanup() {
> +	local port=$1
> +	local subsys_name=$2
> +
> +	_remove_nvmet_subsystem_from_port "${port}" "${subsys_name}"
> +	_remove_nvmet_port "${port}"
> +	_remove_nvmet_passhtru "${subsys_name}"
> +}
> +
>  _filter_discovery() {
>  	sed -n -r -e "s/Generation counter [0-9]+/Generation counter X/" \
>  		  -e '/Discovery Log Number|Log Entry|trtype|subnqn/p'