* Re: [PATCH blktests] block/044: basic block error injection sanity test
2026-06-22 16:08 [PATCH blktests] block/044: basic block error injection sanity test Christoph Hellwig
@ 2026-06-25 6:36 ` Shin'ichiro Kawasaki
2026-06-25 11:54 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Shin'ichiro Kawasaki @ 2026-06-25 6:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-block
[-- 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
^ permalink raw reply related [flat|nested] 3+ messages in thread