Re: [PATCH] generic: test fsync of file with no more hard links

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Fri, Mar 21, 2025 at 11:22 PM Daniel Vacek <neelx@xxxxxxxx> wrote:
>
> On Fri, 21 Mar 2025 at 12:10, <fdmanana@xxxxxxxxxx> wrote:
> >
> > From: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > Test that if we fsync a file that has no more hard links, power fail and
> > then mount the filesystem, after the journal/log is replayed, the file
> > doesn't exists anymore.
> >
> > This exercises a bug recently found and fixed by the following patch for
> > the linux kernel:
> >
> >    btrfs: fix fsync of files with no hard links not persisting deletion
> >
> > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> > ---
> >  tests/generic/764     | 78 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/764.out |  2 ++
> >  2 files changed, 80 insertions(+)
> >  create mode 100755 tests/generic/764
> >  create mode 100644 tests/generic/764.out
> >
> > diff --git a/tests/generic/764 b/tests/generic/764
> > new file mode 100755
> > index 00000000..57d21095
> > --- /dev/null
> > +++ b/tests/generic/764
> > @@ -0,0 +1,78 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2025 SUSE Linux Products GmbH.  All Rights Reserved.
> > +#
> > +# FS QA Test 764
> > +#
> > +# Test that if we fsync a file that has no more hard links, power fail and then
> > +# mount the filesystem, after the journal/log is replayed, the file doesn't
> > +# exists anymore.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick log
> > +
> > +_cleanup()
> > +{
> > +       if [ ! -z $XFS_IO_PID ]; then
> > +               kill $XFS_IO_PID > /dev/null 2>&1
> > +               wait $XFS_IO_PID > /dev/null 2>&1
> > +       fi
> > +       _cleanup_flakey
> > +       cd /
> > +       rm -r -f $tmp.*
> > +}
> > +
> > +. ./common/dmflakey
> > +
> > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> > +       "btrfs: fix fsync of files with no hard links not persisting deletion"
> > +
> > +_require_scratch
> > +_require_dm_target flakey
> > +_require_mknod
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1 || _fail "mkfs failed"
> > +_require_metadata_journaling $SCRATCH_DEV
> > +_init_flakey
> > +_mount_flakey
> > +
> > +touch $SCRATCH_MNT/foo
> > +
> > +# Commit the current transaction and persist the file.
> > +_scratch_sync
> > +
> > +# A fifo to communicate with a background xfs_io process that will fsync the
> > +# file after we deleted its hard link while it's open by xfs_io.
> > +mkfifo $SCRATCH_MNT/fifo
>
> After creating the pipe you can "exec 3<>$SCRATCH_MNT/fifo" (and
> eventually unlink) ...

Yes, I'm aware of exec.
Have used it in other tests written years ago, like btrfs/168,
btrfs/233, and others.

>
> > +
> > +tail -f $SCRATCH_MNT/fifo | $XFS_IO_PROG $SCRATCH_MNT/foo >>$seqres.full &
>
> ... and then simply "$XFS_IO_PROG $SCRATCH_MNT/foo <&3 >>$seqres.full &"

That's arguably more cryptic and not so easy to read.

The approach I chose was exactly due to being a lot simpler to read
and less cryptic.

The goal is to make the test as easy to read as possible, and not to
show off technical skills with bash.

>
> > +XFS_IO_PID=$!
> > +
> > +# Give some time for the xfs_io process to open a file descriptor for the file.
> > +sleep 1
> > +
> > +# Now while the file is open by the xfs_io process, delete its only hard link.
> > +rm -f $SCRATCH_MNT/foo
> > +
> > +# Now that it has no more hard links, make the xfs_io process fsync it.
> > +echo "fsync" > $SCRATCH_MNT/fifo
>
> No need for the quotes. But won't hurt either if that's more readable for you.

Yes, I'm aware they're not needed. I like to use them however, for the
syntax highlighting I get with my editor.

>
> Moreover with the above you can also "echo fsync >&3".

Yes.
Again, I prefer the much more straightforward to read approach of
explicitly redirecting to the fifo.

Thanks.

>
> > +
> > +# Terminate the xfs_io process so that we can unmount.
> > +echo "quit" > $SCRATCH_MNT/fifo
>
> ...
>
> > +wait $XFS_IO_PID
> > +unset XFS_IO_PID
> > +
> > +# Simulate a power failure and then mount again the filesystem to replay the
> > +# journal/log.
> > +_flakey_drop_and_remount
> > +
> > +# We don't expect the file to exist anymore, since it was fsynced when it had no
> > +# more hard links.
> > +[ -f $SCRATCH_MNT/foo ] && echo "file foo still exists"
> > +
> > +_unmount_flakey
> > +
> > +# success, all done
> > +echo "Silence is golden"
> > +status=0
> > +exit
> > diff --git a/tests/generic/764.out b/tests/generic/764.out
> > new file mode 100644
> > index 00000000..bb58e5b8
> > --- /dev/null
> > +++ b/tests/generic/764.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 764
> > +Silence is golden
> > --
> > 2.45.2
> >
> >





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux