| Message ID | 20240619172556.2968660-1-nilay@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCHv4,blktests] nvme: add test for creating/deleting file-ns | expand |
On Wed, Jun 19, 2024 at 10:55:02PM GMT, Nilay Shroff wrote: > + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" > + uuid="$(uuidgen -r)" This adds a new dependency on an external tool. It should be mentioned in the README and added to the list of tools to check for: _check_dependencies(). Alternatively you could skip the test if the tool is not available. Anyway the rest looks good. Reviewed-by: Daniel Wagner <dwagner@suse.de>
On 6/20/24 12:05, Daniel Wagner wrote: > On Wed, Jun 19, 2024 at 10:55:02PM GMT, Nilay Shroff wrote: >> + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" >> + uuid="$(uuidgen -r)" > > This adds a new dependency on an external tool. It should be mentioned > in the README and added to the list of tools to check for: > _check_dependencies(). Alternatively you could skip the test if the tool > is not available. Anyway the rest looks good. > The "uuidgen" is part of util-linux package and I saw that it's already mentioned as one of the required packages for blktest: https://github.com/osandov/blktests > Reviewed-by: Daniel Wagner <dwagner@suse.de> > Thanks, --Nilay
On Thu, Jun 20, 2024 at 12:10:32PM GMT, Nilay Shroff wrote: > > > On 6/20/24 12:05, Daniel Wagner wrote: > > On Wed, Jun 19, 2024 at 10:55:02PM GMT, Nilay Shroff wrote: > >> + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" > >> + uuid="$(uuidgen -r)" > > > > This adds a new dependency on an external tool. It should be mentioned > > in the README and added to the list of tools to check for: > > _check_dependencies(). Alternatively you could skip the test if the tool > > is not available. Anyway the rest looks good. > > > The "uuidgen" is part of util-linux package and I saw that it's already mentioned > as one of the required packages for blktest: https://github.com/osandov/blktests Ah, I just grepped for uuidgen and there is no other user. So all is good.
Tested this patch, with and without the below fix, and observing expected results. https://lore.kernel.org/all/20240613164246.75205-1-kbusch@meta.com/ Tested-by: Venkat Rao Bagalkote <venkat88@linux.vnet.ibm.com> Regards, Venkat. On 20/06/24 12:10 pm, Nilay Shroff wrote: > > On 6/20/24 12:05, Daniel Wagner wrote: >> On Wed, Jun 19, 2024 at 10:55:02PM GMT, Nilay Shroff wrote: >>> + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" >>> + uuid="$(uuidgen -r)" >> This adds a new dependency on an external tool. It should be mentioned >> in the README and added to the list of tools to check for: >> _check_dependencies(). Alternatively you could skip the test if the tool >> is not available. Anyway the rest looks good. >> > The "uuidgen" is part of util-linux package and I saw that it's already mentioned > as one of the required packages for blktest: https://github.com/osandov/blktests > >> Reviewed-by: Daniel Wagner <dwagner@suse.de> >> > Thanks, > --Nilay
On 6/19/24 10:25, Nilay Shroff wrote: > 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> Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck
On Jun 19, 2024 / 22:55, Nilay Shroff wrote: > 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> I've applied it. Thanks!
diff --git a/tests/nvme/052 b/tests/nvme/052 new file mode 100755 index 0000000..cf6061a --- /dev/null +++ b/tests/nvme/052 @@ -0,0 +1,83 @@ +#!/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 "$@" +} + +nvmf_wait_for_ns() { + local ns + local timeout="5" + local uuid="$1" + + ns=$(_find_nvme_ns "${uuid}") + + start_time=$(date +%s) + while [[ -z "$ns" ]]; do + sleep 1 + end_time=$(date +%s) + if (( end_time - start_time > timeout )); then + echo "namespace with uuid \"${uuid}\" not " \ + "found within ${timeout} seconds" + return 1 + fi + ns=$(_find_nvme_ns "${uuid}") + done + + return 0 +} + +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" + uuid="$(uuidgen -r)" + + _create_nvmet_ns "${def_subsysnqn}" "${i}" "$(_nvme_def_file_path).$i" "${uuid}" + + # wait until async request is processed and ns is created + nvmf_wait_for_ns "${uuid}" + if [ $? -eq 1 ]; then + echo "FAIL" + rm "$(_nvme_def_file_path).$i" + break + fi + + _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 v3: - Instead of sleeping for async opeartion (ns creation) to finish, wait until the ns is created by polling ns/uuid (Daniel) 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 | 83 ++++++++++++++++++++++++++++++++++++++++++++++ tests/nvme/052.out | 2 ++ 2 files changed, 85 insertions(+) create mode 100755 tests/nvme/052 create mode 100644 tests/nvme/052.out