[PATCH blktests] dm: add a regression test

[PATCH blktests] dm: add a regression test

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

Verify that reload a dm with itself won't crash the kernel.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 tests/dm/001     | 27 +++++++++++++++++++++++++++
 tests/dm/001.out |  4 ++++
 tests/dm/rc      | 11 +++++++++++
 3 files changed, 42 insertions(+)
 create mode 100755 tests/dm/001
 create mode 100644 tests/dm/001.out
 create mode 100644 tests/dm/rc

diff --git a/tests/dm/001 b/tests/dm/001
new file mode 100755
index 0000000..55e07f3
--- /dev/null
+++ b/tests/dm/001
@@ -0,0 +1,27 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2022 Yu Kuai
+#
+# Regression test for commit 077a4033541f ("block: don't allow a disk link
+# holder to itself")
+
+. tests/dm/rc
+
+DESCRIPTION="reload a dm with itself"
+QUICK=1
+
+requires() {
+	_have_program dmsetup && _have_driver dm-mod
+}
+
+test_device() {
+	echo "Running ${TEST_NAME}"
+
+	dmsetup create test --table "0 8192 linear ${TEST_DEV} 0"
+	dmsetup suspend test
+	dmsetup reload test --table "0 8192 linear /dev/mapper/test 0"
+	dmsetup resume test
+	dmsetup remove test
+
+	echo "Test complete"
+}
diff --git a/tests/dm/001.out b/tests/dm/001.out
new file mode 100644
index 0000000..23cf1f0
--- /dev/null
+++ b/tests/dm/001.out
@@ -0,0 +1,4 @@
+Running dm/001
+device-mapper: reload ioctl on test failed: Invalid argument
+Command failed
+Test complete
diff --git a/tests/dm/rc b/tests/dm/rc
new file mode 100644
index 0000000..e1ad6c6
--- /dev/null
+++ b/tests/dm/rc
@@ -0,0 +1,11 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2022 Yu Kuai
+#
+# TODO: provide a brief description of the group here.
+
+. common/rc
+
+group_requires() {
+	_have_root
+}
-- 
2.31.1

Re: [PATCH blktests] dm: add a regression test

[kernel test robot]

Hi Yu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.2-rc1 next-20221226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yu-Kuai/dm-add-a-regression-test/20221230-143530
patch link:    https://lore.kernel.org/r/20221230065424.19998-1-yukuai1%40huaweicloud.com
patch subject: [PATCH blktests] dm: add a regression test
reproduce:
        scripts/spdxcheck.py

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

spdxcheck warnings: (new ones prefixed by >>)
>> tests/dm/001: 2:27 Invalid License ID: GPL-3.0+
>> tests/dm/rc: 2:27 Invalid License ID: GPL-3.0+

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH blktests] dm: add a regression test

[Shinichiro Kawasaki]

Hello Yu, thanks for the patch. I think it is good to have this test case to
avoid repeating the same regression. Please find my comments in line.

CC+: Mike, dm-devel,

Mike, could you take a look in this new test case? It adds "dm" test group to
blktests. If you have thoughts on how to add device-mapper related test cases
to blktests, please share (Or we may be able to discuss later at LSF 2023).

On Dec 30, 2022 / 14:54, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Verify that reload a dm with itself won't crash the kernel.

With this test case, I observed the system crash on Fedora default kernel
6.0.18-300.fc37. Will the kernel fix be delivered to stable kernels? If not,
it would be the better to require kernel version 6.2 for this test case not
to surprise the blktests users.

Regarding the wording, "reload a dm with itself" is a bit confusing for me.
How about "reload a dm with maps to itself"?

> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  tests/dm/001     | 27 +++++++++++++++++++++++++++
>  tests/dm/001.out |  4 ++++
>  tests/dm/rc      | 11 +++++++++++
>  3 files changed, 42 insertions(+)
>  create mode 100755 tests/dm/001
>  create mode 100644 tests/dm/001.out
>  create mode 100644 tests/dm/rc
> 
> diff --git a/tests/dm/001 b/tests/dm/001
> new file mode 100755
> index 0000000..55e07f3
> --- /dev/null
> +++ b/tests/dm/001
> @@ -0,0 +1,27 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2022 Yu Kuai
> +#
> +# Regression test for commit 077a4033541f ("block: don't allow a disk link
> +# holder to itself")
> +
> +. tests/dm/rc
> +
> +DESCRIPTION="reload a dm with itself"

