Re: [PATCH V3] common/rc: explicitly test for engine availability in _require_fio

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



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@xxxxxxxxxx>
> ---
> 
> 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}
> 
> 
> 





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux