linux-kernel - Re: [PATCH v5 02/12] common/rc: Add _require_fio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250910060715.gc2thcbklvhzaxz2@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com>
Date: Wed, 10 Sep 2025 14:07:15 +0800
From: Zorro Lang <zlang@...hat.com>
To: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
Cc: John Garry <john.g.garry@...cle.com>, fstests@...r.kernel.org,
	Ritesh Harjani <ritesh.list@...il.com>, djwong@...nel.org,
	tytso@....edu, linux-xfs@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH v5 02/12] common/rc: Add _require_fio_version helper

On Tue, Sep 09, 2025 at 02:32:36PM +0530, Ojaswin Mujoo wrote:
> On Tue, Sep 09, 2025 at 08:26:52AM +0100, John Garry wrote:
> > On 09/09/2025 08:16, Ojaswin Mujoo wrote:
> > > > > This requires the user to know the version which corresponds to the feature.
> > > > > Is that how things are done for other such utilities and their versions vs
> > > > > features?
> > > > > 
> > > > > I was going to suggest exporting something like
> > > > > _require_fio_atomic_writes(), and _require_fio_atomic_writes() calls
> > > > > _require_fio_version() to check the version.
> > > > (Sorry, I made a half reply in my last email)
> > > > 
> > > > This looks better than only using _require_fio_version. But the nature is still
> > > > checking fio version. If we don't have a better idea to check if fio really
> > > > support atomic writes, the _require_fio_version is still needed.
> > > > Or we rename it to "__require_fio_version" (one more "_"), to mark it's
> > > > not recommended using directly. But that looks a bit like a trick 😂
> > > > 
> > > > Thanks,
> > > > Zorro
> > > Hey Zorro, I agree with your points that version might not be the best
> > > indicator esp for downstream software, but at this point I'm unsure
> > > what's the workaround.

Hi Ojaswin, I don't have better workaround than require_fio_version for now. I mean:
1) name _require_fio_version as __require_fio_version, to mark it's an internal function
   of another common function.
2) only call __require_fio_version in _require_fio_atomic_writes for now, don't use it
   in any test cases directly.

> > > 
> > > One thing that comes to mind is to let fio do the atomic write and use
> > > the tracepoints to confirm if RWF_ATOMIC was passed, but that adds a lot
> > > of dependency on tracing framework being present (im unsure if something
> > > like this is used somewhere in xfstests before). Further it's messy to
> > > figure out that out of all the IO fio command will do, which one to
> > > check for RWF_ATOMIC.
> > > 
> > > It can be done I suppose but is this sort of complexity something we
> > > want to add is the question. Or do we just go ahead with the version
> > > check.
> > 
> > I think that just checking the version is fine for this specific feature.
> > But I still also think that versioning should be hidden from the end user,
> > i.e. we should provide a helper like _require_fio_atomic_writes
> 
> Sure, I'm okay. @Zorro, does that sound okay to you?

Sure, that's what I tried to say as above, sorry if I made you misunderstand :)

Thanks,
Zorro

> > 
> > thanks,
> > John
>