diff mbox series

generic/707: Test moving directory while being grown

Message ID 20230126151512.5438-1-jack@suse.cz (mailing list archive)
State New, archived
Headers show
Series generic/707: Test moving directory while being grown | expand

Commit Message

Jan Kara Jan. 26, 2023, 3:15 p.m. UTC 
Test how the filesystem can handle moving a directory to a different
directory (so that parent pointer gets updated) while it is grown. Ext4
and UDF had a bug where if the directory got converted to a different
type due to growth while rename is running, the filesystem got
corrupted.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 tests/generic/707     | 54 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/707.out |  2 ++
 2 files changed, 56 insertions(+)
 create mode 100755 tests/generic/707
 create mode 100644 tests/generic/707.out

Comments

Bill O'Donnell Jan. 26, 2023, 3:37 p.m. UTC | #1 
On Thu, Jan 26, 2023 at 04:15:12PM +0100, Jan Kara wrote:
> Test how the filesystem can handle moving a directory to a different
> directory (so that parent pointer gets updated) while it is grown. Ext4
> and UDF had a bug where if the directory got converted to a different
> type due to growth while rename is running, the filesystem got
> corrupted.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Looks fine to me.

Reviewed-by: Bill O'Donnell <bodonnel@redhat.com>

> ---
>  tests/generic/707     | 54 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/707.out |  2 ++
>  2 files changed, 56 insertions(+)
>  create mode 100755 tests/generic/707
>  create mode 100644 tests/generic/707.out
> 
> diff --git a/tests/generic/707 b/tests/generic/707
> new file mode 100755
> index 000000000000..25cbb7e98a23
> --- /dev/null
> +++ b/tests/generic/707
> @@ -0,0 +1,54 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2023 Jan Kara, SUSE Linux.  All Rights Reserved.
> +#
> +# FS QA Test 707
> +#
> +# This is a test verifying whether the filesystem can gracefully handle
> +# modifying of a directory while it is being moved, in particular the cases
> +# where directory format changes
> +# 
> +. ./common/preamble
> +_begin_fstest auto
> +
> +_supported_fs generic
> +_require_scratch
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +cd $SCRATCH_MNT
> +
> +# Loop multiple times trying to hit the race
> +loops=100
> +files=500
> +moves=500
> +
> +create_files()
> +{
> +	# We use slightly longer file name to make directory grow faster and
> +	# hopefully convert between various types
> +	for (( i = 0; i < $files; i++ )); do
> +		touch somewhatlongerfilename$i
> +	done
> +}
> +
> +for (( i = 0; i <= $moves; i++ )); do
> +	mkdir dir$i
> +done
> +
> +for (( l = 0; l < $loops; l++ )); do
> +	mkdir dir0/dir
> +	pushd dir0/dir &>/dev/null
> +	create_files &
> +	popd &>/dev/null
> +	for (( i = 0; i < $moves; i++ )); do
> +		mv dir$i/dir dir$((i+1))/dir
> +	done
> +	wait
> +	rm -r dir$moves/dir
> +done
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/707.out b/tests/generic/707.out
> new file mode 100644
> index 000000000000..8e57a1d8c971
> --- /dev/null
> +++ b/tests/generic/707.out
> @@ -0,0 +1,2 @@
> +QA output created by 707
> +Silence is golden
> -- 
> 2.35.3
>
Zorro Lang Jan. 30, 2023, 5 a.m. UTC | #2 
On Thu, Jan 26, 2023 at 04:15:12PM +0100, Jan Kara wrote:
> Test how the filesystem can handle moving a directory to a different
> directory (so that parent pointer gets updated) while it is grown. Ext4
> and UDF had a bug where if the directory got converted to a different
> type due to growth while rename is running, the filesystem got
> corrupted.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  tests/generic/707     | 54 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/707.out |  2 ++
>  2 files changed, 56 insertions(+)
>  create mode 100755 tests/generic/707
>  create mode 100644 tests/generic/707.out
> 
> diff --git a/tests/generic/707 b/tests/generic/707
> new file mode 100755
> index 000000000000..25cbb7e98a23
> --- /dev/null
> +++ b/tests/generic/707
> @@ -0,0 +1,54 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2023 Jan Kara, SUSE Linux.  All Rights Reserved.
> +#
> +# FS QA Test 707
> +#
> +# This is a test verifying whether the filesystem can gracefully handle
> +# modifying of a directory while it is being moved, in particular the cases
> +# where directory format changes
> +# 
> +. ./common/preamble
> +_begin_fstest auto
> +
> +_supported_fs generic
> +_require_scratch
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +cd $SCRATCH_MNT

Is this necessary? Generally we use absolute path, e.g.

xxxfile=$SCRATCH_MNT/xxxxxxx
touch $xxxfile



> +
> +# Loop multiple times trying to hit the race
> +loops=100
> +files=500
> +moves=500

You might would like to try LOAD_FACTOR or TIME_FACTOR parameters?

> +
> +create_files()
> +{
> +	# We use slightly longer file name to make directory grow faster and
> +	# hopefully convert between various types
> +	for (( i = 0; i < $files; i++ )); do
> +		touch somewhatlongerfilename$i
> +	done
> +}
> +
> +for (( i = 0; i <= $moves; i++ )); do
> +	mkdir dir$i
> +done
> +
> +for (( l = 0; l < $loops; l++ )); do
> +	mkdir dir0/dir
> +	pushd dir0/dir &>/dev/null

Use absolute path can help save the pushd&popd operations.

> +	create_files &

If there're background processes, we should deal with it in _cleanup() clearly.

Thanks,
Zorro

> +	popd &>/dev/null
> +	for (( i = 0; i < $moves; i++ )); do
> +		mv dir$i/dir dir$((i+1))/dir
> +	done
> +	wait
> +	rm -r dir$moves/dir
> +done
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/707.out b/tests/generic/707.out
> new file mode 100644
> index 000000000000..8e57a1d8c971
> --- /dev/null
> +++ b/tests/generic/707.out
> @@ -0,0 +1,2 @@
> +QA output created by 707
> +Silence is golden
> -- 
> 2.35.3
>
Jan Kara Jan. 30, 2023, 11:29 a.m. UTC | #3 
On Mon 30-01-23 13:00:18, Zorro Lang wrote:
> On Thu, Jan 26, 2023 at 04:15:12PM +0100, Jan Kara wrote:
> > Test how the filesystem can handle moving a directory to a different
> > directory (so that parent pointer gets updated) while it is grown. Ext4
> > and UDF had a bug where if the directory got converted to a different
> > type due to growth while rename is running, the filesystem got
> > corrupted.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  tests/generic/707     | 54 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/707.out |  2 ++
> >  2 files changed, 56 insertions(+)
> >  create mode 100755 tests/generic/707
> >  create mode 100644 tests/generic/707.out
> > 
> > diff --git a/tests/generic/707 b/tests/generic/707
> > new file mode 100755
> > index 000000000000..25cbb7e98a23
> > --- /dev/null
> > +++ b/tests/generic/707
> > @@ -0,0 +1,54 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2023 Jan Kara, SUSE Linux.  All Rights Reserved.
> > +#
> > +# FS QA Test 707
> > +#
> > +# This is a test verifying whether the filesystem can gracefully handle
> > +# modifying of a directory while it is being moved, in particular the cases
> > +# where directory format changes
> > +# 
> > +. ./common/preamble
> > +_begin_fstest auto
> > +
> > +_supported_fs generic
> > +_require_scratch
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_scratch_mount
> > +
> > +cd $SCRATCH_MNT
> 
> Is this necessary? Generally we use absolute path, e.g.
> 
> xxxfile=$SCRATCH_MNT/xxxxxxx
> touch $xxxfile

So here I can use absolute path without a problem.

> > +
> > +# Loop multiple times trying to hit the race
> > +loops=100
> > +files=500
> > +moves=500
> 
> You might would like to try LOAD_FACTOR or TIME_FACTOR parameters?

Will do.

> > +
> > +create_files()
> > +{
> > +	# We use slightly longer file name to make directory grow faster and
> > +	# hopefully convert between various types
> > +	for (( i = 0; i < $files; i++ )); do
> > +		touch somewhatlongerfilename$i
> > +	done
> > +}
> > +
> > +for (( i = 0; i <= $moves; i++ )); do
> > +	mkdir dir$i
> > +done
> > +
> > +for (( l = 0; l < $loops; l++ )); do
> > +	mkdir dir0/dir
> > +	pushd dir0/dir &>/dev/null
> 
> Use absolute path can help save the pushd&popd operations.

Not really. The thing is the background process needs to start inside the
directory that is being moved by the foreground process. Because the
absolute path constantly changes due to movement it is not really possible
to implement this without changing cwd.

> > +	create_files &
> 
> If there're background processes, we should deal with it in _cleanup() clearly.

Right, I'll add that. Thanks for review!

								Honza
diff mbox series

Patch

diff --git a/tests/generic/707 b/tests/generic/707
new file mode 100755
index 000000000000..25cbb7e98a23
--- /dev/null
+++ b/tests/generic/707
@@ -0,0 +1,54 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2023 Jan Kara, SUSE Linux.  All Rights Reserved.
+#
+# FS QA Test 707
+#
+# This is a test verifying whether the filesystem can gracefully handle
+# modifying of a directory while it is being moved, in particular the cases
+# where directory format changes
+# 
+. ./common/preamble
+_begin_fstest auto
+
+_supported_fs generic
+_require_scratch
+
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
+cd $SCRATCH_MNT
+
+# Loop multiple times trying to hit the race
+loops=100
+files=500
+moves=500
+
+create_files()
+{
+	# We use slightly longer file name to make directory grow faster and
+	# hopefully convert between various types
+	for (( i = 0; i < $files; i++ )); do
+		touch somewhatlongerfilename$i
+	done
+}
+
+for (( i = 0; i <= $moves; i++ )); do
+	mkdir dir$i
+done
+
+for (( l = 0; l < $loops; l++ )); do
+	mkdir dir0/dir
+	pushd dir0/dir &>/dev/null
+	create_files &
+	popd &>/dev/null
+	for (( i = 0; i < $moves; i++ )); do
+		mv dir$i/dir dir$((i+1))/dir
+	done
+	wait
+	rm -r dir$moves/dir
+done
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/707.out b/tests/generic/707.out
new file mode 100644
index 000000000000..8e57a1d8c971
--- /dev/null
+++ b/tests/generic/707.out
@@ -0,0 +1,2 @@ 
+QA output created by 707
+Silence is golden