[Resend PATCH] generic: test fsync of directory with renamed symlink

[Resend PATCH] generic: test fsync of directory with renamed symlink

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Test that if we fsync a directory, create a symlink inside it, rename
the symlink, fsync again the directory and then power fail, after the
filesystem is mounted again, the symlink exists with the new name and
it has the correct content.

This currently fails on btrfs, because the symlink ends up empty (which
is illegal on Linux), but it is fixed by kernel commit:

    d0e64a981fd841 ("btrfs: always log symlinks in full mode")

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

Resending as this was missed on the last update.
No changes, only rebased on the current 'for-next' branch.

 tests/generic/690     | 89 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/690.out |  2 +
 2 files changed, 91 insertions(+)
 create mode 100755 tests/generic/690
 create mode 100644 tests/generic/690.out

diff --git a/tests/generic/690 b/tests/generic/690
new file mode 100755
index 00000000..0bf47dd7
--- /dev/null
+++ b/tests/generic/690
@@ -0,0 +1,89 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 SUSE Linux Products GmbH.  All Rights Reserved.
+#
+# FS QA Test 690
+#
+# Test that if we fsync a directory, create a symlink inside it, rename the
+# symlink, fsync again the directory and then power fail, after the filesystem
+# is mounted again, the symlink exists with the new name and it has the correct
+# content.
+#
+# On btrfs this used to result in the symlink being empty (i_size 0), and it was
+# fixed by kernel commit:
+#
+#    d0e64a981fd841 ("btrfs: always log symlinks in full mode")
+#
+. ./common/preamble
+_begin_fstest auto quick log
+
+_cleanup()
+{
+	_cleanup_flakey
+	cd /
+	rm -r -f $tmp.*
+}
+
+. ./common/rc
+. ./common/filter
+. ./common/dmflakey
+
+# real QA test starts here
+
+_supported_fs generic
+_require_scratch
+_require_symlinks
+_require_dm_target flakey
+
+rm -f $seqres.full
+
+# f2fs doesn't support fs-op level transaction functionality, so it has no way
+# to persist all metadata updates in one transaction. We have to use its mount
+# option "fastboot" so that it triggers a metadata checkpoint to persist all
+# metadata updates that happen before a fsync call. Without this, after the
+# last fsync in the test, the symlink named "baz" will not exist.
+if [ $FSTYP = "f2fs" ]; then
+	export MOUNT_OPTIONS="-o fastboot $MOUNT_OPTIONS"
+fi
+
+_scratch_mkfs >>$seqres.full 2>&1
+_require_metadata_journaling $SCRATCH_DEV
+_init_flakey
+_mount_flakey
+
+# Create our test directory.
+mkdir $SCRATCH_MNT/testdir
+
+# Commit the current transaction and persist the directory.
+sync
+
+# Create a file in the test directory, so that the next fsync on the directory
+# actually does something (it logs the directory).
+echo -n > $SCRATCH_MNT/testdir/foo
+
+# Fsync the directory.
+$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
+
+# Now create a symlink inside the test directory.
+ln -s $SCRATCH_MNT/testdir/foo $SCRATCH_MNT/testdir/bar
+
+# Rename the symlink.
+mv $SCRATCH_MNT/testdir/bar $SCRATCH_MNT/testdir/baz
+
+# Fsync again the directory.
+$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
+
+# Simulate a power failure and then mount again the filesystem to replay the
+# journal/log.
+_flakey_drop_and_remount
+
+# The symlink should exist, with the name "baz" and its content must be
+# "$SCRATCH_MNT/testdir/foo".
+[ -L $SCRATCH_MNT/testdir/baz ] || echo "symlink 'baz' is missing"
+echo "symlink content: $(readlink $SCRATCH_MNT/testdir/baz | _filter_scratch)"
+
+_unmount_flakey
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/690.out b/tests/generic/690.out
new file mode 100644
index 00000000..84be1247
--- /dev/null
+++ b/tests/generic/690.out
@@ -0,0 +1,2 @@
+QA output created by 690
+symlink content: SCRATCH_MNT/testdir/foo
-- 
2.35.1

Re: [Resend PATCH] generic: test fsync of directory with renamed symlink

[Filipe Manana]

On Tue, May 03, 2022 at 11:57:49AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we fsync a directory, create a symlink inside it, rename
> the symlink, fsync again the directory and then power fail, after the
> filesystem is mounted again, the symlink exists with the new name and
> it has the correct content.
> 
> This currently fails on btrfs, because the symlink ends up empty (which
> is illegal on Linux), but it is fixed by kernel commit:
> 
>     d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> 
> Resending as this was missed on the last update.
> No changes, only rebased on the current 'for-next' branch.

Zorro,

This missed against the last fstests update.
Did this patch fell through the cracks, or do you expect me to do something about it?

Should I rebase and resend again?

Thanks.

> 
>  tests/generic/690     | 89 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/690.out |  2 +
>  2 files changed, 91 insertions(+)
>  create mode 100755 tests/generic/690
>  create mode 100644 tests/generic/690.out
> 
> diff --git a/tests/generic/690 b/tests/generic/690
> new file mode 100755
> index 00000000..0bf47dd7
> --- /dev/null
> +++ b/tests/generic/690
> @@ -0,0 +1,89 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 SUSE Linux Products GmbH.  All Rights Reserved.
> +#
> +# FS QA Test 690
> +#
> +# Test that if we fsync a directory, create a symlink inside it, rename the
> +# symlink, fsync again the directory and then power fail, after the filesystem
> +# is mounted again, the symlink exists with the new name and it has the correct
> +# content.
> +#
> +# On btrfs this used to result in the symlink being empty (i_size 0), and it was
> +# fixed by kernel commit:
> +#
> +#    d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> +#
> +. ./common/preamble
> +_begin_fstest auto quick log
> +
> +_cleanup()
> +{
> +	_cleanup_flakey
> +	cd /
> +	rm -r -f $tmp.*
> +}
> +
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmflakey
> +
> +# real QA test starts here
> +
> +_supported_fs generic
> +_require_scratch
> +_require_symlinks
> +_require_dm_target flakey
> +
> +rm -f $seqres.full
> +
> +# f2fs doesn't support fs-op level transaction functionality, so it has no way
> +# to persist all metadata updates in one transaction. We have to use its mount
> +# option "fastboot" so that it triggers a metadata checkpoint to persist all
> +# metadata updates that happen before a fsync call. Without this, after the
> +# last fsync in the test, the symlink named "baz" will not exist.
> +if [ $FSTYP = "f2fs" ]; then
> +	export MOUNT_OPTIONS="-o fastboot $MOUNT_OPTIONS"
> +fi
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_require_metadata_journaling $SCRATCH_DEV
> +_init_flakey
> +_mount_flakey
> +
> +# Create our test directory.
> +mkdir $SCRATCH_MNT/testdir
> +
> +# Commit the current transaction and persist the directory.
> +sync
> +
> +# Create a file in the test directory, so that the next fsync on the directory
> +# actually does something (it logs the directory).
> +echo -n > $SCRATCH_MNT/testdir/foo
> +
> +# Fsync the directory.
> +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> +
> +# Now create a symlink inside the test directory.
> +ln -s $SCRATCH_MNT/testdir/foo $SCRATCH_MNT/testdir/bar
> +
> +# Rename the symlink.
> +mv $SCRATCH_MNT/testdir/bar $SCRATCH_MNT/testdir/baz
> +
> +# Fsync again the directory.
> +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> +
> +# Simulate a power failure and then mount again the filesystem to replay the
> +# journal/log.
> +_flakey_drop_and_remount
> +
> +# The symlink should exist, with the name "baz" and its content must be
> +# "$SCRATCH_MNT/testdir/foo".
> +[ -L $SCRATCH_MNT/testdir/baz ] || echo "symlink 'baz' is missing"
> +echo "symlink content: $(readlink $SCRATCH_MNT/testdir/baz | _filter_scratch)"
> +
> +_unmount_flakey
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/690.out b/tests/generic/690.out
> new file mode 100644
> index 00000000..84be1247
> --- /dev/null
> +++ b/tests/generic/690.out
> @@ -0,0 +1,2 @@
> +QA output created by 690
> +symlink content: SCRATCH_MNT/testdir/foo
> -- 
> 2.35.1
> 

