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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux