| Message ID | b761e302-8b4e-4a79-b025-d344d7bf5a69@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | common/rc: explicitly test for engine availability in _require_fio | expand |
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} > >
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
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}
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> ---