Re: [Resend PATCH] generic: test fsync of directory with renamed symlink

[Zorro Lang]

On Mon, May 09, 2022 at 11:03:17AM +0100, Filipe Manana wrote:
> On Tue, May 03, 2022 at 11:57:49AM +0100, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> > 
> > Test that if we fsync a directory, create a symlink inside it, rename
> > the symlink, fsync again the directory and then power fail, after the
> > filesystem is mounted again, the symlink exists with the new name and
> > it has the correct content.
> > 
> > This currently fails on btrfs, because the symlink ends up empty (which
> > is illegal on Linux), but it is fixed by kernel commit:
> > 
> >     d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> > 
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> > 
> > Resending as this was missed on the last update.
> > No changes, only rebased on the current 'for-next' branch.
> 
> Zorro,
> 
> This missed against the last fstests update.
> Did this patch fell through the cracks, or do you expect me to do something about it?
> 
> Should I rebase and resend again?

Oh, this patch is in my "still reviewing" list, I remembered I didn't see any
RVB/ACK for it, did I miss some emails? If so I missed something please feel
free to tell me :)

Thanks,
Zorro

> 
> Thanks.
> 
> > 
> >  tests/generic/690     | 89 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/690.out |  2 +
> >  2 files changed, 91 insertions(+)
> >  create mode 100755 tests/generic/690
> >  create mode 100644 tests/generic/690.out
> > 
> > diff --git a/tests/generic/690 b/tests/generic/690
> > new file mode 100755
> > index 00000000..0bf47dd7
> > --- /dev/null
> > +++ b/tests/generic/690
> > @@ -0,0 +1,89 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 SUSE Linux Products GmbH.  All Rights Reserved.
> > +#
> > +# FS QA Test 690
> > +#
> > +# Test that if we fsync a directory, create a symlink inside it, rename the
> > +# symlink, fsync again the directory and then power fail, after the filesystem
> > +# is mounted again, the symlink exists with the new name and it has the correct
> > +# content.
> > +#
> > +# On btrfs this used to result in the symlink being empty (i_size 0), and it was
> > +# fixed by kernel commit:
> > +#
> > +#    d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick log
> > +
> > +_cleanup()
> > +{
> > +	_cleanup_flakey
> > +	cd /
> > +	rm -r -f $tmp.*
> > +}
> > +
> > +. ./common/rc
> > +. ./common/filter
> > +. ./common/dmflakey
> > +
> > +# real QA test starts here
> > +
> > +_supported_fs generic
> > +_require_scratch
> > +_require_symlinks
> > +_require_dm_target flakey
> > +
> > +rm -f $seqres.full
> > +
> > +# f2fs doesn't support fs-op level transaction functionality, so it has no way
> > +# to persist all metadata updates in one transaction. We have to use its mount
> > +# option "fastboot" so that it triggers a metadata checkpoint to persist all
> > +# metadata updates that happen before a fsync call. Without this, after the
> > +# last fsync in the test, the symlink named "baz" will not exist.
> > +if [ $FSTYP = "f2fs" ]; then
> > +	export MOUNT_OPTIONS="-o fastboot $MOUNT_OPTIONS"
> > +fi
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_require_metadata_journaling $SCRATCH_DEV
> > +_init_flakey
> > +_mount_flakey
> > +
> > +# Create our test directory.
> > +mkdir $SCRATCH_MNT/testdir
> > +
> > +# Commit the current transaction and persist the directory.
> > +sync
> > +
> > +# Create a file in the test directory, so that the next fsync on the directory
> > +# actually does something (it logs the directory).
> > +echo -n > $SCRATCH_MNT/testdir/foo
> > +
> > +# Fsync the directory.
> > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> > +
> > +# Now create a symlink inside the test directory.
> > +ln -s $SCRATCH_MNT/testdir/foo $SCRATCH_MNT/testdir/bar
> > +
> > +# Rename the symlink.
> > +mv $SCRATCH_MNT/testdir/bar $SCRATCH_MNT/testdir/baz
> > +
> > +# Fsync again the directory.
> > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> > +
> > +# Simulate a power failure and then mount again the filesystem to replay the
> > +# journal/log.
> > +_flakey_drop_and_remount
> > +
> > +# The symlink should exist, with the name "baz" and its content must be
> > +# "$SCRATCH_MNT/testdir/foo".
> > +[ -L $SCRATCH_MNT/testdir/baz ] || echo "symlink 'baz' is missing"
> > +echo "symlink content: $(readlink $SCRATCH_MNT/testdir/baz | _filter_scratch)"
> > +
> > +_unmount_flakey
> > +
> > +# success, all done
> > +status=0
> > +exit
> > diff --git a/tests/generic/690.out b/tests/generic/690.out
> > new file mode 100644
> > index 00000000..84be1247
> > --- /dev/null
> > +++ b/tests/generic/690.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 690
> > +symlink content: SCRATCH_MNT/testdir/foo
> > -- 
> > 2.35.1
> > 
> 

Re: [Resend PATCH] generic: test fsync of directory with renamed symlink

[Filipe Manana]

On Mon, May 09, 2022 at 07:13:55PM +0800, Zorro Lang wrote:
> On Mon, May 09, 2022 at 11:03:17AM +0100, Filipe Manana wrote:
> > On Tue, May 03, 2022 at 11:57:49AM +0100, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > > 
> > > Test that if we fsync a directory, create a symlink inside it, rename
> > > the symlink, fsync again the directory and then power fail, after the
> > > filesystem is mounted again, the symlink exists with the new name and
> > > it has the correct content.
> > > 
> > > This currently fails on btrfs, because the symlink ends up empty (which
> > > is illegal on Linux), but it is fixed by kernel commit:
> > > 
> > >     d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> > > 
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> > > 
> > > Resending as this was missed on the last update.
> > > No changes, only rebased on the current 'for-next' branch.
> > 
> > Zorro,
> > 
> > This missed against the last fstests update.
> > Did this patch fell through the cracks, or do you expect me to do something about it?
> > 
> > Should I rebase and resend again?
> 
> Oh, this patch is in my "still reviewing" list, I remembered I didn't see any
> RVB/ACK for it, did I miss some emails? If so I missed something please feel
> free to tell me :)

There weren't any reviews, which is not uncommon.
In the past, the maintainers (Eryu, Dave) usually reviewed the patches themselves.

> 
> Thanks,
> Zorro
> 
> > 
> > Thanks.
> > 
> > > 
> > >  tests/generic/690     | 89 +++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/690.out |  2 +
> > >  2 files changed, 91 insertions(+)
> > >  create mode 100755 tests/generic/690
> > >  create mode 100644 tests/generic/690.out
> > > 
> > > diff --git a/tests/generic/690 b/tests/generic/690
> > > new file mode 100755
> > > index 00000000..0bf47dd7
> > > --- /dev/null
> > > +++ b/tests/generic/690
> > > @@ -0,0 +1,89 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2022 SUSE Linux Products GmbH.  All Rights Reserved.
> > > +#
> > > +# FS QA Test 690
> > > +#
> > > +# Test that if we fsync a directory, create a symlink inside it, rename the
> > > +# symlink, fsync again the directory and then power fail, after the filesystem
> > > +# is mounted again, the symlink exists with the new name and it has the correct
> > > +# content.
> > > +#
> > > +# On btrfs this used to result in the symlink being empty (i_size 0), and it was
> > > +# fixed by kernel commit:
> > > +#
> > > +#    d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick log
> > > +
> > > +_cleanup()
> > > +{
> > > +	_cleanup_flakey
> > > +	cd /
> > > +	rm -r -f $tmp.*
> > > +}
> > > +
> > > +. ./common/rc
> > > +. ./common/filter
> > > +. ./common/dmflakey
> > > +
> > > +# real QA test starts here
> > > +
> > > +_supported_fs generic
> > > +_require_scratch
> > > +_require_symlinks
> > > +_require_dm_target flakey
> > > +
> > > +rm -f $seqres.full
> > > +
> > > +# f2fs doesn't support fs-op level transaction functionality, so it has no way
> > > +# to persist all metadata updates in one transaction. We have to use its mount
> > > +# option "fastboot" so that it triggers a metadata checkpoint to persist all
> > > +# metadata updates that happen before a fsync call. Without this, after the
> > > +# last fsync in the test, the symlink named "baz" will not exist.
> > > +if [ $FSTYP = "f2fs" ]; then
> > > +	export MOUNT_OPTIONS="-o fastboot $MOUNT_OPTIONS"
> > > +fi
> > > +
> > > +_scratch_mkfs >>$seqres.full 2>&1
> > > +_require_metadata_journaling $SCRATCH_DEV
> > > +_init_flakey
> > > +_mount_flakey
> > > +
> > > +# Create our test directory.
> > > +mkdir $SCRATCH_MNT/testdir
> > > +
> > > +# Commit the current transaction and persist the directory.
> > > +sync
> > > +
> > > +# Create a file in the test directory, so that the next fsync on the directory
> > > +# actually does something (it logs the directory).
> > > +echo -n > $SCRATCH_MNT/testdir/foo
> > > +
> > > +# Fsync the directory.
> > > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> > > +
> > > +# Now create a symlink inside the test directory.
> > > +ln -s $SCRATCH_MNT/testdir/foo $SCRATCH_MNT/testdir/bar
> > > +
> > > +# Rename the symlink.
> > > +mv $SCRATCH_MNT/testdir/bar $SCRATCH_MNT/testdir/baz
> > > +
> > > +# Fsync again the directory.
> > > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> > > +
> > > +# Simulate a power failure and then mount again the filesystem to replay the
> > > +# journal/log.
> > > +_flakey_drop_and_remount
> > > +
> > > +# The symlink should exist, with the name "baz" and its content must be
> > > +# "$SCRATCH_MNT/testdir/foo".
> > > +[ -L $SCRATCH_MNT/testdir/baz ] || echo "symlink 'baz' is missing"
> > > +echo "symlink content: $(readlink $SCRATCH_MNT/testdir/baz | _filter_scratch)"
> > > +
> > > +_unmount_flakey
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/690.out b/tests/generic/690.out
> > > new file mode 100644
> > > index 00000000..84be1247
> > > --- /dev/null
> > > +++ b/tests/generic/690.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 690
> > > +symlink content: SCRATCH_MNT/testdir/foo
> > > -- 
> > > 2.35.1
> > > 
> > 

Re: [Resend PATCH] generic: test fsync of directory with renamed symlink

[David Disseldorp]

On Tue,  3 May 2022 11:57:49 +0100, fdmanana@kernel.org wrote:

> From: Filipe Manana <fdmanana@suse.com>
> 
> Test that if we fsync a directory, create a symlink inside it, rename
> the symlink, fsync again the directory and then power fail, after the
> filesystem is mounted again, the symlink exists with the new name and
> it has the correct content.
> 
> This currently fails on btrfs, because the symlink ends up empty (which
> is illegal on Linux), but it is fixed by kernel commit:
> 
>     d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

This looks fine and works for me.
Reviewed-by: David Disseldorp <ddiss@suse.de>

...
> +mkdir $SCRATCH_MNT/testdir

nit: It's worth quoting the variable here (and elsewhere). That said, I
highly doubt anyone is using a SCRATCH_MNT with a space in it, so it
should be okay as is.

