On 6/18/24 07:05, Chaitanya Kulkarni wrote: > On 6/17/24 02:17, Nilay Shroff wrote: > > I think subject line should start with nvme ? > > nvme: add test for creating/deleting file-ns Ok will do. > >> This is regression test for commit be647e2c76b2 >> (nvme: use srcu for iterating namespace list) >> >> This test uses a regulare file backed loop device >> for creating and then deleting an NVMe namespace >> in a loop. > > > s/regulare/regular/ ? yes will correct the spelling. > > nit:- commit log looks a bit short :- > > This is regression test for commit be647e2c76b2 > (nvme: use srcu for iterating namespace list) > > This test uses a regulare file backed loop device for creating and > then deleting an NVMe namespace in a loop. > >> Signed-off-by: Nilay Shroff <nilay@xxxxxxxxxxxxx> >> --- >> This regression was first reported[1], and now it's >> fixed in 6.10-rc4[2] >> >> [1] https://lore.kernel.org/all/2312e6c3-a069-4388-a863-df7e261b9d70@xxxxxxxxxxxxxxxxxx/ >> [2] commit ff0ffe5b7c3c (nvme: fix namespace removal list) > > it will be helpful in long run to add above information > into the commit log, Shinichiro any thoughts ? > yes I will add more information in the commit log. >> --- >> tests/nvme/051 | 65 ++++++++++++++++++++++++++++++++++++++++++++++ >> tests/nvme/051.out | 2 ++ >> 2 files changed, 67 insertions(+) >> create mode 100755 tests/nvme/051 >> create mode 100644 tests/nvme/051.out >> >> diff --git a/tests/nvme/051 b/tests/nvme/051 >> new file mode 100755 >> index 0000000..0de5c56 >> --- /dev/null >> +++ b/tests/nvme/051 >> @@ -0,0 +1,65 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-3.0+ >> +# Copyright (C) 2024 Nilay Shroff(nilay@xxxxxxxxxxxxx) > > not sure we need to have email address here as it's a part of > commit log anyways ... > I saw in some test cases email address was added and so I also included mine. But anyways, I will remove the email address as you suggested. >> +# >> +# Regression test for commit be647e2c76b2(nvme: use srcu for iterating >> +# namespace list) >> + >> +. tests/nvme/rc >> + >> +DESCRIPTION="Test file-ns creation/deletion under one subsystem" >> + >> +requires() { >> + _nvme_requires >> + _have_loop >> + _require_nvme_trtype_is_loop >> +} >> + >> +set_conditions() { >> + _set_nvme_trtype "$@" >> +} >> + >> +test() { >> + echo "Running ${TEST_NAME}" >> + >> + _setup_nvmet >> + >> + local subsys="blktests-subsystem-1" >> + local iterations="${NVME_NUM_ITER}" > > no need for above var, I think direct use of NVME_NUM_ITER is > clear here ... Alright, accepted. > >> + local loop_dev >> + local port >> + >> + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path)" >> + >> + loop_dev="$(losetup -f --show "$(_nvme_def_file_path)")" >> + >> + port="$(_create_nvmet_port "${nvme_trtype}")" >> + >> + _nvmet_target_setup --subsysnqn "${subsys}" --blkdev "${loop_dev}" >> + >> + _nvme_connect_subsys --subsysnqn "${subsys}" >> + >> + for ((i = 2; i <= iterations; i++)); do { > > small comment would be useful to explain why are starting at 2 ... Yes I will add the relevant comment. > >> + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" >> + _create_nvmet_ns "${subsys}" "${i}" "$(_nvme_def_file_path).$i" >> + >> + # allow async request to be processed >> + sleep 1 >> + >> + _remove_nvmet_ns "${subsys}" "${i}" >> + rm "$(_nvme_def_file_path).$i" >> + } >> + done >> + >> + _nvme_disconnect_subsys --subsysnqn "${subsys}" >> "${FULL}" 2>&1 >> + >> + _nvmet_target_cleanup --subsysnqn "${subsys}" --blkdev "${loop_dev}" >> + >> + _remove_nvmet_port "${port}" >> + >> + losetup -d "$loop_dev" >> + >> + rm "$(_nvme_def_file_path)" >> + >> + echo "Test complete" >> +} >> diff --git a/tests/nvme/051.out b/tests/nvme/051.out >> new file mode 100644 >> index 0000000..156f068 >> --- /dev/null >> +++ b/tests/nvme/051.out >> @@ -0,0 +1,2 @@ >> +Running nvme/051 >> +Test complete > > thanks for the test, I think this is much needed test especially with > recent reported issues ... > Thank you for your review and comments! Much appreciated... I will send v2 with above comments incorporated. Thanks, --Nilay