Same comment on wording as the commit message.

> +QUICK=1
> +
> +requires() {
> +	_have_program dmsetup && _have_driver dm-mod

I suggest to move these checks to group_requires in dm/rc, since future new test
cases in dm group will require them also.

Nit: '&&' is no longer required to check multiple requirements. Just,

	_have_program dmsetup
	_have_driver dm-mod

is fine and a bit better.

> +}
> +
> +test_device() {
> +	echo "Running ${TEST_NAME}"
> +
> +	dmsetup create test --table "0 8192 linear ${TEST_DEV} 0"
> +	dmsetup suspend test
> +	dmsetup reload test --table "0 8192 linear /dev/mapper/test 0"
> +	dmsetup resume test
> +	dmsetup remove test
> +
> +	echo "Test complete"
> +}
> diff --git a/tests/dm/001.out b/tests/dm/001.out
> new file mode 100644
> index 0000000..23cf1f0
> --- /dev/null
> +++ b/tests/dm/001.out
> @@ -0,0 +1,4 @@
> +Running dm/001
> +device-mapper: reload ioctl on test failed: Invalid argument
> +Command failed
> +Test complete

With my test system and kernel v6.2-rc3, the messages above were slightly
diffrent. To make the test case pass, I needed changes below:

diff --git a/tests/dm/001.out b/tests/dm/001.out
index 23cf1f0..543b1db 100644
--- a/tests/dm/001.out
+++ b/tests/dm/001.out
@@ -1,4 +1,4 @@
 Running dm/001
-device-mapper: reload ioctl on test failed: Invalid argument
-Command failed
+device-mapper: reload ioctl on test  failed: Invalid argument
+Command failed.
 Test complete


> diff --git a/tests/dm/rc b/tests/dm/rc
> new file mode 100644
> index 0000000..e1ad6c6
> --- /dev/null
> +++ b/tests/dm/rc
> @@ -0,0 +1,11 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2022 Yu Kuai
> +#
> +# TODO: provide a brief description of the group here.

Let's not leave this TODO. How about "# Tests for device-mapper."?

> +
> +. common/rc
> +
> +group_requires() {
> +	_have_root
> +}
> -- 
> 2.31.1
> 

-- 
Shin'ichiro Kawasaki

Re: [PATCH blktests] dm: add a regression test

[Yu Kuai]

Hi,

在 2023/01/12 9:05, Shinichiro Kawasaki 写道:
> Hello Yu, thanks for the patch. I think it is good to have this test case to
> avoid repeating the same regression. Please find my comments in line.
> 
> CC+: Mike, dm-devel,
> 
> Mike, could you take a look in this new test case? It adds "dm" test group to
> blktests. If you have thoughts on how to add device-mapper related test cases
> to blktests, please share (Or we may be able to discuss later at LSF 2023).

Can we add "dm" test group to blktests? I'll send a new version if we
can.

Thanks,
Kuai
> 
> On Dec 30, 2022 / 14:54, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Verify that reload a dm with itself won't crash the kernel.
> 
> With this test case, I observed the system crash on Fedora default kernel
> 6.0.18-300.fc37. Will the kernel fix be delivered to stable kernels? If not,
> it would be the better to require kernel version 6.2 for this test case not
> to surprise the blktests users.
> 
> Regarding the wording, "reload a dm with itself" is a bit confusing for me.
> How about "reload a dm with maps to itself"?
> 
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   tests/dm/001     | 27 +++++++++++++++++++++++++++
>>   tests/dm/001.out |  4 ++++
>>   tests/dm/rc      | 11 +++++++++++
>>   3 files changed, 42 insertions(+)
>>   create mode 100755 tests/dm/001
>>   create mode 100644 tests/dm/001.out
>>   create mode 100644 tests/dm/rc
>>
>> diff --git a/tests/dm/001 b/tests/dm/001
>> new file mode 100755
>> index 0000000..55e07f3
>> --- /dev/null
>> +++ b/tests/dm/001
>> @@ -0,0 +1,27 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2022 Yu Kuai
>> +#
>> +# Regression test for commit 077a4033541f ("block: don't allow a disk link
>> +# holder to itself")
>> +
>> +. tests/dm/rc
>> +
>> +DESCRIPTION="reload a dm with itself"
> 
> Same comment on wording as the commit message.
> 
>> +QUICK=1
>> +
>> +requires() {
>> +	_have_program dmsetup && _have_driver dm-mod
> 
> I suggest to move these checks to group_requires in dm/rc, since future new test
> cases in dm group will require them also.
> 
> Nit: '&&' is no longer required to check multiple requirements. Just,
> 
> 	_have_program dmsetup
> 	_have_driver dm-mod
> 
> is fine and a bit better.
> 
>> +}
>> +
>> +test_device() {
>> +	echo "Running ${TEST_NAME}"
>> +
>> +	dmsetup create test --table "0 8192 linear ${TEST_DEV} 0"
>> +	dmsetup suspend test
>> +	dmsetup reload test --table "0 8192 linear /dev/mapper/test 0"
>> +	dmsetup resume test
>> +	dmsetup remove test
>> +
>> +	echo "Test complete"
>> +}
>> diff --git a/tests/dm/001.out b/tests/dm/001.out
>> new file mode 100644
>> index 0000000..23cf1f0
>> --- /dev/null
>> +++ b/tests/dm/001.out
>> @@ -0,0 +1,4 @@
>> +Running dm/001
>> +device-mapper: reload ioctl on test failed: Invalid argument
>> +Command failed
>> +Test complete
> 
> With my test system and kernel v6.2-rc3, the messages above were slightly
> diffrent. To make the test case pass, I needed changes below:
> 
> diff --git a/tests/dm/001.out b/tests/dm/001.out
> index 23cf1f0..543b1db 100644
> --- a/tests/dm/001.out
> +++ b/tests/dm/001.out
> @@ -1,4 +1,4 @@
>   Running dm/001
> -device-mapper: reload ioctl on test failed: Invalid argument
> -Command failed
> +device-mapper: reload ioctl on test  failed: Invalid argument
> +Command failed.
>   Test complete
> 
> 
>> diff --git a/tests/dm/rc b/tests/dm/rc
>> new file mode 100644
>> index 0000000..e1ad6c6
>> --- /dev/null
>> +++ b/tests/dm/rc
>> @@ -0,0 +1,11 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2022 Yu Kuai
>> +#
>> +# TODO: provide a brief description of the group here.
> 
> Let's not leave this TODO. How about "# Tests for device-mapper."?
> 
>> +
>> +. common/rc
>> +
>> +group_requires() {
>> +	_have_root
>> +}
>> -- 
>> 2.31.1
>>
> 

Re: [PATCH blktests] dm: add a regression test

[Shin'ichiro Kawasaki]

On Apr 25, 2023 / 16:22, Yu Kuai wrote:
> Hi,
> 
> 在 2023/01/12 9:05, Shinichiro Kawasaki 写道:
> > Hello Yu, thanks for the patch. I think it is good to have this test case to
> > avoid repeating the same regression. Please find my comments in line.
> > 
> > CC+: Mike, dm-devel,
> > 
> > Mike, could you take a look in this new test case? It adds "dm" test group to
> > blktests. If you have thoughts on how to add device-mapper related test cases
> > to blktests, please share (Or we may be able to discuss later at LSF 2023).
> 
> Can we add "dm" test group to blktests? I'll send a new version if we
> can.

I suggest to wait for LSF discussion in May, which could be a good chance to
hear opinions of dm experts. I think your new test case is valuable, so IMO it
should be added to the new "dm" group, or at least to the existing "block"
group. Let's decide the target group after LSF.

Re: [PATCH blktests] dm: add a regression test

[Mike Snitzer]

On Tue, Apr 25 2023 at  8:15P -0400,
Shin'ichiro Kawasaki <shinichiro@fastmail.com> wrote:

> On Apr 25, 2023 / 16:22, Yu Kuai wrote:
> > Hi,
> > 
> > 在 2023/01/12 9:05, Shinichiro Kawasaki 写道:
> > > Hello Yu, thanks for the patch. I think it is good to have this test case to
> > > avoid repeating the same regression. Please find my comments in line.
> > > 
> > > CC+: Mike, dm-devel,
> > > 
> > > Mike, could you take a look in this new test case? It adds "dm" test group to
> > > blktests. If you have thoughts on how to add device-mapper related test cases
> > > to blktests, please share (Or we may be able to discuss later at LSF 2023).
> > 
> > Can we add "dm" test group to blktests? I'll send a new version if we
> > can.
> 
> I suggest to wait for LSF discussion in May, which could be a good chance to
> hear opinions of dm experts. I think your new test case is valuable, so IMO it
> should be added to the new "dm" group, or at least to the existing "block"
> group. Let's decide the target group after LSF.
> 

It's obviously fine to add a new "dm" test group to blktests.

But just so others are aware: more elaborate dm testing is currently
spread across multiple testsuites (e.g. lvm2, cryptsetup, mptest,
device-mapper-test-suite, etc).

There is new effort to port device-mapper-test-suite tests (which use
ruby) to a new python harness currently named "dmtest-python", Joe
Thornber is leading this effort (with the assistance of
ChatGPT.. apparently it has been wonderful in helping Joe glue python
code together to accomplish anything he's needed):
https://github.com/jthornber/dmtest-python

(we've discussed renaming "dmtest-python" to "dmtests")

I've also discussed with Joe the plan to wrap the other disparate
testsuites with DM coverage in terms of the new dmtest-python.
blktests can be made to be one of the testsuites we add support for
(so that all blktests run on various types of DM targets).

Really all we need is a means to:
1) list all tests in a testsuite
2) initiate running each test individually

Mike

Re: [PATCH blktests] dm: add a regression test

[Chaitanya Kulkarni]

On 4/25/23 10:24, Mike Snitzer wrote:
> On Tue, Apr 25 2023 at  8:15P -0400,
> Shin'ichiro Kawasaki <shinichiro@fastmail.com> wrote:
>
>> On Apr 25, 2023 / 16:22, Yu Kuai wrote:
>>> Hi,
>>>
>>> 在 2023/01/12 9:05, Shinichiro Kawasaki 写道:
>>>> Hello Yu, thanks for the patch. I think it is good to have this test case to
>>>> avoid repeating the same regression. Please find my comments in line.
>>>>
>>>> CC+: Mike, dm-devel,
>>>>
>>>> Mike, could you take a look in this new test case? It adds "dm" test group to
>>>> blktests. If you have thoughts on how to add device-mapper related test cases
>>>> to blktests, please share (Or we may be able to discuss later at LSF 2023).
>>> Can we add "dm" test group to blktests? I'll send a new version if we
>>> can.
>> I suggest to wait for LSF discussion in May, which could be a good chance to
>> hear opinions of dm experts. I think your new test case is valuable, so IMO it
>> should be added to the new "dm" group, or at least to the existing "block"
>> group. Let's decide the target group after LSF.
>>
> It's obviously fine to add a new "dm" test group to blktests.
>
> But just so others are aware: more elaborate dm testing is currently
> spread across multiple testsuites (e.g. lvm2, cryptsetup, mptest,
> device-mapper-test-suite, etc).
>
> There is new effort to port device-mapper-test-suite tests (which use
> ruby) to a new python harness currently named "dmtest-python", Joe
> Thornber is leading this effort (with the assistance of
> ChatGPT.. apparently it has been wonderful in helping Joe glue python
> code together to accomplish anything he's needed):
> https://github.com/jthornber/dmtest-python
>
> (we've discussed renaming "dmtest-python" to "dmtests")
>
> I've also discussed with Joe the plan to wrap the other disparate
> testsuites with DM coverage in terms of the new dmtest-python.
> blktests can be made to be one of the testsuites we add support for
> (so that all blktests run on various types of DM targets).
>
> Really all we need is a means to:
> 1) list all tests in a testsuite
> 2) initiate running each test individually
>
> Mike

Thanks Mike for the detailed information, we did talk about DM testcases
in last LSFMM, this is really important piece of blktest that is missing
and need to be discussed this year's LSFMM so we can integrate above
work in blktests as right now we are not able to establish complete
stability due to lack of of the dm tests as we are doing it for block
layer code or nvme for example.

-ck

Re: [PATCH blktests] dm: add a regression test

[Shin'ichiro Kawasaki]

On Apr 26, 2023 / 08:42, Chaitanya Kulkarni wrote:
> On 4/25/23 10:24, Mike Snitzer wrote:
> > On Tue, Apr 25 2023 at  8:15P -0400,
> > Shin'ichiro Kawasaki <shinichiro@fastmail.com> wrote:
> >
> >> On Apr 25, 2023 / 16:22, Yu Kuai wrote:
> >>> Hi,
> >>>
> >>> 在 2023/01/12 9:05, Shinichiro Kawasaki 写道:
> >>>> Hello Yu, thanks for the patch. I think it is good to have this test case to
> >>>> avoid repeating the same regression. Please find my comments in line.
> >>>>
> >>>> CC+: Mike, dm-devel,
> >>>>
> >>>> Mike, could you take a look in this new test case? It adds "dm" test group to
> >>>> blktests. If you have thoughts on how to add device-mapper related test cases
> >>>> to blktests, please share (Or we may be able to discuss later at LSF 2023).
> >>> Can we add "dm" test group to blktests? I'll send a new version if we
> >>> can.
> >> I suggest to wait for LSF discussion in May, which could be a good chance to
> >> hear opinions of dm experts. I think your new test case is valuable, so IMO it
> >> should be added to the new "dm" group, or at least to the existing "block"
> >> group. Let's decide the target group after LSF.
> >>
> > It's obviously fine to add a new "dm" test group to blktests.

Mike, thanks for the positive comment. Now I know that there are various
testsuites related to dm as you noted below. I think the new dm test group in
blktests can have different coverage from other dm related testsuites: tests to
confirm fixes for failures related to block layer and device-mapper. The new
test Yu Kuai suggests will fit into it.

Yu, could you post the new version of your patch? Let's add the dm group and the
first test case.

> >
> > But just so others are aware: more elaborate dm testing is currently
> > spread across multiple testsuites (e.g. lvm2, cryptsetup, mptest,
> > device-mapper-test-suite, etc).

Thanks. Good to know that dm test has wide varieties. I found the testsuites on
the net. Will try to look into them.

[1] lvm2: https://github.com/lvmteam/lvm2/tree/master/test
[2] cryptsetup: https://gitlab.com/cryptsetup/cryptsetup/-/tree/main/tests
[3] mptest: https://github.com/snitm/mptest
[4] device-mapper-test-suite: https://github.com/jthornber/device-mapper-test-suite

> >
> > There is new effort to port device-mapper-test-suite tests (which use
> > ruby) to a new python harness currently named "dmtest-python", Joe
> > Thornber is leading this effort (with the assistance of
> > ChatGPT.. apparently it has been wonderful in helping Joe glue python
> > code together to accomplish anything he's needed):
> > https://github.com/jthornber/dmtest-python
> >
> > (we've discussed renaming "dmtest-python" to "dmtests")
> >
> > I've also discussed with Joe the plan to wrap the other disparate
> > testsuites with DM coverage in terms of the new dmtest-python.

It sounds a valuable action to gather the various testsuites so that the dm
maintainer or dm developers can run them all easily.

I also think that the list of testsuites will help developers to tell what kind
of test goes to which testsuite. When an developer adds a test case related to
dm, the developer needs to choose which testsuite to add it. As an example, I
wonder about dm-zoned. In case we would add test cases to exercise dm-zoned, is
there any good testsuite to add them? (Can we set up dm-zoned with dmtest-
python?)

> > blktests can be made to be one of the testsuites we add support for
> > (so that all blktests run on various types of DM targets).
> >
> > Really all we need is a means to:
> > 1) list all tests in a testsuite
> > 2) initiate running each test individually

IIUC, the dmtest-python takes the role to prepare the various DM targets for
test, and each testsuite runs test cases on the targets, right? From blktests
point of view, the "block" group of blktests can be run with the various DM
targets. Also the "zbd" group of blktests can be run with DM targets with
DM_TARGET_ZONED_HM feature. Other test groups such as "nvme" or "scsi" won't
run with DM targets. I think it is not difficult for dmtest-python to prepare
blktests config files to specify DM targets and the block/zbd test group. If
blktests side change is desired, we can discuss.

> >
> > Mike
> 
> Thanks Mike for the detailed information, we did talk about DM testcases
> in last LSFMM, this is really important piece of blktest that is missing
> and need to be discussed this year's LSFMM so we can integrate above
> work in blktests as right now we are not able to establish complete
> stability due to lack of of the dm tests as we are doing it for block
> layer code or nvme for example.
> 
> -ck
> 
>