[PATCH blktests] block/044: basic block error injection sanity test

[PATCH blktests] block/044: basic block error injection sanity test

[Christoph Hellwig]

Test the basic block layer error injection functionality.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 tests/block/044     | 71 +++++++++++++++++++++++++++++++++++++++++++++
 tests/block/044.out |  9 ++++++
 2 files changed, 80 insertions(+)
 create mode 100755 tests/block/044
 create mode 100644 tests/block/044.out

diff --git a/tests/block/044 b/tests/block/044
new file mode 100755
index 000000000000..8baf9fa59c68
--- /dev/null
+++ b/tests/block/044
@@ -0,0 +1,71 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Christoph Hellwig.
+#
+# Basic block error injection test.
+
+. tests/block/rc
+. common/scsi_debug
+
+DESCRIPTION="basic block error injection test"
+QUICK=1
+
+requires()
+{	
+	_have_loadable_scsi_debug
+	_have_program xfs_io
+}
+
+# load and remove scsi_debug once to test the static_key bug in the
+# initial commit
+_test_load_unload()
+{
+	if ! _init_scsi_debug dev_size_mb=500; then
+		return 1
+	fi
+
+	local dev=${SCSI_DEBUG_DEVICES[0]}
+	local debugfs_file="/sys/kernel/debug/block/$dev/error_injection"
+	if [[ ! -f "${debugfs_file}" ]]; then
+		SKIP_REASONS+=("error injection not supported")
+		_exit_scsi_debug
+		return 1
+	fi
+	echo "Testing unload without rules"
+	_exit_scsi_debug
+}
+
+_test_rules()
+{
+	if ! _init_scsi_debug dev_size_mb=500; then
+		return 1
+	fi
+
+	local dev=${SCSI_DEBUG_DEVICES[0]}
+	local debugfs_file="/sys/kernel/debug/block/$dev/error_injection"
+
+	echo "Testing valid rules"
+	echo "add,op=WRITE,status=RESOURCE,start=0,nr_sectors=8" > $debugfs_file
+	echo "add,op=READ,status=IOERR,start=16,nr_sectors=8" > $debugfs_file
+	xfs_io -d -c 'pwrite -q 0 4096' /dev/$dev
+	xfs_io -d -c 'pread -q 0 4096' /dev/$dev
+	xfs_io -d -c 'pwrite -q 4096 4096' /dev/$dev
+	xfs_io -d -c 'pread -q 8192 8192' /dev/$dev
+
+	echo "Testing invalid rules"
+	echo "op=READ,status=IOERR" > $debugfs_file
+	echo "add,op=READ,status=EIO,start=32" > $debugfs_file
+	_exit_scsi_debug
+}
+
+test()
+{
+	echo "Running ${TEST_NAME}"
+
+	local ret
+
+	_test_load_unload
+	_test_rules
+
+	echo "Test complete"
+}
diff --git a/tests/block/044.out b/tests/block/044.out
new file mode 100644
index 000000000000..92efcddf7c8e
--- /dev/null
+++ b/tests/block/044.out
@@ -0,0 +1,9 @@
+Running block/044
+Testing unload without rules
+Testing valid rules
+pwrite: Cannot allocate memory
+pread: Input/output error
+Testing invalid rules
+tests/block/044: line 56: echo: write error: Invalid argument
+tests/block/044: line 57: echo: write error: Invalid argument
+Test complete
-- 
2.53.0

Re: [PATCH blktests] block/044: basic block error injection sanity test

