| Message ID | 20240619104705.2921801-1-nilay@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCHv3,blktests] nvme: add test for creating/deleting file-ns | expand |
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
On Wed, Jun 19, 2024 at 04:16:40PM GMT, Nilay Shroff wrote: > + # start iteration from ns-id 2 because ns-id 1 is created > + # by default when nvme target is setup. Also ns-id 1 is > + # deleted when nvme target is cleaned up. > + for ((i = 2; i <= iterations; i++)); do { > + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" > + _create_nvmet_ns "${def_subsysnqn}" "${i}" "$(_nvme_def_file_path).$i" > + > + # allow async request to be processed > + sleep 1 This looks a bit fragile to ensure all request have been processed. Would it possible to wait on a state? E.g. something like nvmf_wait_for_state() ? > + > + _remove_nvmet_ns "${def_subsysnqn}" "${i}" > + rm "$(_nvme_def_file_path).$i" > + }
On 6/19/24 16:29, Daniel Wagner wrote: > On Wed, Jun 19, 2024 at 04:16:40PM GMT, Nilay Shroff wrote: >> + # start iteration from ns-id 2 because ns-id 1 is created >> + # by default when nvme target is setup. Also ns-id 1 is >> + # deleted when nvme target is cleaned up. >> + for ((i = 2; i <= iterations; i++)); do { >> + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" >> + _create_nvmet_ns "${def_subsysnqn}" "${i}" "$(_nvme_def_file_path).$i" >> + >> + # allow async request to be processed >> + sleep 1 > > This looks a bit fragile to ensure all request have been processed. Would > it possible to wait on a state? E.g. something like > > nvmf_wait_for_state() > > ? OK that's a good idea! I think it's possible to wait using _find_nvme_ns() instead of using sleep here. We can write a wrapper around _find_nvme_ns() and wait until namespace is ready/created. I will send next patch with the above change. > >> + >> + _remove_nvmet_ns "${def_subsysnqn}" "${i}" >> + rm "$(_nvme_def_file_path).$i" >> + } > Thanks, --Nilay
diff --git a/tests/nvme/052 b/tests/nvme/052 new file mode 100755 index 0000000..ec72e1f --- /dev/null +++ b/tests/nvme/052 @@ -0,0 +1,54 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2024 Nilay Shroff +# +# Regression test for commit be647e2c76b2(nvme: use srcu for iterating +# namespace list). This regression is resolved with commit ff0ffe5b7c3c +# (nvme: fix namespace removal 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 iterations=20 + + _nvmet_target_setup + + _nvme_connect_subsys + + # start iteration from ns-id 2 because ns-id 1 is created + # by default when nvme target is setup. Also ns-id 1 is + # deleted when nvme target is cleaned up. + for ((i = 2; i <= iterations; i++)); do { + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" + _create_nvmet_ns "${def_subsysnqn}" "${i}" "$(_nvme_def_file_path).$i" + + # allow async request to be processed + sleep 1 + + _remove_nvmet_ns "${def_subsysnqn}" "${i}" + rm "$(_nvme_def_file_path).$i" + } + done + + _nvme_disconnect_subsys >> "${FULL}" 2>&1 + + _nvmet_target_cleanup + + echo "Test complete" +} diff --git a/tests/nvme/052.out b/tests/nvme/052.out new file mode 100644 index 0000000..f2d186d --- /dev/null +++ b/tests/nvme/052.out @@ -0,0 +1,2 @@ +Running nvme/052 +Test complete
This is regression test for commit be647e2c76b2 (nvme: use srcu for iterating namespace list)[1]. It is fixed in commit ff0ffe5b7c3c(nvme: fix namespace removal list)[2]. This test uses a regular file backed loop device for creating and then deleting an NVMe namespace in a loop. [1] https://lore.kernel.org/all/2312e6c3-a069-4388-a863-df7e261b9d70@linux.vnet.ibm.com/ [2] https://lore.kernel.org/all/20240613164246.75205-1-kbusch@meta.com/ Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> --- Changes from v2: - Use defaults for creating and connecting to nvme target (Daniel) Changes from v1: - Add nvme prefix in the subject line instead of loop (Chaitanya) - Enrich the commit log with details including link to regression discussion and fix commit (Chaitanya) - Few other formatting cleanup (Chaitanya) - Add fix commit information in the test header (Shinichiro) - Instead of using default 1000 iteration for test, set the test iteration count to 20 (Shinichiro) - Update test case no. to nvme/052 (Shinichiro) --- tests/nvme/052 | 54 ++++++++++++++++++++++++++++++++++++++++++++++ tests/nvme/052.out | 2 ++ 2 files changed, 56 insertions(+) create mode 100755 tests/nvme/052 create mode 100644 tests/nvme/052.out