Re: [PATCH blktests V2 1/3] nvme: allow target to set cntlid min/max & model

[Omar Sandoval <osandov@osandov.com>]

On Fri, Feb 14, 2020 at 05:38:29PM -0800, Chaitanya Kulkarni wrote:
> This patch updates helper function create_nvmet_subsystem() to handle
> newly introduced model, cntlid_min and cntlid_max attributes.
> Also, this adds SKIP reason when attributes are not found in the
> configfs.
> 
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> ---
>  tests/nvme/rc | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 40f0413..e4b57cb 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -121,11 +121,36 @@ _create_nvmet_subsystem() {
>  	local nvmet_subsystem="$1"
>  	local blkdev="$2"
>  	local uuid=$3
> +	local cntlid_min=$4
> +	local cntlid_max=$5
> +	local model=$6
>  	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
>  
>  	mkdir -p "${cfs_path}"
>  	echo 1 > "${cfs_path}/attr_allow_any_host"
> +
> +	if [ $# -eq 5 ]; then

I still don't like that we ignore this if we also pass the model.
Instead, just make this

if [[ -n $cntlid ]]

Then the caller can pass an empty argument if they want it ignored.

> +		if [ -f "${cfs_path}"/attr_cntlid_min ]; then
> +			echo "${cntlid_min}" > "${cfs_path}"/attr_cntlid_min
> +			echo "${cntlid_max}" > "${cfs_path}"/attr_cntlid_max
> +		else
> +			SKIP_REASON="attr_cntlid_[min|max] not found"
> +			rmdir "${cfs_path}"
> +			return 1
> +		fi
> +	fi