On Fri, Jun 6, 2025 at 3:45 AM Zorro Lang <zlang@xxxxxxxxxx> wrote:
>
> On Thu, Jun 05, 2025 at 08:38:30PM +0200, Amir Goldstein wrote:
> > On Thu, Jun 5, 2025 at 7:32 PM Zorro Lang <zlang@xxxxxxxxxx> wrote:
> > >
> > > On Tue, Jun 03, 2025 at 12:07:43PM +0200, Amir Goldstein wrote:
> > > > This test performs shutdown via xfs_io -c shutdown.
> > > >
> > > > Overlayfs tests can use _scratch_shutdown, but they cannot use
> > > > "-c shutdown" xfs_io command without jumping through hoops, so by
> > > > default we do not support it.
> > > >
> > > > Add this condition to _require_xfs_io_command and add the require
> > > > statement to test generic/623 so it wont run with overlayfs.
> > > >
> > > > Reported-by: André Almeida <andrealmeid@xxxxxxxxxx>
> > > > Tested-by: André Almeida <andrealmeid@xxxxxxxxxx>
> > > > Link: https://lore.kernel.org/linux-fsdevel/20250521-ovl_ro-v1-1-2350b1493d94@xxxxxxxxxx/
> > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > > ---
> > > > common/rc | 8 ++++++++
> > > > tests/generic/623 | 1 +
> > > > 2 files changed, 9 insertions(+)
> > > >
> > > > diff --git a/common/rc b/common/rc
> > > > index d8ee8328..bffd576a 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -3033,6 +3033,14 @@ _require_xfs_io_command()
> > > > touch $testfile
> > > > testio=`$XFS_IO_PROG -c "syncfs" $testfile 2>&1`
> > > > ;;
> > > > + "shutdown")
> > > > + if [ $FSTYP = "overlay" ]; then
> > > > + # Overlayfs tests can use _scratch_shutdown, but they
> > > > + # cannot use "-c shutdown" xfs_io command without jumping
> > > > + # through hoops, so by default we do not support it.
> > > > + _notrun "xfs_io $command not supported on $FSTYP"
> > > > + fi
> > > > + ;;
> > >
> > > Hmm... I'm not sure this's a good way.
> > > For example, overlay/087 does xfs_io shutdown too,
> >
> > Yes it does but look at the effort needed to do that properly:
> >
> > $XFS_IO_PROG -x -c "open $(_scratch_shutdown_handle)" -c 'shutdown -f
> > ' -c close -c syncfs $SCRATCH_MNT | \
> > grep -vF '[00'
> >
> > > generally it should calls
> > > _require_xfs_io_command "shutdown" although it doesn't. If someone overlay
> > > test case hope to test as o/087 does, and it calls _require_xfs_io_command "shutdown",
> > > then it'll be _notrun.
> >
> > If someone knows enough to perform the dance above with _scratch_shutdown_handle
> > then that someone should know enough not to call
> > _require_xfs_io_command "shutdown".
> > OTOH, if someone doesn't know then default is to not run.
>
> Sure, I can understand that, just this logic is a bit *obscure* :) It sounds like:
> "If an overlay test case wants to do xfs_io shutdown, it shouldn't call
> _require_xfs_io_command "shutdown". Or call that to skip a shutdown test
> on overlay :)"
>
> And the expected result of _require_xfs_io_command "shutdown" will be totally
> opposite with _require_scratch_shutdown on overlay, that might be confused.
> Can we have a clearer way to deal with that?
>
I don't really understand the confusion.
_require_xfs_io_command "shutdown"
Like any other _require statement
requires support for what this test does -
meaning that a test does xfs_io -c shutdown, just like test generic/623 does
and _require_scratch_shutdown implies that the test does
_scratch_shutdown
FSTYP overlay happens to be able to do _scratch_shutdown
but not able to do xfs_io -c shutdown $SCRATCH_MNT
The different _require statements simply reflect reality as it is.
We can solve the confused about o/087 not having
_require_xfs_io_command "shutdown"
by moving the special hand crafted xfs_io command in o/087
to a helper _scratch_shutdown_and_syncfs to hide those internal
implementation details from test writers.
See attached patch.
Thanks,
Amir.
From 3b4d6ff3ce909fa4b385a8d075bbac1f32b82cb8 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@xxxxxxxxx>
Date: Fri, 6 Jun 2025 09:11:17 +0200
Subject: [PATCH] fstests: add helper _scratch_shutdown_and_syncfs
Test xfs/546 has to chain syncfs after shutdown and cannot
use the _scratch_shitdown helper, because after shutdown a fd
cannot be opened to execute syncfs on.
The xfs_io command of chaining syncfs after shutdown is rather
more complex to execute in the derived overlayfs test overlay/087.
Add a helper to abstract this complexity from test writers.
Add a _require statement to match.
Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
---
common/rc | 27 +++++++++++++++++++++++++++
tests/overlay/087 | 13 +++----------
tests/xfs/546 | 5 ++---
3 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/common/rc b/common/rc
index 96d65d1c..77683b82 100644
--- a/common/rc
+++ b/common/rc
@@ -595,6 +595,27 @@ _scratch_shutdown_handle()
fi
}
+_scratch_shutdown_and_syncfs()
+{
+ if [ $FSTYP = "overlay" ]; then
+ # In lagacy overlay usage, it may specify directory as
+ # SCRATCH_DEV, in this case OVL_BASE_SCRATCH_DEV
+ # will be null, so check OVL_BASE_SCRATCH_DEV before
+ # running shutdown to avoid shutting down base fs accidently.
+ if [ -z $OVL_BASE_SCRATCH_DEV ]; then
+ _fail "_scratch_shutdown: call _require_scratch_shutdown first in test"
+ fi
+ # This command is complicated a bit because in the case of overlayfs the
+ # syncfs fd needs to be opened before shutdown and it is different from the
+ # shutdown fd, so we cannot use the _scratch_shutdown() helper.
+ # Filter out xfs_io output of active fds.
+ $XFS_IO_PROG -x -c "open $(_scratch_shutdown_handle)" -c 'shutdown -f ' \
+ -c close -c syncfs $SCRATCH_MNT | grep -vF '[00'
+ else
+ $XFS_IO_PROG -x -c 'shutdown -f ' -c syncfs $SCRATCH_MNT
+ fi
+}
+
_move_mount()
{
local mnt=$1
@@ -4110,6 +4131,12 @@ _require_scratch_shutdown()
_scratch_unmount
}
+_require_scratch_shutdown_and_syncfs()
+{
+ _require_xfs_io_command syncfs
+ _require_scratch_shutdown
+}
+
_check_s_dax()
{
local target=$1
diff --git a/tests/overlay/087 b/tests/overlay/087
index a5afb0d5..2ad069db 100755
--- a/tests/overlay/087
+++ b/tests/overlay/087
@@ -32,9 +32,8 @@ _begin_fstest auto quick mount shutdown
# Modify as appropriate.
-_require_xfs_io_command syncfs
_require_scratch_nocheck
-_require_scratch_shutdown
+_require_scratch_shutdown_and_syncfs
[ "$OVL_BASE_FSTYP" == "xfs" ] || \
_notrun "base fs $OVL_BASE_FSTYP has unknown behavior with syncfs after shutdown"
@@ -43,19 +42,13 @@ _require_scratch_shutdown
# bother checking the filesystem afterwards since we never wrote anything.
echo "=== syncfs after shutdown"
_scratch_mount
-# This command is complicated a bit because in the case of overlayfs the
-# syncfs fd needs to be opened before shutdown and it is different from the
-# shutdown fd, so we cannot use the _scratch_shutdown() helper.
-# Filter out xfs_io output of active fds.
-$XFS_IO_PROG -x -c "open $(_scratch_shutdown_handle)" -c 'shutdown -f ' -c close -c syncfs $SCRATCH_MNT | \
- grep -vF '[00'
+_scratch_shutdown_and_syncfs
# Now repeat the same test with a volatile overlayfs mount and expect no error
_scratch_unmount
echo "=== syncfs after shutdown (volatile)"
_scratch_mount -o volatile
-$XFS_IO_PROG -x -c "open $(_scratch_shutdown_handle)" -c 'shutdown -f ' -c close -c syncfs $SCRATCH_MNT | \
- grep -vF '[00'
+_scratch_shutdown_and_syncfs
# success, all done
status=0
diff --git a/tests/xfs/546 b/tests/xfs/546
index 316ffc50..c50d41a6 100755
--- a/tests/xfs/546
+++ b/tests/xfs/546
@@ -27,14 +27,13 @@ _begin_fstest auto quick shutdown
# Modify as appropriate.
-_require_xfs_io_command syncfs
_require_scratch_nocheck
-_require_scratch_shutdown
+_require_scratch_shutdown_and_syncfs
# Reuse the fs formatted when we checked for the shutdown ioctl, and don't
# bother checking the filesystem afterwards since we never wrote anything.
_scratch_mount
-$XFS_IO_PROG -x -c 'shutdown -f ' -c syncfs $SCRATCH_MNT
+_scratch_shutdown_and_syncfs
# success, all done
status=0
--
2.34.1
