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

[Eric Sandeen <sandeen@redhat.com>]

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