Re: [PATCH 2/3] fstests: add helper _require_xfs_io_shutdown

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 10, 2025 at 08:34:01PM +0200, Amir Goldstein wrote:
> On Tue, Jun 10, 2025 at 4:42 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Mon, Jun 09, 2025 at 05:19:14PM +0200, Amir Goldstein wrote:
> > > Requirements for tests that shutdown fs using "xfs_io -c shutdown".
> > > The requirements are stricter than the requirement for tests that
> > > shutdown fs using _scratch_shutdown helper.
> > >
> > > Generally, with overlay fs, tests can do _scratch_shutdown, but not
> > > xfs_io -c shutdown.
> > >
> > > Encode this stricter requirement in helper _require_xfs_io_shutdown
> > > and use it in test generic/623, to express that it cannot run on
> > > overalyfs.
> > >
> > > Reported-by: André Almeida <andrealmeid@igalia.com>
> > > Link: https://lore.kernel.org/linux-fsdevel/20250521-ovl_ro-v1-1-2350b1493d94@igalia.com/
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > Makes sense to me, assuming you don't want to try to integrate the 'open
> > shutdown handle' dance into this test.
> >
> > ioargs=(-x -c "mmap 0 4k" -c "mwrite 0 4k")
> > case "$FSTYP" in
> > overlayfs)
> >         ioargs+=(-c "open $(_scratch_shutdown_handle)" -c 'shutdown -f ' -c close)
> >         ;;
> > *)
> >         ioargs+=(-c shutdown)
> >         ;;
> > esac
> > ioargs+=(-c fsync -c "mwrite 0 4k" $file)
> >
> > $XFS_IO_PROG "${ioargs[@]}" | _filter_xfs_io
> >
> > (Though I don't know if you actually tried that and it didn't work, or
> > maybe overlayfs mmap is weird, etc...)
> 
> I did not try it.
> I had briefly considered trying and decided it's not worth it.
> overlayfs doesn't have an aops of its own, and mmap results in
> a memory map of the underlying file directly, so the test will essentially
> testing the base via overlayfs which does not add much test coverage.
> 
> I do not object to making this test run on overlayfs, but I do wish to
> keep the helper around for future tests that will not do the dance.

<shrug> If fstests were written in a modern hipster language then
passing around a lambda would be easy and trivial...

...but this is bash.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> Thanks,
> Amir.