[Shin'ichiro Kawasaki]

[-- Attachment #1: Type: text/plain, Size: 3966 bytes --]

Hi Christoph, thanks for the patch. I ran the test case with block/for-next
kernel branch tip and confirmed that it is working as expected.

Please find my comments in line. FYI, I atttach the patch which reflects my
comments. If you are fine with the changes, please let me know so that I can
fold in the change and apply this patch.

On Jun 22, 2026 / 18:08, Christoph Hellwig wrote:
> Test the basic block layer error injection functionality.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  tests/block/044     | 71 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/block/044.out |  9 ++++++
>  2 files changed, 80 insertions(+)
>  create mode 100755 tests/block/044
>  create mode 100644 tests/block/044.out
> 
> diff --git a/tests/block/044 b/tests/block/044
> new file mode 100755
> index 000000000000..8baf9fa59c68
> --- /dev/null
> +++ b/tests/block/044
> @@ -0,0 +1,71 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0

Nit: Majority of the blktests test cases have GPL-3.0+. If you do not mind,
I suggest GPL-3.0+.

> +# Copyright (c) 2026 Christoph Hellwig.
> +#
> +# Basic block error injection test.
> +
> +. tests/block/rc
> +. common/scsi_debug
> +
> +DESCRIPTION="basic block error injection test"
> +QUICK=1
> +
> +requires()
> +{	

Nit: a stray tab in the line above.

I suggest to add the line below.

       _have_kernel_option BLK_ERROR_INJECTION

This way, we can confirm the kernel has the required changes and
the dependent feature is enabled.

> +	_have_loadable_scsi_debug
> +	_have_program xfs_io
> +}
> +
> +# load and remove scsi_debug once to test the static_key bug in the
> +# initial commit
> +_test_load_unload()
> +{
> +	if ! _init_scsi_debug dev_size_mb=500; then
> +		return 1
> +	fi
> +
> +	local dev=${SCSI_DEBUG_DEVICES[0]}
> +	local debugfs_file="/sys/kernel/debug/block/$dev/error_injection"
> +	if [[ ! -f "${debugfs_file}" ]]; then
> +		SKIP_REASONS+=("error injection not supported")
> +		_exit_scsi_debug
> +		return 1
> +	fi

With BLK_ERROR_INJECTION check in requires(), 7 lines above will not be
required. This will simplify this bash function.

> +	echo "Testing unload without rules"
> +	_exit_scsi_debug
> +}
> +
> +_test_rules()
> +{
> +	if ! _init_scsi_debug dev_size_mb=500; then
> +		return 1
> +	fi
> +
> +	local dev=${SCSI_DEBUG_DEVICES[0]}
> +	local debugfs_file="/sys/kernel/debug/block/$dev/error_injection"
> +
> +	echo "Testing valid rules"
> +	echo "add,op=WRITE,status=RESOURCE,start=0,nr_sectors=8" > $debugfs_file
> +	echo "add,op=READ,status=IOERR,start=16,nr_sectors=8" > $debugfs_file
> +	xfs_io -d -c 'pwrite -q 0 4096' /dev/$dev
> +	xfs_io -d -c 'pread -q 0 4096' /dev/$dev
> +	xfs_io -d -c 'pwrite -q 4096 4096' /dev/$dev
> +	xfs_io -d -c 'pread -q 8192 8192' /dev/$dev
> +
> +	echo "Testing invalid rules"
> +	echo "op=READ,status=IOERR" > $debugfs_file
> +	echo "add,op=READ,status=EIO,start=32" > $debugfs_file

$debugfs_file and $dev in this function require double quotation. Otherwise,
shellcheck will warn about them.

$ make check-parallel
Running shellcheck with 4 parallel jobs...
tests/block/044:48:61: note: Double quote to prevent globbing and word splitting. [SC2086]
tests/block/044:49:58: note: Double quote to prevent globbing and word splitting. [SC2086]
tests/block/044:50:39: note: Double quote to prevent globbing and word splitting. [SC2086]
tests/block/044:51:38: note: Double quote to prevent globbing and word splitting. [SC2086]
tests/block/044:52:42: note: Double quote to prevent globbing and word splitting. [SC2086]
tests/block/044:53:41: note: Double quote to prevent globbing and word splitting. [SC2086]
tests/block/044:56:32: note: Double quote to prevent globbing and word splitting. [SC2086]
tests/block/044:57:43: note: Double quote to prevent globbing and word splitting. [SC2086]

> +	_exit_scsi_debug
> +}
> +
> +test()
> +{
> +	echo "Running ${TEST_NAME}"
> +
> +	local ret
> +

The local variable declaration is not required.



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1885 bytes --]

diff --git a/tests/block/044 b/tests/block/044
index 8baf9fa..d31aa4c 100755
--- a/tests/block/044
+++ b/tests/block/044
@@ -11,7 +11,8 @@ DESCRIPTION="basic block error injection test"
 QUICK=1
 
 requires()
-{	
+{
+	_have_kernel_option BLK_ERROR_INJECTION
 	_have_loadable_scsi_debug
 	_have_program xfs_io
 }
@@ -23,14 +24,6 @@ _test_load_unload()
 	if ! _init_scsi_debug dev_size_mb=500; then
 		return 1
 	fi
-
-	local dev=${SCSI_DEBUG_DEVICES[0]}
-	local debugfs_file="/sys/kernel/debug/block/$dev/error_injection"
-	if [[ ! -f "${debugfs_file}" ]]; then
-		SKIP_REASONS+=("error injection not supported")
-		_exit_scsi_debug
-		return 1
-	fi
 	echo "Testing unload without rules"
 	_exit_scsi_debug
 }
@@ -45,16 +38,16 @@ _test_rules()
 	local debugfs_file="/sys/kernel/debug/block/$dev/error_injection"
 
 	echo "Testing valid rules"
-	echo "add,op=WRITE,status=RESOURCE,start=0,nr_sectors=8" > $debugfs_file
-	echo "add,op=READ,status=IOERR,start=16,nr_sectors=8" > $debugfs_file
-	xfs_io -d -c 'pwrite -q 0 4096' /dev/$dev
-	xfs_io -d -c 'pread -q 0 4096' /dev/$dev
-	xfs_io -d -c 'pwrite -q 4096 4096' /dev/$dev
-	xfs_io -d -c 'pread -q 8192 8192' /dev/$dev
+	echo "add,op=WRITE,status=RESOURCE,start=0,nr_sectors=8" > "$debugfs_file"
+	echo "add,op=READ,status=IOERR,start=16,nr_sectors=8" > "$debugfs_file"
+	xfs_io -d -c 'pwrite -q 0 4096' /dev/"$dev"
+	xfs_io -d -c 'pread -q 0 4096' /dev/"$dev"
+	xfs_io -d -c 'pwrite -q 4096 4096' /dev/"$dev"
+	xfs_io -d -c 'pread -q 8192 8192' /dev/"$dev"
 
 	echo "Testing invalid rules"
-	echo "op=READ,status=IOERR" > $debugfs_file
-	echo "add,op=READ,status=EIO,start=32" > $debugfs_file
+	echo "op=READ,status=IOERR" > "$debugfs_file"
+	echo "add,op=READ,status=EIO,start=32" > "$debugfs_file"
 	_exit_scsi_debug
 }
 
@@ -62,8 +55,6 @@ test()
 {
 	echo "Running ${TEST_NAME}"
 
-	local ret
-
 	_test_load_unload
 	_test_rules
 

Re: [PATCH blktests] block/044: basic block error injection sanity test

[Christoph Hellwig]

On Thu, Jun 25, 2026 at 03:36:19PM +0900, Shin'ichiro Kawasaki wrote:
> Hi Christoph, thanks for the patch. I ran the test case with block/for-next
> kernel branch tip and confirmed that it is working as expected.
> 
> Please find my comments in line. FYI, I atttach the patch which reflects my
> comments. If you are fine with the changes, please let me know so that I can
> fold in the change and apply this patch.

> > +# SPDX-License-Identifier: GPL-2.0
> 
> Nit: Majority of the blktests test cases have GPL-3.0+. If you do not mind,
> I suggest GPL-3.0+.

Well a mix of licenses is obviously bad, although I hate the GPL 3 with
passion.

> I suggest to add the line below.
> 
>        _have_kernel_option BLK_ERROR_INJECTION
> 
> This way, we can confirm the kernel has the required changes and
> the dependent feature is enabled.

Sounds good, although this assumes we actually have /proc/config.gz?

Otherwise the changes looks fine.

Re: [PATCH blktests] block/044: basic block error injection sanity test

[Shin'ichiro Kawasaki]

On Jun 25, 2026 / 13:54, Christoph Hellwig wrote:
> On Thu, Jun 25, 2026 at 03:36:19PM +0900, Shin'ichiro Kawasaki wrote:
> > Hi Christoph, thanks for the patch. I ran the test case with block/for-next
> > kernel branch tip and confirmed that it is working as expected.
> > 
> > Please find my comments in line. FYI, I atttach the patch which reflects my
> > comments. If you are fine with the changes, please let me know so that I can
> > fold in the change and apply this patch.
> 
> > > +# SPDX-License-Identifier: GPL-2.0
> > 
> > Nit: Majority of the blktests test cases have GPL-3.0+. If you do not mind,
> > I suggest GPL-3.0+.
> 
> Well a mix of licenses is obviously bad, although I hate the GPL 3 with
> passion.

I see, I respoect author's choice.

> 
> > I suggest to add the line below.
> > 
> >        _have_kernel_option BLK_ERROR_INJECTION
> > 
> > This way, we can confirm the kernel has the required changes and
> > the dependent feature is enabled.
> 
> Sounds good, although this assumes we actually have /proc/config.gz?

Blktests checks both /proc/config.gz and /boot/config-$(uname -r), then I
think this to work on all test systems.

> 
> Otherwise the changes looks fine.

Re: [PATCH blktests] block/044: basic block error injection sanity test

[Christoph Hellwig]

On Fri, Jun 26, 2026 at 01:31:06PM +0900, Shin'ichiro Kawasaki wrote:
> > > Nit: Majority of the blktests test cases have GPL-3.0+. If you do not mind,
> > > I suggest GPL-3.0+.
> > 
> > Well a mix of licenses is obviously bad, although I hate the GPL 3 with
> > passion.
> 
> I see, I respoect author's choice.

Well, if the common bits are GPLv3+ we can't actually legally combine
them with test that have a pure GPLv2 license.  So I need at least
GPL-2.0+.

And we need to as a few authors to also fix this up for:

block/029
block/030
block/031
block/040
block/042
block/043
block/044