Re: [PATCH blktests v3 07/12] nvme: Make test image size configurable

From: Shinichiro Kawasaki
Date: Sun May 07 2023 - 19:10:28 EST


Please find three nit comments in line.

On May 03, 2023 / 10:02, Daniel Wagner wrote:
> The reduce the overall runtime of the testsuite by making the default
> size of the test image small. For verification jobs, the default can be
> overwriten via the newly introduced nvme_img_size environment variable.
>
> Reviewed-by: Hannes Reinecke <hare@xxxxxxx>
> Signed-off-by: Daniel Wagner <dwagner@xxxxxxx>
> ---
> Documentation/running-tests.md | 3 +++
> tests/nvme/004 | 2 +-
> tests/nvme/005 | 2 +-
> tests/nvme/006 | 2 +-
> tests/nvme/007 | 2 +-
> tests/nvme/008 | 2 +-
> tests/nvme/009 | 2 +-
> tests/nvme/010 | 5 +++--
> tests/nvme/011 | 5 +++--
> tests/nvme/012 | 2 +-
> tests/nvme/013 | 2 +-
> tests/nvme/014 | 2 +-
> tests/nvme/015 | 2 +-
> tests/nvme/017 | 2 +-
> tests/nvme/018 | 2 +-
> tests/nvme/019 | 2 +-
> tests/nvme/020 | 2 +-
> tests/nvme/021 | 2 +-
> tests/nvme/022 | 2 +-
> tests/nvme/023 | 2 +-
> tests/nvme/024 | 2 +-
> tests/nvme/025 | 2 +-
> tests/nvme/026 | 2 +-
> tests/nvme/027 | 2 +-
> tests/nvme/028 | 2 +-
> tests/nvme/029 | 3 ++-
> tests/nvme/031 | 2 +-
> tests/nvme/032 | 1 -
> tests/nvme/034 | 2 +-
> tests/nvme/035 | 4 ++--
> tests/nvme/040 | 2 +-
> tests/nvme/041 | 2 +-
> tests/nvme/042 | 2 +-
> tests/nvme/043 | 2 +-
> tests/nvme/044 | 2 +-
> tests/nvme/045 | 2 +-
> tests/nvme/047 | 2 +-
> tests/nvme/048 | 2 +-
> tests/nvme/rc | 1 +
> 39 files changed, 46 insertions(+), 40 deletions(-)
>
> diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md
> index 7e827fba7ac0..6b34a253fd97 100644
> --- a/Documentation/running-tests.md
> +++ b/Documentation/running-tests.md
> @@ -104,6 +104,9 @@ The NVMe tests can be additionally parameterized via environment variables.
>
> - nvme_tr_type: 'loop' (default), 'tcp', 'rdma' and 'fc'
> Run the tests with the given transport.
> +- nvme_img_size: '1G' (default)
> + Run the tests with given image size in bytes. 'm' and 'g' postfix

It would be the better to list all of 'm', 'M', 'g' and 'G', since '1G' is the
default value.

