* [PATCH] common/rc: explicitly test for engine availability in _require_fio
@ 2025-03-18 10:08 Zorro Lang
2025-03-18 13:23 ` Zorro Lang
0 siblings, 1 reply; 5+ messages in thread
From: Zorro Lang @ 2025-03-18 10:08 UTC (permalink / raw)
To: fstests; +Cc: djwong
From: Eric Sandeen <sandeen@sandeen.net>
The current test in _require_fio (--warnings-fatal --showcmd) does not
fail if an invalid/unavailable io engine is specified.
Add an explicit test that every requested io engine in the job file
is actually available.
Also, remove the "ioe_e4defrag" entries in the [global] stanza of several
ext4 tests which use fio jobfiles. While ioengines can be set in the
[global] section, they can also be overridden in individual, subsequent
stanzas. In each affected test (ext4/301, ext4/302, ext4/303, and
ext4/304) every individual stanza after [global]re-specifies an ioengine;
either with ioengine=e4defrag or ioengine=libaio.
Because of this re-specification, the ioengine in the [global] section
is ignored. This is a good thing, because ioe_e4defrag is not a valid
ioengine, and would fail this new hand-rolled check, even though fio
did not complain.
So rather than over-complicate this new check, simply remove the unused,
invalid "ioengine=ioe_e4defrag" lines in these tests.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Zorro Lang <zlang@redhat.com>
Signed-off-by: Zorro Lang <zlang@kernel.org>
---
common/rc | 7 +++++++
tests/ext4/301 | 1 -
tests/ext4/302 | 1 -
tests/ext4/303 | 1 -
tests/ext4/304 | 1 -
5 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/common/rc b/common/rc
index dcdfa86e4..e51686389 100644
--- a/common/rc
+++ b/common/rc
@@ -3985,12 +3985,19 @@ _require_scratch_dev_pool_equal_size()
_require_fio()
{
local job=$1
+ local eng
_require_command "$FIO_PROG" fio
if [ -z "$1" ]; then
return 1;
fi
+ # Explicitly check for every ioengine listed in the job
+ for eng in `sed -n "s/ioengine=\(.*\)/\1/p" $job | sort -u`; do
+ grep -wq $eng <($FIO_PROG --enghelp 2>/dev/null) || \
+ _notrun "fio engine $eng not available"
+ done
+
$FIO_PROG --warnings-fatal --showcmd $job >> $seqres.full 2>&1
[ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
}
diff --git a/tests/ext4/301 b/tests/ext4/301
index abf47d4b7..a05b8e8a8 100755
--- a/tests/ext4/301
+++ b/tests/ext4/301
@@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
diff --git a/tests/ext4/302 b/tests/ext4/302
index 87820184e..e0f5f2f98 100755
--- a/tests/ext4/302
+++ b/tests/ext4/302
@@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
diff --git a/tests/ext4/303 b/tests/ext4/303
index 2381f0477..0a83e86cd 100755
--- a/tests/ext4/303
+++ b/tests/ext4/303
@@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (3+1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
diff --git a/tests/ext4/304 b/tests/ext4/304
index 53b522ee8..5f2ae4bdc 100755
--- a/tests/ext4/304
+++ b/tests/ext4/304
@@ -32,7 +32,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
--
2.47.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] common/rc: explicitly test for engine availability in _require_fio
2025-03-18 10:08 [PATCH] common/rc: explicitly test for engine availability in _require_fio Zorro Lang
@ 2025-03-18 13:23 ` Zorro Lang
0 siblings, 0 replies; 5+ messages in thread
From: Zorro Lang @ 2025-03-18 13:23 UTC (permalink / raw)
To: fstests; +Cc: djwong
On Tue, Mar 18, 2025 at 06:08:48PM +0800, Zorro Lang wrote:
> From: Eric Sandeen <sandeen@sandeen.net>
>
> The current test in _require_fio (--warnings-fatal --showcmd) does not
> fail if an invalid/unavailable io engine is specified.
>
> Add an explicit test that every requested io engine in the job file
> is actually available.
>
> Also, remove the "ioe_e4defrag" entries in the [global] stanza of several
> ext4 tests which use fio jobfiles. While ioengines can be set in the
> [global] section, they can also be overridden in individual, subsequent
> stanzas. In each affected test (ext4/301, ext4/302, ext4/303, and
> ext4/304) every individual stanza after [global]re-specifies an ioengine;
> either with ioengine=e4defrag or ioengine=libaio.
>
> Because of this re-specification, the ioengine in the [global] section
> is ignored. This is a good thing, because ioe_e4defrag is not a valid
> ioengine, and would fail this new hand-rolled check, even though fio
> did not complain.
>
> So rather than over-complicate this new check, simply remove the unused,
> invalid "ioengine=ioe_e4defrag" lines in these tests.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> Signed-off-by: Zorro Lang <zlang@kernel.org>
> ---
Please ignore this patch, I send wrong commit...
Thanks,
Zorro
> common/rc | 7 +++++++
> tests/ext4/301 | 1 -
> tests/ext4/302 | 1 -
> tests/ext4/303 | 1 -
> tests/ext4/304 | 1 -
> 5 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index dcdfa86e4..e51686389 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3985,12 +3985,19 @@ _require_scratch_dev_pool_equal_size()
> _require_fio()
> {
> local job=$1
> + local eng
>
> _require_command "$FIO_PROG" fio
> if [ -z "$1" ]; then
> return 1;
> fi
>
> + # Explicitly check for every ioengine listed in the job
> + for eng in `sed -n "s/ioengine=\(.*\)/\1/p" $job | sort -u`; do
> + grep -wq $eng <($FIO_PROG --enghelp 2>/dev/null) || \
> + _notrun "fio engine $eng not available"
> + done
> +
> $FIO_PROG --warnings-fatal --showcmd $job >> $seqres.full 2>&1
> [ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
> }
> diff --git a/tests/ext4/301 b/tests/ext4/301
> index abf47d4b7..a05b8e8a8 100755
> --- a/tests/ext4/301
> +++ b/tests/ext4/301
> @@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
> diff --git a/tests/ext4/302 b/tests/ext4/302
> index 87820184e..e0f5f2f98 100755
> --- a/tests/ext4/302
> +++ b/tests/ext4/302
> @@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
> diff --git a/tests/ext4/303 b/tests/ext4/303
> index 2381f0477..0a83e86cd 100755
> --- a/tests/ext4/303
> +++ b/tests/ext4/303
> @@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (3+1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
> diff --git a/tests/ext4/304 b/tests/ext4/304
> index 53b522ee8..5f2ae4bdc 100755
> --- a/tests/ext4/304
> +++ b/tests/ext4/304
> @@ -32,7 +32,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
> --
> 2.47.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] common/rc: explicitly test for engine availability in _require_fio
@ 2025-03-12 16:48 Eric Sandeen
2025-03-14 14:36 ` Zorro Lang
0 siblings, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2025-03-12 16:48 UTC (permalink / raw)
To: fstests@vger.kernel.org; +Cc: Jeff Moyer
The current test in _require_fio (--warnings-fatal --showcmd) does not
fail if an invalid/unavailable io engine is specified.
Add an explicit test that every requested io engine in the job file
is actually available.
Remove the "ioe_e4defrag" entries in ext4 tests - an engine with this
name has seemingly never existed, but in each case later stanzas
overrode the io engine, so it did not cause problems without this
explicit parsing and checking.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/common/rc b/common/rc
index ca755055..c155bb46 100644
--- a/common/rc
+++ b/common/rc
@@ -3983,6 +3983,12 @@ _require_fio()
return 1;
fi
+ # Explicitly check for every ioengine availability
+ for ENGINE in `grep ioengine= $job | awk -F= '{print $2}'; sort`; do
+ fio --enghelp | grep -qw $ENGINE || \
+ _notrun "fio engine $ENGINE not available"
+ done
+
$FIO_PROG --warnings-fatal --showcmd $job >> $seqres.full 2>&1
[ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
}
diff --git a/tests/ext4/301 b/tests/ext4/301
index abf47d4b..a05b8e8a 100755
--- a/tests/ext4/301
+++ b/tests/ext4/301
@@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
diff --git a/tests/ext4/302 b/tests/ext4/302
index 87820184..e0f5f2f9 100755
--- a/tests/ext4/302
+++ b/tests/ext4/302
@@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
diff --git a/tests/ext4/303 b/tests/ext4/303
index 2381f047..0a83e86c 100755
--- a/tests/ext4/303
+++ b/tests/ext4/303
@@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (3+1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
diff --git a/tests/ext4/304 b/tests/ext4/304
index 53b522ee..5f2ae4bd 100755
--- a/tests/ext4/304
+++ b/tests/ext4/304
@@ -32,7 +32,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
cat >$fio_config <<EOF
# Common e4defrag regression tests
[global]
-ioengine=ioe_e4defrag
iodepth=1
directory=${SCRATCH_MNT}
filesize=${FILE_SIZE}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] common/rc: explicitly test for engine availability in _require_fio
2025-03-12 16:48 Eric Sandeen
@ 2025-03-14 14:36 ` Zorro Lang
2025-03-14 14:39 ` Eric Sandeen
0 siblings, 1 reply; 5+ messages in thread
From: Zorro Lang @ 2025-03-14 14:36 UTC (permalink / raw)
To: Eric Sandeen; +Cc: fstests@vger.kernel.org, Jeff Moyer
On Wed, Mar 12, 2025 at 11:48:11AM -0500, Eric Sandeen wrote:
> The current test in _require_fio (--warnings-fatal --showcmd) does not
> fail if an invalid/unavailable io engine is specified.
>
> Add an explicit test that every requested io engine in the job file
> is actually available.
>
> Remove the "ioe_e4defrag" entries in ext4 tests - an engine with this
> name has seemingly never existed, but in each case later stanzas
> overrode the io engine, so it did not cause problems without this
> explicit parsing and checking.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/common/rc b/common/rc
> index ca755055..c155bb46 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3983,6 +3983,12 @@ _require_fio()
> return 1;
> fi
>
> + # Explicitly check for every ioengine availability
> + for ENGINE in `grep ioengine= $job | awk -F= '{print $2}'; sort`; do
^^^^^^
I think a local variable is enough, e.g. "local eng".
And, how about:
for eng in `sed -n "s/ioengine=\(.*\)/\1/p" $job`
> + fio --enghelp | grep -qw $ENGINE || \
^^^
$FIO_PROG
And to make sure the "||" checks the return value of grep, how about:
grep -wq $eng <($FIO_PROG --enghelp 2>/dev/null) ||
Others look good to me.
Thanks,
Zorro
> + _notrun "fio engine $ENGINE not available"
> + done
> +
> $FIO_PROG --warnings-fatal --showcmd $job >> $seqres.full 2>&1
> [ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full"
> }
> diff --git a/tests/ext4/301 b/tests/ext4/301
> index abf47d4b..a05b8e8a 100755
> --- a/tests/ext4/301
> +++ b/tests/ext4/301
> @@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
> diff --git a/tests/ext4/302 b/tests/ext4/302
> index 87820184..e0f5f2f9 100755
> --- a/tests/ext4/302
> +++ b/tests/ext4/302
> @@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
> diff --git a/tests/ext4/303 b/tests/ext4/303
> index 2381f047..0a83e86c 100755
> --- a/tests/ext4/303
> +++ b/tests/ext4/303
> @@ -31,7 +31,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (3+1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
> diff --git a/tests/ext4/304 b/tests/ext4/304
> index 53b522ee..5f2ae4bd 100755
> --- a/tests/ext4/304
> +++ b/tests/ext4/304
> @@ -32,7 +32,6 @@ FILE_SIZE=$((BLK_DEV_SIZE * (512 / (2 + 1))))
> cat >$fio_config <<EOF
> # Common e4defrag regression tests
> [global]
> -ioengine=ioe_e4defrag
> iodepth=1
> directory=${SCRATCH_MNT}
> filesize=${FILE_SIZE}
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] common/rc: explicitly test for engine availability in _require_fio
2025-03-14 14:36 ` Zorro Lang
@ 2025-03-14 14:39 ` Eric Sandeen
0 siblings, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2025-03-14 14:39 UTC (permalink / raw)
To: Zorro Lang; +Cc: fstests@vger.kernel.org, Jeff Moyer
On 3/14/25 9:36 AM, Zorro Lang wrote:
> On Wed, Mar 12, 2025 at 11:48:11AM -0500, Eric Sandeen wrote:
>> The current test in _require_fio (--warnings-fatal --showcmd) does not
>> fail if an invalid/unavailable io engine is specified.
>>
>> Add an explicit test that every requested io engine in the job file
>> is actually available.
>>
>> Remove the "ioe_e4defrag" entries in ext4 tests - an engine with this
>> name has seemingly never existed, but in each case later stanzas
>> overrode the io engine, so it did not cause problems without this
>> explicit parsing and checking.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/common/rc b/common/rc
>> index ca755055..c155bb46 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -3983,6 +3983,12 @@ _require_fio()
>> return 1;
>> fi
>>
>> + # Explicitly check for every ioengine availability
>> + for ENGINE in `grep ioengine= $job | awk -F= '{print $2}'; sort`; do
> ^^^^^^
>
> I think a local variable is enough, e.g. "local eng".
>
> And, how about:
> for eng in `sed -n "s/ioengine=\(.*\)/\1/p" $job`
>
>> + fio --enghelp | grep -qw $ENGINE || \
> ^^^
> $FIO_PROG
>
> And to make sure the "||" checks the return value of grep, how about:
>
> grep -wq $eng <($FIO_PROG --enghelp 2>/dev/null) ||
Clearly my bash-fu is lacking!
>
> Others look good to me.
Ok, I'll try this. Also noticing that I typo'd "sort" which was supposed
to be via a pipe, as well as a missing "uniq" :( sorry about that.
-Eric
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-18 13:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-18 10:08 [PATCH] common/rc: explicitly test for engine availability in _require_fio Zorro Lang
2025-03-18 13:23 ` Zorro Lang
-- strict thread matches above, loose matches on Subject: below --
2025-03-12 16:48 Eric Sandeen
2025-03-14 14:36 ` Zorro Lang
2025-03-14 14:39 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox