| Message ID | 20200129232921.11771-4-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvme: add cntlid and model testcases | expand |
On Wed, Jan 29, 2020 at 03:29:19PM -0800, Chaitanya Kulkarni wrote: > This patch updates helper function create_nvmet_subsystem() to handle > newly introduced model attribute. > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > --- > tests/nvme/rc | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/tests/nvme/rc b/tests/nvme/rc > index d1fa796..377c7f7 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -123,6 +123,7 @@ _create_nvmet_subsystem() { > 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}" > @@ -133,6 +134,9 @@ _create_nvmet_subsystem() { > echo ${cntlid_min} > ${cfs_path}/attr_cntlid_min > echo ${cntlid_max} > ${cfs_path}/attr_cntlid_max > fi It's not in the git diff context, but the line above is if [ $# -eq 5 ] && [ -f ${cfs_path}/attr_cntlid_min ]; then So if we pass 6 arguments, the cntlid arguments are ignored? These checks should probably be -ge > + if [ $# -eq 6 ] && [ -f ${cfs_path}/attr_model ]; then > + echo ${model} > ${cfs_path}/attr_model > + fi More shellcheck warnings about unquoted variables. > } > > _remove_nvmet_ns() { > -- > 2.22.1 >
On 02/11/2020 02:00 PM, Omar Sandoval wrote: >> >> > mkdir -p "${cfs_path}" >> >@@ -133,6 +134,9 @@ _create_nvmet_subsystem() { >> > echo ${cntlid_min} > ${cfs_path}/attr_cntlid_min >> > echo ${cntlid_max} > ${cfs_path}/attr_cntlid_max >> > fi > It's not in the git diff context, but the line above is > > if [ $# -eq 5 ] && [ -f ${cfs_path}/attr_cntlid_min ]; then > > So if we pass 6 arguments, the cntlid arguments are ignored? These > checks should probably be -ge > Yes, -eq forces caller to test cntlid exclusive (without model), that was the intent here to either test cntlid or model. In this way caller can pass 0 or "" and that will be ignored. with -ge then caller has to pass meaningful value and set it properly. Do we still want to make it to -ge ?
On 02/11/2020 02:00 PM, Omar Sandoval wrote: >> fi > It's not in the git diff context, but the line above is > > if [ $# -eq 5 ] && [ -f ${cfs_path}/attr_cntlid_min ]; then > > So if we pass 6 arguments, the cntlid arguments are ignored? These > checks should probably be -ge > With the -ge change it will also print wrong skip reason for 034 I remember now I think that is the reason why kept it -eq :- Here is with -ge :- blktests (master) # ./check tests/nvme/033 nvme/033 (Test NVMeOF target cntlid[min|max] attributes) [not run] attr_cntlid_[min|max] not found blktests (master) # ./check tests/nvme/034 nvme/034 (Test NVMeOF target model attribute) [not run] attr_cntlid_[min|max] not found With -eq :- blktests (master) # ./check tests/nvme/033 nvme/033 (Test NVMeOF target cntlid[min|max] attributes) [not run] attr_cntlid_[min|max] not found blktests (master) # ./check tests/nvme/034 nvme/034 (Test NVMeOF target model attribute) [not run] attr_model not found
diff --git a/tests/nvme/rc b/tests/nvme/rc index d1fa796..377c7f7 100644 --- a/tests/nvme/rc +++ b/tests/nvme/rc @@ -123,6 +123,7 @@ _create_nvmet_subsystem() { 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}" @@ -133,6 +134,9 @@ _create_nvmet_subsystem() { echo ${cntlid_min} > ${cfs_path}/attr_cntlid_min echo ${cntlid_max} > ${cfs_path}/attr_cntlid_max fi + if [ $# -eq 6 ] && [ -f ${cfs_path}/attr_model ]; then + echo ${model} > ${cfs_path}/attr_model + fi } _remove_nvmet_ns() {
This patch updates helper function create_nvmet_subsystem() to handle newly introduced model attribute. Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- tests/nvme/rc | 4 ++++ 1 file changed, 4 insertions(+)