Re: [PATCH v5 02/12] common/rc: Add _require_fio_version helper

[Ojaswin Mujoo]

On Tue, Aug 26, 2025 at 12:08:01AM +0800, Zorro Lang wrote:
> On Fri, Aug 22, 2025 at 01:32:01PM +0530, Ojaswin Mujoo wrote:
> > The main motivation of adding this function on top of _require_fio is
> > that there has been a case in fio where atomic= option was added but
> > later it was changed to noop since kernel didn't yet have support for
> > atomic writes. It was then again utilized to do atomic writes in a later
> > version, once kernel got the support. Due to this there is a point in
> > fio where _require_fio w/ atomic=1 will succeed even though it would
> > not be doing atomic writes.
> > 
> > Hence, add an explicit helper to ensure tests to require specific
> > versions of fio to work past such issues.
> 
> Actually I'm wondering if fstests really needs to care about this. This's
> just a temporary issue of fio, not kernel or any fs usespace program. Do
> we need to add a seperated helper only for a temporary fio issue? If fio
> doesn't break fstests running, let it run. Just the testers install proper
> fio (maybe latest) they need. What do you and others think?
> 
> Thanks,
> Zorro

Hey Zorro,

Sure I'm okay with not keeping the helper and letting the user make sure
the fio version is correct.

@John, does that sound okay?

Regards,
ojaswin
> 
> > 
> > Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
> > ---
> >  common/rc | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 35a1c835..f45b9a38 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -5997,6 +5997,38 @@ _max() {
> >  	echo $ret
> >  }
> >  
> > +# Check the required fio version. Examples:
> > +#   _require_fio_version 3.38 (matches 3.38 only)
> > +#   _require_fio_version 3.38+ (matches 3.38 and above)
> > +#   _require_fio_version 3.38- (matches 3.38 and below)
> > +_require_fio_version() {
> > +	local req_ver="$1"
> > +	local fio_ver
> > +
> > +	_require_fio
> > +	_require_math
> > +
> > +	fio_ver=$(fio -v | cut -d"-" -f2)
> > +
> > +	case "$req_ver" in
> > +	*+)
> > +		req_ver=${req_ver%+}
> > +		test $(_math "$fio_ver >= $req_ver") -eq 1 || \
> > +			_notrun "need fio >= $req_ver (found $fio_ver)"
> > +		;;
> > +	*-)
> > +		req_ver=${req_ver%-}
> > +		test $(_math "$fio_ver <= $req_ver") -eq 1 || \
> > +			_notrun "need fio <= $req_ver (found $fio_ver)"
> > +		;;
> > +	*)
> > +		req_ver=${req_ver%-}
> > +		test $(_math "$fio_ver == $req_ver") -eq 1 || \
> > +			_notrun "need fio = $req_ver (found $fio_ver)"
> > +		;;
> > +	esac
> > +}
> > +
> >  ################################################################################
> >  # make sure this script returns success
> >  /bin/true
> > -- 
> > 2.49.0
> > 
>