| Message ID | 3d35c97d-8c04-4d0b-b7d7-bc689edbc1ed@sandeen.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V3] common/rc: explicitly test for engine availability in _require_fio | expand |
On Fri, Mar 14, 2025 at 01:37:45PM -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. > > 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> I appreciate this extra bit in the commit message explaining that the removed lines were previously defunct. Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > > V2: Be Better at Bash, thanks Zorro. Also add | sort | uniq to ensure > we only test each requested engine once. > V3: I've got a fever, and the only prescription is more pedantry! > > diff --git a/common/rc b/common/rc > index ca755055..ac443ec2 100644 > --- a/common/rc > +++ b/common/rc > @@ -3977,12 +3977,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 | uniq`; 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 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} > > >
On Fri, Mar 14, 2025 at 01:37:45PM -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. > > 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> > --- > > V2: Be Better at Bash, thanks Zorro. Also add | sort | uniq to ensure > we only test each requested engine once. > V3: I've got a fever, and the only prescription is more pedantry! > > diff --git a/common/rc b/common/rc > index ca755055..ac443ec2 100644 > --- a/common/rc > +++ b/common/rc > @@ -3977,12 +3977,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 | uniq`; do ^^^^^^^^^^^ sort -u ? If you agree, I can help to change that when I merge it. Thanks, Zorro > + 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 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} > > >
On 3/14/25 11:37 PM, Zorro Lang wrote: ... >> + # Explicitly check for every ioengine listed in the job >> + for eng in `sed -n "s/ioengine=\(.*\)/\1/p" $job | sort | uniq`; do > ^^^^^^^^^^^ > sort -u ? > > If you agree, I can help to change that when I merge it. Sure Zorro, that's fine. -Eric > Thanks, > Zorro
diff --git a/common/rc b/common/rc index ca755055..ac443ec2 100644 --- a/common/rc +++ b/common/rc @@ -3977,12 +3977,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 | uniq`; 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 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}
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> --- V2: Be Better at Bash, thanks Zorro. Also add | sort | uniq to ensure we only test each requested engine once. V3: I've got a fever, and the only prescription is more pedantry!