Cheers, David

Re: [Resend PATCH] generic: test fsync of directory with renamed symlink

[Filipe Manana]

On Mon, May 9, 2022 at 1:15 PM David Disseldorp <ddiss@suse.de> wrote:
>
> On Tue,  3 May 2022 11:57:49 +0100, fdmanana@kernel.org wrote:
>
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > Test that if we fsync a directory, create a symlink inside it, rename
> > the symlink, fsync again the directory and then power fail, after the
> > filesystem is mounted again, the symlink exists with the new name and
> > it has the correct content.
> >
> > This currently fails on btrfs, because the symlink ends up empty (which
> > is illegal on Linux), but it is fixed by kernel commit:
> >
> >     d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
>
> This looks fine and works for me.
> Reviewed-by: David Disseldorp <ddiss@suse.de>
>
> ...
> > +mkdir $SCRATCH_MNT/testdir
>
> nit: It's worth quoting the variable here (and elsewhere). That said, I
> highly doubt anyone is using a SCRATCH_MNT with a space in it, so it
> should be okay as is.

Indeed.
I'll send an updated version later.

Thanks David.

>
> Cheers, David

Re: [Resend PATCH] generic: test fsync of directory with renamed symlink

[Zorro Lang]

On Mon, May 09, 2022 at 12:32:43PM +0100, Filipe Manana wrote:
> On Mon, May 09, 2022 at 07:13:55PM +0800, Zorro Lang wrote:
> > On Mon, May 09, 2022 at 11:03:17AM +0100, Filipe Manana wrote:
> > > On Tue, May 03, 2022 at 11:57:49AM +0100, fdmanana@kernel.org wrote:
> > > > From: Filipe Manana <fdmanana@suse.com>
> > > > 
> > > > Test that if we fsync a directory, create a symlink inside it, rename
> > > > the symlink, fsync again the directory and then power fail, after the
> > > > filesystem is mounted again, the symlink exists with the new name and
> > > > it has the correct content.
> > > > 
> > > > This currently fails on btrfs, because the symlink ends up empty (which
> > > > is illegal on Linux), but it is fixed by kernel commit:
> > > > 
> > > >     d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> > > > 
> > > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > > ---
> > > > 
> > > > Resending as this was missed on the last update.
> > > > No changes, only rebased on the current 'for-next' branch.
> > > 
> > > Zorro,
> > > 
> > > This missed against the last fstests update.
> > > Did this patch fell through the cracks, or do you expect me to do something about it?
> > > 
> > > Should I rebase and resend again?
> > 
> > Oh, this patch is in my "still reviewing" list, I remembered I didn't see any
> > RVB/ACK for it, did I miss some emails? If so I missed something please feel
> > free to tell me :)
> 
> There weren't any reviews, which is not uncommon.
> In the past, the maintainers (Eryu, Dave) usually reviewed the patches themselves.

Hi Filipe,

Actually I've checked it from fstests common side, but generally I hope to give a
specific fs case several days to see if related experts would like to give it some
review points at first. If no response, I'll ask and think about merging it after
basic reviewing and testing. You resent it on May 03, if you're hurry to merge a
btrfs patch when you sent it, better to CC some related folks to help that out,
that'll help to facilitate the process effectively, thanks for your understanding :)

Thanks,
Zorro

> 
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > Thanks.
> > > 
> > > > 
> > > >  tests/generic/690     | 89 +++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/generic/690.out |  2 +
> > > >  2 files changed, 91 insertions(+)
> > > >  create mode 100755 tests/generic/690
> > > >  create mode 100644 tests/generic/690.out
> > > > 
> > > > diff --git a/tests/generic/690 b/tests/generic/690
> > > > new file mode 100755
> > > > index 00000000..0bf47dd7
> > > > --- /dev/null
> > > > +++ b/tests/generic/690
> > > > @@ -0,0 +1,89 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2022 SUSE Linux Products GmbH.  All Rights Reserved.
> > > > +#
> > > > +# FS QA Test 690
> > > > +#
> > > > +# Test that if we fsync a directory, create a symlink inside it, rename the
> > > > +# symlink, fsync again the directory and then power fail, after the filesystem
> > > > +# is mounted again, the symlink exists with the new name and it has the correct
> > > > +# content.
> > > > +#
> > > > +# On btrfs this used to result in the symlink being empty (i_size 0), and it was
> > > > +# fixed by kernel commit:
> > > > +#
> > > > +#    d0e64a981fd841 ("btrfs: always log symlinks in full mode")
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest auto quick log
> > > > +
> > > > +_cleanup()
> > > > +{
> > > > +	_cleanup_flakey
> > > > +	cd /
> > > > +	rm -r -f $tmp.*
> > > > +}
> > > > +
> > > > +. ./common/rc
> > > > +. ./common/filter
> > > > +. ./common/dmflakey
> > > > +
> > > > +# real QA test starts here
> > > > +
> > > > +_supported_fs generic
> > > > +_require_scratch
> > > > +_require_symlinks
> > > > +_require_dm_target flakey
> > > > +
> > > > +rm -f $seqres.full
> > > > +
> > > > +# f2fs doesn't support fs-op level transaction functionality, so it has no way
> > > > +# to persist all metadata updates in one transaction. We have to use its mount
> > > > +# option "fastboot" so that it triggers a metadata checkpoint to persist all
> > > > +# metadata updates that happen before a fsync call. Without this, after the
> > > > +# last fsync in the test, the symlink named "baz" will not exist.
> > > > +if [ $FSTYP = "f2fs" ]; then
> > > > +	export MOUNT_OPTIONS="-o fastboot $MOUNT_OPTIONS"
> > > > +fi
> > > > +
> > > > +_scratch_mkfs >>$seqres.full 2>&1
> > > > +_require_metadata_journaling $SCRATCH_DEV
> > > > +_init_flakey
> > > > +_mount_flakey
> > > > +
> > > > +# Create our test directory.
> > > > +mkdir $SCRATCH_MNT/testdir
> > > > +
> > > > +# Commit the current transaction and persist the directory.
> > > > +sync
> > > > +
> > > > +# Create a file in the test directory, so that the next fsync on the directory
> > > > +# actually does something (it logs the directory).
> > > > +echo -n > $SCRATCH_MNT/testdir/foo
> > > > +
> > > > +# Fsync the directory.
> > > > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> > > > +
> > > > +# Now create a symlink inside the test directory.
> > > > +ln -s $SCRATCH_MNT/testdir/foo $SCRATCH_MNT/testdir/bar
> > > > +
> > > > +# Rename the symlink.
> > > > +mv $SCRATCH_MNT/testdir/bar $SCRATCH_MNT/testdir/baz
> > > > +
> > > > +# Fsync again the directory.
> > > > +$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir
> > > > +
> > > > +# Simulate a power failure and then mount again the filesystem to replay the
> > > > +# journal/log.
> > > > +_flakey_drop_and_remount
> > > > +
> > > > +# The symlink should exist, with the name "baz" and its content must be
> > > > +# "$SCRATCH_MNT/testdir/foo".
> > > > +[ -L $SCRATCH_MNT/testdir/baz ] || echo "symlink 'baz' is missing"
> > > > +echo "symlink content: $(readlink $SCRATCH_MNT/testdir/baz | _filter_scratch)"
> > > > +
> > > > +_unmount_flakey
> > > > +
> > > > +# success, all done
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/generic/690.out b/tests/generic/690.out
> > > > new file mode 100644
> > > > index 00000000..84be1247
> > > > --- /dev/null
> > > > +++ b/tests/generic/690.out
> > > > @@ -0,0 +1,2 @@
> > > > +QA output created by 690
> > > > +symlink content: SCRATCH_MNT/testdir/foo
> > > > -- 
> > > > 2.35.1
> > > > 
> > > 
>