diff mbox series

[blktests,v3,2/2] nvme: test the nvme reservation feature

Message ID 20241012111157.44368-3-kanie@linux.alibaba.com (mailing list archive)
State New
Headers show
Series Test the NVMe reservation feature | expand

Commit Message

Guixin Liu Oct. 12, 2024, 11:11 a.m. UTC 
Test the NVMe reservation feature, including register, acquire,
release and report.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 tests/nvme/054     |  99 +++++++++++++++++++++++++++++++++++++++++
 tests/nvme/054.out | 108 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 207 insertions(+)
 create mode 100644 tests/nvme/054
 create mode 100644 tests/nvme/054.out

Comments

Chaitanya Kulkarni Oct. 14, 2024, 6:43 a.m. UTC | #1 
On 10/12/24 04:11, Guixin Liu wrote:
> Test the NVMe reservation feature, including register, acquire,
> release and report.
>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>   tests/nvme/054     |  99 +++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/054.out | 108 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 207 insertions(+)
>   create mode 100644 tests/nvme/054
>   create mode 100644 tests/nvme/054.out
>
> diff --git a/tests/nvme/054 b/tests/nvme/054
> new file mode 100644
> index 0000000..f352c73
> --- /dev/null
> +++ b/tests/nvme/054
> @@ -0,0 +1,99 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024 Guixin Liu
> +# Copyright (C) 2024 Alibaba Group.
> +#
> +# Test the NVMe reservation feature
> +#
> +. tests/nvme/rc
> +
> +DESCRIPTION="Test the NVMe reservation feature"
> +QUICK=1
> +nvme_trtype="loop"
> +
> +requires() {
> +	_nvme_requires
> +}
> +
> +resv_report() {
> +	local nvmedev=$1
> +	local report_arg=$2
> +
> +	nvme resv-report "/dev/${nvmedev}n1" "${report_arg}" | grep -v "hostid"
> +}
> +
> +test_resv() {
> +	local nvmedev=$1
> +	local report_arg="--cdw11=1"
> +
> +	if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
> +		report_arg="--eds"
> +	fi
> +
> +	echo "Register"
> +	resv_report "${nvmedev}" "${report_arg}"
> +	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
> +	resv_report "${nvmedev}" "${report_arg}"
> +
> +	echo "Replace"
> +	nvme resv-register "/dev/${nvmedev}n1" --crkey=4 --nrkey=5 --rrega=2
> +	resv_report "${nvmedev}" "${report_arg}"
> +
> +	echo "Unregister"
> +	nvme resv-register "/dev/${nvmedev}n1" --crkey=5 --rrega=1
> +	resv_report "${nvmedev}" "${report_arg}"
> +
> +	echo "Acquire"
> +	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
> +	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
> +	resv_report "${nvmedev}" "${report_arg}"
> +
> +	echo "Preempt"
> +	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --racqa=1
> +	resv_report "${nvmedev}" "${report_arg}"
> +
> +	echo "Release"
> +	nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --rrela=0
> +	resv_report "${nvmedev}" "${report_arg}"
> +
> +	echo "Clear"
> +	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
> +	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
> +	resv_report "${nvmedev}" "${report_arg}"
> +	nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rrela=1
> +}
> +
> +

make it easier to debug totally untested :-

test_resv() {
         local nvmedev=$1
         local report_arg="--cdw11=1"
         test_dev="/dev/${nvmedev}n1"

         if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
                 report_arg="--eds"
         fi

         echo "Register"
         resv_report "${nvmedev}" "${report_arg}"
         nvme resv-register "${test_dev}" --nrkey=4 --rrega=0
         resv_report "${nvmedev}" "${report_arg}"

         echo "Replace"
         nvme resv-register "${test_dev}" --crkey=4 --nrkey=5 --rrega=2
         resv_report "${nvmedev}" "${report_arg}"

         echo "Unregister"
         nvme resv-register "${test_dev}" --crkey=5 --rrega=1
         resv_report "${nvmedev}" "${report_arg}"

         echo "Acquire"
         nvme resv-register "${test_dev}" --nrkey=4 --rrega=0
         nvme resv-acquire "${test_dev}" --crkey=4 --rtype=1 --racqa=0
         resv_report "${nvmedev}" "${report_arg}"

         echo "Preempt"
         nvme resv-acquire "${test_dev}" --crkey=4 --rtype=2 --racqa=1
         resv_report "${nvmedev}" "${report_arg}"

         echo "Release"
         nvme resv-release "${test_dev}" --crkey=4 --rtype=2 --rrela=0
         resv_report "${nvmedev}" "${report_arg}"

         echo "Clear"
         nvme resv-register "${test_dev}" --nrkey=4 --rrega=0
         nvme resv-acquire "${test_dev}" --crkey=4 --rtype=1 --racqa=0
         resv_report "${nvmedev}" "${report_arg}"
         nvme resv-release "${test_dev}" --crkey=4 --rrela=1
}


