Re: [PATCH v6 02/12] common/rc: Add fio atomic write helpers

[John Garry <john.g.garry@oracle.com>]

On 11/09/2025 18:13, 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 internal helper __require_fio_version to require specific
> versions of fio to work past such issues. Further, add the high level
> _require_fio_atomic_writes helper which tests can use to ensure fio
> has the right version for atomic writes.
> 
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Thanks for doing this.

Reviewed-by: John Garry <john.g.garry@oracle.com>


> ---
>   common/rc | 43 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 43 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 28fbbcbb..8a023b9d 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -6000,6 +6000,49 @@ _max() {
>   	echo $ret
>   }
>   
> +# Due to reasons explained in fio commit 40f1fc11d, fio version between
> +# v3.33 and v3.38 have atomic= feature but it is a no-op and doesn't do
> +# RWF_ATOMIC write. Hence, use this helper to ensure fio has the
> +# required support. Currently, the simplest way we have is to ensure
> +# the version.
> +_require_fio_atomic_writes() {
> +	__require_fio_version "3.38+"
> +}
> +
> +# 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)
> +#
> +# Internal helper, avoid using directly in tests.
> +__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