[PATCHv2 blktests 2/2] nvme: add test for writing to file-ns just after disabling it

[Shinichiro Kawasaki]

Thanks for this v2 patch. I took a quick look. Please find my comments in line.
Will do another round of review when v3 comes out.

On Dec 04, 2024 / 17:17, Nilay Shroff wrote:
> This is a regression test for commit 505363957fad ("nvmet: fix nvme
> status code when namespace is disabled")[1].
> 
> This test creates a regular file backed loop target namespace, disables
> the asynchronous event notification for ns-changed events and then write
> to the namespace just after disabling it.
> 
> [1] https://lore.kernel.org/linux-nvme/tqcy3sveity7p56v7ywp7ssyviwcb3w4623cnxj3knoobfcanq@yxgt2mjkbkam/
> 
> Signed-off-by: Nilay Shroff <nilay at linux.ibm.com>
> ---
>  tests/nvme/055     | 117 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/nvme/055.out |   2 +
>  2 files changed, 119 insertions(+)
>  create mode 100755 tests/nvme/055
>  create mode 100644 tests/nvme/055.out
> 
> diff --git a/tests/nvme/055 b/tests/nvme/055
> new file mode 100755
> index 0000000..f1f19fe
> --- /dev/null
> +++ b/tests/nvme/055
> @@ -0,0 +1,117 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024 Nilay Shroff
> +#
> +# Regression test for commit 505363957fad ("nvmet: fix nvme
> +# status code when namespace is disabled").

As I commented for v1 series after this v2 came out, I suggest to describe that
this test intents to be independent from udev rules.

> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Test nvme write to a loop target ns just after ns is disabled"
> +
> +QUICK=1
> +

[...]

> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	local iteration=1 i=0
> +
> +	_setup_nvmet
> +
> +	_nvmet_target_setup
> +
> +	_nvme_connect_subsys
> +
> +	img="$(mktemp /tmp/blk_img_XXXXXX)"
> +	dd if=/dev/urandom of="$img" bs=512 count=1 status=none

mktemp is not recommended, since blktests provides $TMPDIR as described
in the "new" file.

> +
> +	subsys_path="${NVMET_CFS}/subsystems/${def_subsysnqn}"
> +	ns_path="${subsys_path}/namespaces/${def_nsid}"
> +
> +	while (( i < iteration )); do
> +		# Wait until async request is processed and default ns
> +		# is created

[...]

> +
> +		# disable target namespace and write to it
> +		echo 0 > ${ns_path}/enable
> +		nvme write --start-block=1 --block-count=0 \
> +			--data-size=512 --data="$img" "$disk" 2>>"$FULL"

Instead of --data="$img", can we use"--data=/dev/urandam"? If it works,
we can avoid the img file.

> +
> +		((i++))
> +	done
> +
> +	rm "$img"

If we need img file, let's use $TMPDIR to skip this rm command.

> +
> +	_nvme_disconnect_subsys >> "$FULL" 2>&1
> +
> +	_nvmet_target_cleanup
> +
> +	echo "Test complete"
> +}

[...]