irrespective of that looks good :-

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck
Daniel Wagner Oct. 14, 2024, 7:23 a.m. UTC | #2 
On Sat, Oct 12, 2024 at 07:11:57PM GMT, Guixin Liu wrote:
> +requires() {
> +	_nvme_requires
> +}

It's probably a good idea to test if the reservation feature is
available, otherwise this test just fails on older kernels.

> +Running nvme/054
> +Register
> +
> +NVME Reservation status:
> +
> +gen       : 0
> +rtype     : 0
> +regctl    : 0
> +ptpls     : 0
> +
> +NVME Reservation  success

I think this is a bit fragile, because the output of the nvme command
might be extended or reformatted (obviously not on purpose but happens).
I'd recommend to explicitly check for the expected values in the test.
Guixin Liu Oct. 14, 2024, 8:34 a.m. UTC | #3 
在 2024/10/14 14:43, Chaitanya Kulkarni 写道:
> On 10/12/24 04:11, Guixin Liu wrote:
>> Test the NVMe reservation feature, including register, acquire,
>> release and report.
>>
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>> ---
>>    tests/nvme/054     |  99 +++++++++++++++++++++++++++++++++++++++++
>>    tests/nvme/054.out | 108 +++++++++++++++++++++++++++++++++++++++++++++
>>    2 files changed, 207 insertions(+)
>>    create mode 100644 tests/nvme/054
>>    create mode 100644 tests/nvme/054.out
>>
>> diff --git a/tests/nvme/054 b/tests/nvme/054
>> new file mode 100644
>> index 0000000..f352c73
>> --- /dev/null
>> +++ b/tests/nvme/054
>> @@ -0,0 +1,99 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2024 Guixin Liu
>> +# Copyright (C) 2024 Alibaba Group.
>> +#
>> +# Test the NVMe reservation feature
>> +#
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Test the NVMe reservation feature"
>> +QUICK=1
>> +nvme_trtype="loop"
>> +
>> +requires() {
>> +	_nvme_requires
>> +}
>> +
>> +resv_report() {
>> +	local nvmedev=$1
>> +	local report_arg=$2
>> +
>> +	nvme resv-report "/dev/${nvmedev}n1" "${report_arg}" | grep -v "hostid"
>> +}
>> +
>> +test_resv() {
>> +	local nvmedev=$1
>> +	local report_arg="--cdw11=1"
>> +
>> +	if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
>> +		report_arg="--eds"
>> +	fi
>> +
>> +	echo "Register"
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +
>> +	echo "Replace"
>> +	nvme resv-register "/dev/${nvmedev}n1" --crkey=4 --nrkey=5 --rrega=2
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +
>> +	echo "Unregister"
>> +	nvme resv-register "/dev/${nvmedev}n1" --crkey=5 --rrega=1
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +
>> +	echo "Acquire"
>> +	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
>> +	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +
>> +	echo "Preempt"
>> +	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --racqa=1
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +
>> +	echo "Release"
>> +	nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --rrela=0
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +
>> +	echo "Clear"
>> +	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
>> +	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
>> +	resv_report "${nvmedev}" "${report_arg}"
>> +	nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rrela=1
>> +}
>> +
>> +
> make it easier to debug totally untested :-
>
> test_resv() {
>           local nvmedev=$1
>           local report_arg="--cdw11=1"
>           test_dev="/dev/${nvmedev}n1"
>
>           if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
>                   report_arg="--eds"
>           fi
>
>           echo "Register"
>           resv_report "${nvmedev}" "${report_arg}"
>           nvme resv-register "${test_dev}" --nrkey=4 --rrega=0
>           resv_report "${nvmedev}" "${report_arg}"
>
>           echo "Replace"
>           nvme resv-register "${test_dev}" --crkey=4 --nrkey=5 --rrega=2
>           resv_report "${nvmedev}" "${report_arg}"
>
>           echo "Unregister"
>           nvme resv-register "${test_dev}" --crkey=5 --rrega=1
>           resv_report "${nvmedev}" "${report_arg}"
>
>           echo "Acquire"
>           nvme resv-register "${test_dev}" --nrkey=4 --rrega=0
>           nvme resv-acquire "${test_dev}" --crkey=4 --rtype=1 --racqa=0
>           resv_report "${nvmedev}" "${report_arg}"
>
>           echo "Preempt"
>           nvme resv-acquire "${test_dev}" --crkey=4 --rtype=2 --racqa=1
>           resv_report "${nvmedev}" "${report_arg}"
>
>           echo "Release"
>           nvme resv-release "${test_dev}" --crkey=4 --rtype=2 --rrela=0
>           resv_report "${nvmedev}" "${report_arg}"
>
>           echo "Clear"
>           nvme resv-register "${test_dev}" --nrkey=4 --rrega=0
>           nvme resv-acquire "${test_dev}" --crkey=4 --rtype=1 --racqa=0
>           resv_report "${nvmedev}" "${report_arg}"
>           nvme resv-release "${test_dev}" --crkey=4 --rrela=1
> }
>
Thanks, changed in v4, and also change resv_report()'s firt param to

