| Message ID | 20230130145639.30027-1-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] generic/707: Test moving directory while being grown | expand |
On Mon, Jan 30, 2023 at 03:56:39PM +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. > > Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > Signed-off-by: Jan Kara <jack@suse.cz> > --- > tests/generic/707 | 63 +++++++++++++++++++++++++++++++++++++++++++ > tests/generic/707.out | 2 ++ > 2 files changed, 65 insertions(+) > create mode 100755 tests/generic/707 > create mode 100644 tests/generic/707.out > > Changes since v1: > * Added TIME_FACTOR multiplier > * Handle background process in _cleanup > * Use absolute paths where possible > > diff --git a/tests/generic/707 b/tests/generic/707 > new file mode 100755 > index 000000000000..d1d299563813 > --- /dev/null > +++ b/tests/generic/707 > @@ -0,0 +1,63 @@ > +#! /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 > +# ^ A trailing whitespace at here :) > +. ./common/preamble > +_begin_fstest auto > + > +_supported_fs generic > +_require_scratch > + > +_scratch_mkfs >>$seqres.full 2>&1 > +_scratch_mount > + > +_cleanup() > +{ > + if [ -n "$BGPID" ]; then > + # Stop background process > + kill -9 $BGPID &>/dev/null > + wait > + fi Neet to keep the code in common/preamble:_cleanup(), if you cover it: cd / rm -f $tmp.* especially you use pushd and popd. I forgot to ask, is this case written for someone known fix or change? If so, better to use _fixed_by_kernel_commit or _wants_kernel_commit. > +} > + > +# Loop multiple times trying to hit the race > +loops=$((100*TIME_FACTOR)) > +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 $SCRATCH_MNT/dir$i > +done > + > +for (( l = 0; l < $loops; l++ )); do > + mkdir $SCRATCH_MNT/dir0/dir > + pushd $SCRATCH_MNT/dir0/dir &>/dev/null > + create_files & > + BGPID=$! > + popd &>/dev/null > + for (( i = 0; i < $moves; i++ )); do > + mv $SCRATCH_MNT/dir$i/dir $SCRATCH_MNT/dir$((i+1))/dir > + done > + wait > + BGPID="" unset BGPID might be better, but anyway :) Others looks good to me. Thanks, Zorro > + rm -r $SCRATCH_MNT/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 >
On Mon, Jan 30, 2023 at 03:56:39PM +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. > > Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > Signed-off-by: Jan Kara <jack@suse.cz> With Zoro's suggested changes, this looks fine. Reviewed-by: Bill O'Donnell <bodonnel@redhat.com> > --- > tests/generic/707 | 63 +++++++++++++++++++++++++++++++++++++++++++ > tests/generic/707.out | 2 ++ > 2 files changed, 65 insertions(+) > create mode 100755 tests/generic/707 > create mode 100644 tests/generic/707.out > > Changes since v1: > * Added TIME_FACTOR multiplier > * Handle background process in _cleanup > * Use absolute paths where possible > > diff --git a/tests/generic/707 b/tests/generic/707 > new file mode 100755 > index 000000000000..d1d299563813 > --- /dev/null > +++ b/tests/generic/707 > @@ -0,0 +1,63 @@ > +#! /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 > + > +_cleanup() > +{ > + if [ -n "$BGPID" ]; then > + # Stop background process > + kill -9 $BGPID &>/dev/null > + wait > + fi > +} > + > +# Loop multiple times trying to hit the race > +loops=$((100*TIME_FACTOR)) > +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 $SCRATCH_MNT/dir$i > +done > + > +for (( l = 0; l < $loops; l++ )); do > + mkdir $SCRATCH_MNT/dir0/dir > + pushd $SCRATCH_MNT/dir0/dir &>/dev/null > + create_files & > + BGPID=$! > + popd &>/dev/null > + for (( i = 0; i < $moves; i++ )); do > + mv $SCRATCH_MNT/dir$i/dir $SCRATCH_MNT/dir$((i+1))/dir > + done > + wait > + BGPID="" > + rm -r $SCRATCH_MNT/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 >
On Tue 31-01-23 00:04:01, Zorro Lang wrote: > On Mon, Jan 30, 2023 at 03:56:39PM +0100, Jan Kara wrote: > > +. ./common/preamble > > +_begin_fstest auto > > + > > +_supported_fs generic > > +_require_scratch > > + > > +_scratch_mkfs >>$seqres.full 2>&1 > > +_scratch_mount > > + > > +_cleanup() > > +{ > > + if [ -n "$BGPID" ]; then > > + # Stop background process > > + kill -9 $BGPID &>/dev/null > > + wait > > + fi > > Neet to keep the code in common/preamble:_cleanup(), if you cover it: > > cd / > rm -f $tmp.* > > especially you use pushd and popd. OK, done. > I forgot to ask, is this case written for someone known fix or change? If so, > better to use _fixed_by_kernel_commit or _wants_kernel_commit. Yes, good point. It tests for UDF & ext4 corruption issue. Ext4 commit is not yet merged but I'll add there something. > > +} > > + > > +# Loop multiple times trying to hit the race > > +loops=$((100*TIME_FACTOR)) > > +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 $SCRATCH_MNT/dir$i > > +done > > + > > +for (( l = 0; l < $loops; l++ )); do > > + mkdir $SCRATCH_MNT/dir0/dir > > + pushd $SCRATCH_MNT/dir0/dir &>/dev/null > > + create_files & > > + BGPID=$! > > + popd &>/dev/null > > + for (( i = 0; i < $moves; i++ )); do > > + mv $SCRATCH_MNT/dir$i/dir $SCRATCH_MNT/dir$((i+1))/dir > > + done > > + wait > > + BGPID="" > > unset BGPID might be better, but anyway :) Yeah, nicer. Done. Honza
diff --git a/tests/generic/707 b/tests/generic/707 new file mode 100755 index 000000000000..d1d299563813 --- /dev/null +++ b/tests/generic/707 @@ -0,0 +1,63 @@ +#! /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 + +_cleanup() +{ + if [ -n "$BGPID" ]; then + # Stop background process + kill -9 $BGPID &>/dev/null + wait + fi +} + +# Loop multiple times trying to hit the race +loops=$((100*TIME_FACTOR)) +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 $SCRATCH_MNT/dir$i +done + +for (( l = 0; l < $loops; l++ )); do + mkdir $SCRATCH_MNT/dir0/dir + pushd $SCRATCH_MNT/dir0/dir &>/dev/null + create_files & + BGPID=$! + popd &>/dev/null + for (( i = 0; i < $moves; i++ )); do + mv $SCRATCH_MNT/dir$i/dir $SCRATCH_MNT/dir$((i+1))/dir + done + wait + BGPID="" + rm -r $SCRATCH_MNT/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