On Wed, Sep 10, 2025 at 02:07:15PM +0800, Zorro Lang wrote: > 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. Got it, I'll make this change, thanks > > > > > > > > > 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 :) Right I was just confirming. Thanks for the review :) Regards, ojaswin > > Thanks, > Zorro > > > > > > > thanks, > > > John > > >
