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@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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
