[PATCH] generic: test fsync of directory after renaming new symlink

[PATCH] generic: test fsync of directory after renaming new symlink

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Test that if we fsync a directory that has a new symlink, then rename the
symlink and fsync again the directory, after a power failure the symlink
exists with the new name and not the old one.

This is to exercise a bug in btrfs where we ended up not persisting the
new name of the symlink. That is fixed by a kernel patch that has the
following subject:

 "btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/generic/779     | 60 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/779.out |  2 ++
 2 files changed, 62 insertions(+)
 create mode 100755 tests/generic/779
 create mode 100644 tests/generic/779.out

diff --git a/tests/generic/779 b/tests/generic/779
new file mode 100755
index 00000000..40d1a86c
--- /dev/null
+++ b/tests/generic/779
@@ -0,0 +1,60 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2025 SUSE S.A.  All Rights Reserved.
+#
+# FS QA Test 779
+#
+# Test that if we fsync a directory that has a new symlink, then rename the
+# symlink and fsync again the directory, after a power failure the symlink
+# exists with the new name and not the old one.
+#
+. ./common/preamble
+_begin_fstest auto quick log
+
+_cleanup()
+{
+	_cleanup_flakey
+	cd /
+	rm -r -f $tmp.*
+}
+
+. ./common/dmflakey
+
+_require_scratch
+_require_symlinks
+_require_dm_target flakey
+
+[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
+	"btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
+
+rm -f $seqres.full
+
+_scratch_mkfs >>$seqres.full 2>&1 || _fail "mkfs failed"
+_require_metadata_journaling $SCRATCH_DEV
+_init_flakey
+_mount_flakey
+
+# Create our test dir and add a symlink inside it.
+mkdir $SCRATCH_MNT/dir
+ln -s foobar $SCRATCH_MNT/dir/old-slink
+
+# Fsync the test dir, should persist the symlink.
+$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
+
+# Rename the symlink and fsync the directory. It should persist the new symlink
+# name.
+mv $SCRATCH_MNT/dir/old-slink $SCRATCH_MNT/dir/new-slink
+$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
+
+# Simulate a power failure and then mount again the filesystem to replay the
+# journal/log.
+_flakey_drop_and_remount
+
+# Check that the symlink exists with the new name and has the correct content.
+[ -L $SCRATCH_MNT/dir/new-slink ] || echo "symlink dir/new-slink not found"
+echo "symlink content: $(readlink $SCRATCH_MNT/dir/new-slink)"
+
+_unmount_flakey
+
+# success, all done
+_exit 0
diff --git a/tests/generic/779.out b/tests/generic/779.out
new file mode 100644
index 00000000..c595cd01
--- /dev/null
+++ b/tests/generic/779.out
@@ -0,0 +1,2 @@
+QA output created by 779
+symlink content: foobar
-- 
2.47.2

Re: [PATCH] generic: test fsync of directory after renaming new symlink

[Boris Burkov]

On Fri, Oct 24, 2025 at 12:53:12PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we fsync a directory that has a new symlink, then rename the
> symlink and fsync again the directory, after a power failure the symlink
> exists with the new name and not the old one.
> 
> This is to exercise a bug in btrfs where we ended up not persisting the
> new name of the symlink. That is fixed by a kernel patch that has the
> following subject:
> 
>  "btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Boris Burkov <boris@bur.io>

> ---
>  tests/generic/779     | 60 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/779.out |  2 ++
>  2 files changed, 62 insertions(+)
>  create mode 100755 tests/generic/779
>  create mode 100644 tests/generic/779.out
> 
> diff --git a/tests/generic/779 b/tests/generic/779
> new file mode 100755
> index 00000000..40d1a86c
> --- /dev/null
> +++ b/tests/generic/779
> @@ -0,0 +1,60 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025 SUSE S.A.  All Rights Reserved.
> +#
> +# FS QA Test 779
> +#
> +# Test that if we fsync a directory that has a new symlink, then rename the
> +# symlink and fsync again the directory, after a power failure the symlink
> +# exists with the new name and not the old one.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick log
> +
> +_cleanup()
> +{
> +	_cleanup_flakey
> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +. ./common/dmflakey
> +
> +_require_scratch
> +_require_symlinks
> +_require_dm_target flakey
> +
> +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> +	"btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs >>$seqres.full 2>&1 || _fail "mkfs failed"
> +_require_metadata_journaling $SCRATCH_DEV
> +_init_flakey
> +_mount_flakey
> +
> +# Create our test dir and add a symlink inside it.
> +mkdir $SCRATCH_MNT/dir
> +ln -s foobar $SCRATCH_MNT/dir/old-slink
> +
> +# Fsync the test dir, should persist the symlink.
> +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> +
> +# Rename the symlink and fsync the directory. It should persist the new symlink
> +# name.
> +mv $SCRATCH_MNT/dir/old-slink $SCRATCH_MNT/dir/new-slink
> +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> +
> +# Simulate a power failure and then mount again the filesystem to replay the
> +# journal/log.
> +_flakey_drop_and_remount
> +
> +# Check that the symlink exists with the new name and has the correct content.
> +[ -L $SCRATCH_MNT/dir/new-slink ] || echo "symlink dir/new-slink not found"
> +echo "symlink content: $(readlink $SCRATCH_MNT/dir/new-slink)"
> +
> +_unmount_flakey
> +
> +# success, all done
> +_exit 0
> diff --git a/tests/generic/779.out b/tests/generic/779.out
> new file mode 100644
> index 00000000..c595cd01
> --- /dev/null
> +++ b/tests/generic/779.out
> @@ -0,0 +1,2 @@
> +QA output created by 779
> +symlink content: foobar
> -- 
> 2.47.2
> 

Re: [PATCH] generic: test fsync of directory after renaming new symlink

[Qu Wenruo]

在 2025/10/24 22:23, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we fsync a directory that has a new symlink, then rename the
> symlink and fsync again the directory, after a power failure the symlink
> exists with the new name and not the old one.
> 
> This is to exercise a bug in btrfs where we ended up not persisting the
> new name of the symlink. That is fixed by a kernel patch that has the
> following subject:
> 
>   "btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>   tests/generic/779     | 60 +++++++++++++++++++++++++++++++++++++++++++
>   tests/generic/779.out |  2 ++
>   2 files changed, 62 insertions(+)
>   create mode 100755 tests/generic/779
>   create mode 100644 tests/generic/779.out
> 
> diff --git a/tests/generic/779 b/tests/generic/779
> new file mode 100755
> index 00000000..40d1a86c
> --- /dev/null
> +++ b/tests/generic/779
> @@ -0,0 +1,60 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025 SUSE S.A.  All Rights Reserved.
> +#
> +# FS QA Test 779
> +#
> +# Test that if we fsync a directory that has a new symlink, then rename the
> +# symlink and fsync again the directory, after a power failure the symlink
> +# exists with the new name and not the old one.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick log
> +
> +_cleanup()
> +{
> +	_cleanup_flakey
> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +. ./common/dmflakey
> +
> +_require_scratch
> +_require_symlinks
> +_require_dm_target flakey
> +
> +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> +	"btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> +
> +rm -f $seqres.full

Looks like a rouge command?


Otherwise looks good to me.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> +
> +_scratch_mkfs >>$seqres.full 2>&1 || _fail "mkfs failed"
> +_require_metadata_journaling $SCRATCH_DEV
> +_init_flakey
> +_mount_flakey
> +
> +# Create our test dir and add a symlink inside it.
> +mkdir $SCRATCH_MNT/dir
> +ln -s foobar $SCRATCH_MNT/dir/old-slink
> +
> +# Fsync the test dir, should persist the symlink.
> +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> +
> +# Rename the symlink and fsync the directory. It should persist the new symlink
> +# name.
> +mv $SCRATCH_MNT/dir/old-slink $SCRATCH_MNT/dir/new-slink
> +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> +
> +# Simulate a power failure and then mount again the filesystem to replay the
> +# journal/log.
> +_flakey_drop_and_remount
> +
> +# Check that the symlink exists with the new name and has the correct content.
> +[ -L $SCRATCH_MNT/dir/new-slink ] || echo "symlink dir/new-slink not found"
> +echo "symlink content: $(readlink $SCRATCH_MNT/dir/new-slink)"
> +
> +_unmount_flakey
> +
> +# success, all done
> +_exit 0
> diff --git a/tests/generic/779.out b/tests/generic/779.out
> new file mode 100644
> index 00000000..c595cd01
> --- /dev/null
> +++ b/tests/generic/779.out
> @@ -0,0 +1,2 @@
> +QA output created by 779
> +symlink content: foobar

Re: [PATCH] generic: test fsync of directory after renaming new symlink

[Filipe Manana]

On Sat, Oct 25, 2025 at 11:13 AM Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> 在 2025/10/24 22:23, fdmanana@kernel.org 写道:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Test that if we fsync a directory that has a new symlink, then rename the
> > symlink and fsync again the directory, after a power failure the symlink
> > exists with the new name and not the old one.
> >
> > This is to exercise a bug in btrfs where we ended up not persisting the
> > new name of the symlink. That is fixed by a kernel patch that has the
> > following subject:
> >
> >   "btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >   tests/generic/779     | 60 +++++++++++++++++++++++++++++++++++++++++++
> >   tests/generic/779.out |  2 ++
> >   2 files changed, 62 insertions(+)
> >   create mode 100755 tests/generic/779
> >   create mode 100644 tests/generic/779.out
> >
> > diff --git a/tests/generic/779 b/tests/generic/779
> > new file mode 100755
> > index 00000000..40d1a86c
> > --- /dev/null
> > +++ b/tests/generic/779
> > @@ -0,0 +1,60 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2025 SUSE S.A.  All Rights Reserved.
> > +#
> > +# FS QA Test 779
> > +#
> > +# Test that if we fsync a directory that has a new symlink, then rename the
> > +# symlink and fsync again the directory, after a power failure the symlink
> > +# exists with the new name and not the old one.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick log
> > +
> > +_cleanup()
> > +{
> > +     _cleanup_flakey
> > +     cd /
> > +     rm -r -f $tmp.*
> > +}
> > +
> > +. ./common/dmflakey
> > +
> > +_require_scratch
> > +_require_symlinks
> > +_require_dm_target flakey
> > +
> > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> > +     "btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> > +
> > +rm -f $seqres.full
>
> Looks like a rouge command?

It was copied from some other test.
A few tests have this, but it doesn't seem to be needed (anymore at least).

I'll let Zerro remove the line when he picks this.

>
>
> Otherwise looks good to me.
>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
>
> Thanks,
> Qu
>
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1 || _fail "mkfs failed"
> > +_require_metadata_journaling $SCRATCH_DEV
> > +_init_flakey
> > +_mount_flakey
> > +
> > +# Create our test dir and add a symlink inside it.
> > +mkdir $SCRATCH_MNT/dir
> > +ln -s foobar $SCRATCH_MNT/dir/old-slink
> > +
> > +# Fsync the test dir, should persist the symlink.
> > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> > +
> > +# Rename the symlink and fsync the directory. It should persist the new symlink
> > +# name.
> > +mv $SCRATCH_MNT/dir/old-slink $SCRATCH_MNT/dir/new-slink
> > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> > +
> > +# Simulate a power failure and then mount again the filesystem to replay the
> > +# journal/log.
> > +_flakey_drop_and_remount
> > +
> > +# Check that the symlink exists with the new name and has the correct content.
> > +[ -L $SCRATCH_MNT/dir/new-slink ] || echo "symlink dir/new-slink not found"
> > +echo "symlink content: $(readlink $SCRATCH_MNT/dir/new-slink)"
> > +
> > +_unmount_flakey
> > +
> > +# success, all done
> > +_exit 0
> > diff --git a/tests/generic/779.out b/tests/generic/779.out
> > new file mode 100644
> > index 00000000..c595cd01
> > --- /dev/null
> > +++ b/tests/generic/779.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 779
> > +symlink content: foobar
>

Re: [PATCH] generic: test fsync of directory after renaming new symlink

[Zorro Lang]

On Sat, Oct 25, 2025 at 08:39:56PM +0100, Filipe Manana wrote:
> On Sat, Oct 25, 2025 at 11:13 AM Qu Wenruo <wqu@suse.com> wrote:
> >
> >
> >
> > 在 2025/10/24 22:23, fdmanana@kernel.org 写道:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > Test that if we fsync a directory that has a new symlink, then rename the
> > > symlink and fsync again the directory, after a power failure the symlink
> > > exists with the new name and not the old one.
> > >
> > > This is to exercise a bug in btrfs where we ended up not persisting the
> > > new name of the symlink. That is fixed by a kernel patch that has the
> > > following subject:
> > >
> > >   "btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> > >   tests/generic/779     | 60 +++++++++++++++++++++++++++++++++++++++++++
> > >   tests/generic/779.out |  2 ++
> > >   2 files changed, 62 insertions(+)
> > >   create mode 100755 tests/generic/779
> > >   create mode 100644 tests/generic/779.out
> > >
> > > diff --git a/tests/generic/779 b/tests/generic/779
> > > new file mode 100755
> > > index 00000000..40d1a86c
> > > --- /dev/null
> > > +++ b/tests/generic/779
> > > @@ -0,0 +1,60 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2025 SUSE S.A.  All Rights Reserved.
> > > +#
> > > +# FS QA Test 779
> > > +#
> > > +# Test that if we fsync a directory that has a new symlink, then rename the
> > > +# symlink and fsync again the directory, after a power failure the symlink
> > > +# exists with the new name and not the old one.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick log
> > > +
> > > +_cleanup()
> > > +{
> > > +     _cleanup_flakey
> > > +     cd /
> > > +     rm -r -f $tmp.*
> > > +}
> > > +
> > > +. ./common/dmflakey
> > > +
> > > +_require_scratch
> > > +_require_symlinks
> > > +_require_dm_target flakey
> > > +
> > > +[ "$FSTYP" = "btrfs" ] && _fixed_by_kernel_commit xxxxxxxxxxxx \
> > > +     "btrfs: set inode flag BTRFS_INODE_COPY_EVERYTHING when logging new name"
> > > +
> > > +rm -f $seqres.full
> >
> > Looks like a rouge command?
> 
> It was copied from some other test.
> A few tests have this, but it doesn't seem to be needed (anymore at least).
> 
> I'll let Zerro remove the line when he picks this.

Sure, just removed and merged, this patch is good to me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

Thanks,
Zorro

> 
> >
> >
> > Otherwise looks good to me.
> >
> > Reviewed-by: Qu Wenruo <wqu@suse.com>
> >
> > Thanks,
> > Qu
> >
> > > +
> > > +_scratch_mkfs >>$seqres.full 2>&1 || _fail "mkfs failed"
> > > +_require_metadata_journaling $SCRATCH_DEV
> > > +_init_flakey
> > > +_mount_flakey
> > > +
> > > +# Create our test dir and add a symlink inside it.
> > > +mkdir $SCRATCH_MNT/dir
> > > +ln -s foobar $SCRATCH_MNT/dir/old-slink
> > > +
> > > +# Fsync the test dir, should persist the symlink.
> > > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> > > +
> > > +# Rename the symlink and fsync the directory. It should persist the new symlink
> > > +# name.
> > > +mv $SCRATCH_MNT/dir/old-slink $SCRATCH_MNT/dir/new-slink
> > > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/dir
> > > +
> > > +# Simulate a power failure and then mount again the filesystem to replay the
> > > +# journal/log.
> > > +_flakey_drop_and_remount
> > > +
> > > +# Check that the symlink exists with the new name and has the correct content.
> > > +[ -L $SCRATCH_MNT/dir/new-slink ] || echo "symlink dir/new-slink not found"
> > > +echo "symlink content: $(readlink $SCRATCH_MNT/dir/new-slink)"
> > > +
> > > +_unmount_flakey
> > > +
> > > +# success, all done
> > > +_exit 0
> > > diff --git a/tests/generic/779.out b/tests/generic/779.out
> > > new file mode 100644
> > > index 00000000..c595cd01
> > > --- /dev/null
> > > +++ b/tests/generic/779.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 779
> > > +symlink content: foobar
> >
>