test_dev instead of nvmedev.

Best Regards,

Guixin Liu

> irrespective of that looks good :-
>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
>
> -ck
>
>
Guixin Liu Oct. 14, 2024, 8:43 a.m. UTC | #4 
在 2024/10/14 15:23, Daniel Wagner 写道:
> On Sat, Oct 12, 2024 at 07:11:57PM GMT, Guixin Liu wrote:
>> +requires() {
>> +	_nvme_requires
>> +}
> It's probably a good idea to test if the reservation feature is
> available, otherwise this test just fails on older kernels.

I checked the exists of resv_enable,

"if [[ -f "${ns_path}/resv_enable" ]] ; then", if not exist, skip this

test case.

>> +Running nvme/054
>> +Register
>> +
>> +NVME Reservation status:
>> +
>> +gen       : 0
>> +rtype     : 0
>> +regctl    : 0
>> +ptpls     : 0
>> +
>> +NVME Reservation  success
> I think this is a bit fragile, because the output of the nvme command
> might be extended or reformatted (obviously not on purpose but happens).
> I'd recommend to explicitly check for the expected values in the test.

OK, I will add a "grep -E "xxx|xxx|xxx" " to filter the result we cared now.

Best Regards,

Guixin Liu
diff mbox series

Patch

diff --git a/tests/nvme/054 b/tests/nvme/054
new file mode 100644
index 0000000..f352c73
--- /dev/null
+++ b/tests/nvme/054
@@ -0,0 +1,99 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024 Guixin Liu
+# Copyright (C) 2024 Alibaba Group.
+#
+# Test the NVMe reservation feature
+#
+. tests/nvme/rc
+
+DESCRIPTION="Test the NVMe reservation feature"
+QUICK=1
+nvme_trtype="loop"
+
+requires() {
+	_nvme_requires
+}
+
+resv_report() {
+	local nvmedev=$1
+	local report_arg=$2
+
+	nvme resv-report "/dev/${nvmedev}n1" "${report_arg}" | grep -v "hostid"
+}
+
+test_resv() {
+	local nvmedev=$1
+	local report_arg="--cdw11=1"
+
+	if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
+		report_arg="--eds"
+	fi
+
+	echo "Register"
+	resv_report "${nvmedev}" "${report_arg}"
+	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
+	resv_report "${nvmedev}" "${report_arg}"
+
+	echo "Replace"
+	nvme resv-register "/dev/${nvmedev}n1" --crkey=4 --nrkey=5 --rrega=2
+	resv_report "${nvmedev}" "${report_arg}"
+
+	echo "Unregister"
+	nvme resv-register "/dev/${nvmedev}n1" --crkey=5 --rrega=1
+	resv_report "${nvmedev}" "${report_arg}"
+
+	echo "Acquire"
+	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
+	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
+	resv_report "${nvmedev}" "${report_arg}"
+
+	echo "Preempt"
+	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --racqa=1
+	resv_report "${nvmedev}" "${report_arg}"
+
+	echo "Release"
+	nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --rrela=0
+	resv_report "${nvmedev}" "${report_arg}"
+
+	echo "Clear"
+	nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
+	nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
+	resv_report "${nvmedev}" "${report_arg}"
+	nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rrela=1
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	local nvmedev
+	local skipped=false
+	local subsys_path=""
+	local ns_path=""
+
+	_nvmet_target_setup --blkdev file --resv_enable
+	subsys_path="${NVMET_CFS}/subsystems/${def_subsysnqn}"
+	ns_path="${subsys_path}/namespaces/1"
+
+	if [[ -f "${ns_path}/resv_enable" ]] ; then
+		_nvme_connect_subsys
+
+		nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
+
+		test_resv "${nvmedev}"
+		_nvme_disconnect_subsys
+	else
+		SKIP_REASONS+=("missing reservation feature")
+		skipped=true
+	fi
+
+	_nvmet_target_cleanup
+
+	if [[ "${skipped}" = true ]] ; then
+		return 1
+	fi
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/054.out b/tests/nvme/054.out
new file mode 100644
index 0000000..66e51dd
--- /dev/null
+++ b/tests/nvme/054.out
@@ -0,0 +1,108 @@ 
+Running nvme/054
+Register
+
+NVME Reservation status:
+
+gen       : 0
+rtype     : 0
+regctl    : 0
+ptpls     : 0
+
+NVME Reservation  success
+
+NVME Reservation status:
+
+gen       : 1
+rtype     : 0
+regctl    : 1
+ptpls     : 0
+regctlext[0] :
+  cntlid     : ffff
+  rcsts      : 0
+  rkey       : 4
+
+Replace
+NVME Reservation  success
+
+NVME Reservation status:
+
+gen       : 2
+rtype     : 0
+regctl    : 1
+ptpls     : 0
+regctlext[0] :
+  cntlid     : ffff
+  rcsts      : 0
+  rkey       : 5
+
+Unregister
+NVME Reservation  success
+
+NVME Reservation status:
+
+gen       : 3
+rtype     : 0
+regctl    : 0
+ptpls     : 0
+
+Acquire
+NVME Reservation  success
+NVME Reservation Acquire success
+
+NVME Reservation status:
+
+gen       : 4
+rtype     : 1
+regctl    : 1
+ptpls     : 0
+regctlext[0] :
+  cntlid     : ffff
+  rcsts      : 1
+  rkey       : 4
+
+Preempt
+NVME Reservation Acquire success
+
+NVME Reservation status:
+
+gen       : 5
+rtype     : 2
+regctl    : 1
+ptpls     : 0
+regctlext[0] :
+  cntlid     : ffff
+  rcsts      : 1
+  rkey       : 4
+
+Release
+NVME Reservation Release success
+
+NVME Reservation status:
+
+gen       : 5
+rtype     : 0
+regctl    : 1
+ptpls     : 0
+regctlext[0] :
+  cntlid     : ffff
+  rcsts      : 0
+  rkey       : 4
+
+Clear
+NVME Reservation  success
+NVME Reservation Acquire success
+
+NVME Reservation status:
+
+gen       : 6
+rtype     : 1
+regctl    : 1
+ptpls     : 0
+regctlext[0] :
+  cntlid     : ffff
+  rcsts      : 1
+  rkey       : 4
+
+NVME Reservation Release success
+disconnected 1 controller(s)
+Test complete