> + are supported.
>
> ### Running nvme-rdma nvmeof-mp srp tests
>
> diff --git a/tests/nvme/004 b/tests/nvme/004
> index 9dda538b1ac0..cab98ff44326 100755
> --- a/tests/nvme/004
> +++ b/tests/nvme/004
> @@ -25,7 +25,7 @@ test() {
> local port
> port="$(_create_nvmet_port "${nvme_trtype}")"
>
> - truncate -s 1G "$TMPDIR/img"
> + truncate -s "${nvme_img_size}" "$TMPDIR/img"
>
> local loop_dev
> loop_dev="$(losetup -f --show "$TMPDIR/img")"
> diff --git a/tests/nvme/005 b/tests/nvme/005
> index de567a74a891..8e15a13f3794 100755
> --- a/tests/nvme/005
> +++ b/tests/nvme/005
> @@ -24,7 +24,7 @@ test() {
> local port
> port="$(_create_nvmet_port "${nvme_trtype}")"
>
> - truncate -s 1G "$TMPDIR/img"
> + truncate -s "${nvme_img_size}" "$TMPDIR/img"
>
> local loop_dev
> loop_dev="$(losetup -f --show "$TMPDIR/img")"
> diff --git a/tests/nvme/006 b/tests/nvme/006
> index d993861c06ba..ea0db93791a7 100755
> --- a/tests/nvme/006
> +++ b/tests/nvme/006
> @@ -24,7 +24,7 @@ test() {
>
> _setup_nvmet
>
> - truncate -s 1G "$TMPDIR/img"
> + truncate -s "${nvme_img_size}" "$TMPDIR/img"
>
> loop_dev="$(losetup -f --show "$TMPDIR/img")"
>
> diff --git a/tests/nvme/007 b/tests/nvme/007
> index d53100f3ff7b..243a79f5a254 100755
> --- a/tests/nvme/007
> +++ b/tests/nvme/007
> @@ -25,7 +25,7 @@ test() {
>
> file_path="${TMPDIR}/img"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/008 b/tests/nvme/008
> index 5568fe46e463..5abc4240ca46 100755
> --- a/tests/nvme/008
> +++ b/tests/nvme/008
> @@ -26,7 +26,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
>
> diff --git a/tests/nvme/009 b/tests/nvme/009
> index 2814c79164ee..491d3c809ab0 100755
> --- a/tests/nvme/009
> +++ b/tests/nvme/009
> @@ -24,7 +24,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/010 b/tests/nvme/010
> index b7b1d5188e9b..805f80d40620 100755
> --- a/tests/nvme/010
> +++ b/tests/nvme/010
> @@ -26,7 +26,7 @@ test() {
> local file_path="${TMPDIR}/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
>
> @@ -41,7 +41,8 @@ test() {
> cat "/sys/block/${nvmedev}n1/uuid"
> cat "/sys/block/${nvmedev}n1/wwid"
>
> - _run_fio_verify_io --size=950m --filename="/dev/${nvmedev}n1"
> + _run_fio_verify_io --size=${nvme_img_size} \

ShellCheck compains no double quotes in the line above.

> + --filename="/dev/${nvmedev}n1"
>
> _nvme_disconnect_subsys "${subsys_name}"
>
> diff --git a/tests/nvme/011 b/tests/nvme/011
> index 4bfe9af084e4..da8cbac11124 100755
> --- a/tests/nvme/011
> +++ b/tests/nvme/011
> @@ -26,7 +26,7 @@ test() {
> local file_path="${TMPDIR}/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> @@ -39,7 +39,8 @@ test() {
> cat "/sys/block/${nvmedev}n1/uuid"
> cat "/sys/block/${nvmedev}n1/wwid"
>
> - _run_fio_verify_io --size=950m --filename="/dev/${nvmedev}n1"
> + _run_fio_verify_io --size="${nvme_img_size}" \
> + --filename="/dev/${nvmedev}n1"
>
> _nvme_disconnect_subsys "${subsys_name}"
>
> diff --git a/tests/nvme/012 b/tests/nvme/012
> index c9d24388306d..ecf44fcb5a51 100755
> --- a/tests/nvme/012
> +++ b/tests/nvme/012
> @@ -29,7 +29,7 @@ test() {
> local file_path="${TMPDIR}/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
>
> diff --git a/tests/nvme/013 b/tests/nvme/013
> index 265b6968fd34..e249add46295 100755
> --- a/tests/nvme/013
> +++ b/tests/nvme/013
> @@ -28,7 +28,7 @@ test() {
>
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/014 b/tests/nvme/014
> index 875e99eea346..55d920f2660b 100755
> --- a/tests/nvme/014
> +++ b/tests/nvme/014
> @@ -29,7 +29,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
>
> diff --git a/tests/nvme/015 b/tests/nvme/015
> index 8cb343e1d31c..071141380e43 100755
> --- a/tests/nvme/015
> +++ b/tests/nvme/015
> @@ -28,7 +28,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/017 b/tests/nvme/017
> index f2a95cf276cb..0248aee9bc41 100755
> --- a/tests/nvme/017
> +++ b/tests/nvme/017
> @@ -25,7 +25,7 @@ test() {
>
> file_path="${TMPDIR}/img"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/018 b/tests/nvme/018
> index 315e79534348..78e9b2bb94d8 100755
> --- a/tests/nvme/018
> +++ b/tests/nvme/018
> @@ -26,7 +26,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/019 b/tests/nvme/019
> index 4cb3509a12b2..9fff8ccaac9c 100755
> --- a/tests/nvme/019
> +++ b/tests/nvme/019
> @@ -28,7 +28,7 @@ test() {
> local nblk_range="10,10,10,10,10,10,10,10,10,10"
> local sblk_range="100,200,300,400,500,600,700,800,900,1000"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
>
> diff --git a/tests/nvme/020 b/tests/nvme/020
> index 16fdfcc94918..1966d5925213 100755
> --- a/tests/nvme/020
> +++ b/tests/nvme/020
> @@ -26,7 +26,7 @@ test() {
> local nblk_range="10,10,10,10,10,10,10,10,10,10"
> local sblk_range="100,200,300,400,500,600,700,800,900,1000"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/021 b/tests/nvme/021
> index 6ee0af1fe158..1fefc0fbca00 100755
> --- a/tests/nvme/021
> +++ b/tests/nvme/021
> @@ -25,7 +25,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/022 b/tests/nvme/022
> index 1d76ffa44178..1ff915786f86 100755
> --- a/tests/nvme/022
> +++ b/tests/nvme/022
> @@ -25,7 +25,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/023 b/tests/nvme/023
> index b65be07edc38..90316230a3d7 100755
> --- a/tests/nvme/023
> +++ b/tests/nvme/023
> @@ -26,7 +26,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
>
> diff --git a/tests/nvme/024 b/tests/nvme/024
> index f756797d6f29..384a8266e741 100755
> --- a/tests/nvme/024
> +++ b/tests/nvme/024
> @@ -25,7 +25,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/025 b/tests/nvme/025
> index 941bf36f67fb..815223d1c28d 100755
> --- a/tests/nvme/025
> +++ b/tests/nvme/025
> @@ -25,7 +25,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/026 b/tests/nvme/026
> index c3f06c2a377c..d2203f19f026 100755
> --- a/tests/nvme/026
> +++ b/tests/nvme/026
> @@ -25,7 +25,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/027 b/tests/nvme/027
> index 0ad663ace811..97fe70e78344 100755
> --- a/tests/nvme/027
> +++ b/tests/nvme/027
> @@ -25,7 +25,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/028 b/tests/nvme/028
> index 7de977a81213..c539620bee28 100755
> --- a/tests/nvme/028
> +++ b/tests/nvme/028
> @@ -25,7 +25,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> _create_nvmet_subsystem "${subsys_name}" "${file_path}" \
> "91fdba0d-f87b-4c25-b80f-db7be1418b9e"
> diff --git a/tests/nvme/029 b/tests/nvme/029
> index f8b4cbbb9156..1808b7b0edf1 100755
> --- a/tests/nvme/029
> +++ b/tests/nvme/029
> @@ -14,6 +14,7 @@ requires() {
> _nvme_requires
> _have_loop
> _require_nvme_trtype_is_fabrics
> + _require_test_dev_size 1M

The line above is added here, and removed in the next patch. I suppose these
are left over from the working code.

> }
>
> test_user_io()
> @@ -59,7 +60,7 @@ test() {
> local file_path="$TMPDIR/img"
> local subsys_name="blktests-subsystem-1"
>
> - truncate -s 1G "${file_path}"
> + truncate -s "${nvme_img_size}" "${file_path}"
>
> loop_dev="$(losetup -f --show "${file_path}")"
>
> diff --git a/tests/nvme/031 b/tests/nvme/031
> index 4e1798246db1..e70898819a86 100755
> --- a/tests/nvme/031
> +++ b/tests/nvme/031
> @@ -33,7 +33,7 @@ test() {
>
> _setup_nvmet
>
> - truncate -s 1G "$TMPDIR/img"
> + truncate -s "${nvme_img_size}" "$TMPDIR/img"
>
> loop_dev="$(losetup -f --show "$TMPDIR/img")"
>
> diff --git a/tests/nvme/032 b/tests/nvme/032
> index 2e4b7f0760c7..46bbff25e70d 100755
> --- a/tests/nvme/032
> +++ b/tests/nvme/032
> @@ -40,7 +40,6 @@ test_device() {
> # start fio job
> _run_fio_rand_io --filename="$TEST_DEV" \
> --group_reporting --time_based --runtime=-1 &> /dev/null &
> -
> sleep 5
>
> if [[ ! -d "$sysfs" ]]; then

The hunk above does not look required.