* [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function @ 2018-06-18 17:44 Marco Benatto 2018-06-18 17:44 ` [PATCH 2/2] xfstests: Test root inode parent pointer repairing Marco Benatto 2018-06-18 19:11 ` [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function Darrick J. Wong 0 siblings, 2 replies; 10+ messages in thread From: Marco Benatto @ 2018-06-18 17:44 UTC (permalink / raw) To: fstests; +Cc: linux-xfs, mbenatto, darrick.wong, sandeen Move get_sfdir_prefix function from xfs/278 to commom/xfs and rename it to _scratch_get_sfdir_prefix so it can be used in other xfs tests. This commit also changes xfs/278 to make use of _scratch_get_sfdir_prefix instead previous one. Signed-off-by: Marco Benatto <mbenatto@redhat.com> --- common/xfs | 15 +++++++++++++++ tests/xfs/278 | 16 +--------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/common/xfs b/common/xfs index 55cfa94..ecf54bb 100644 --- a/common/xfs +++ b/common/xfs @@ -760,3 +760,18 @@ _require_xfs_spaceman_command() _notrun "xfs_spaceman $command doesn't support $param" fi } + +_scratch_get_sfdir_prefix() { + local dir_ino="$1" + + for prefix in "u.sfdir3" "u.sfdir2" "u3.sfdir3"; do + if [ -n "$(_scratch_xfs_get_metadata_field \ + "${prefix}.hdr.parent.i4" \ + "inode ${dir_ino}")" ]; then + echo "${prefix}" + return 0 + fi + done + _scratch_xfs_db -c "inode ${dir_ino}" -c 'p' >> $seqres.full + return 1 +} diff --git a/tests/xfs/278 b/tests/xfs/278 index 3d2a846..c208e61 100755 --- a/tests/xfs/278 +++ b/tests/xfs/278 @@ -46,25 +46,11 @@ _scratch_unmount echo "Silence is goodness..." -get_sfdir_prefix() { - local dir_ino="$1" - - for prefix in "u.sfdir3" "u.sfdir2" "u3.sfdir3"; do - if [ -n "$(_scratch_xfs_get_metadata_field \ - "${prefix}.hdr.parent.i4" \ - "inode ${dir_ino}")" ]; then - echo "${prefix}" - return 0 - fi - done - _scratch_xfs_db -c "inode ${dir_ino}" -c 'p' >> $seqres.full - return 1 -} set_ifield() { _scratch_xfs_set_metadata_field "$1" 0 "inode $2" >> $seqres.full } -sfdir_prefix="$(get_sfdir_prefix "$DIR_INO" || \ +sfdir_prefix="$(_scratch_get_sfdir_prefix "$DIR_INO" || \ _fail "Cannot determine sfdir prefix")" # Corrupt DIR -- 2.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-18 17:44 [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function Marco Benatto @ 2018-06-18 17:44 ` Marco Benatto 2018-06-18 19:26 ` Darrick J. Wong ` (2 more replies) 2018-06-18 19:11 ` [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function Darrick J. Wong 1 sibling, 3 replies; 10+ messages in thread From: Marco Benatto @ 2018-06-18 17:44 UTC (permalink / raw) To: fstests; +Cc: linux-xfs, mbenatto, darrick.wong, sandeen Recently we found out xfs_repair were not repairing root inode parent pointer when root inode is on short-form and parent points to an invalid inode number (refer to: "xfs_repair: Fix root inode's parent when it's bogus for sf directory" on xfs-devel list). This test checks if xfs_repair successfully repair the filesystem in the scenario mentioned before. Signed-off-by: Marco Benatto <mbenatto@redhat.com> --- tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/xfs/450.out | 1 + tests/xfs/group | 1 + 3 files changed, 55 insertions(+) create mode 100755 tests/xfs/450 create mode 100644 tests/xfs/450.out diff --git a/tests/xfs/450 b/tests/xfs/450 new file mode 100755 index 0000000..dc7f244 --- /dev/null +++ b/tests/xfs/450 @@ -0,0 +1,53 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. +# +# FS QA Test 450 +# +# Make sure xfs_repair can repair root inode parent's pointer +# when it contains a bogus ino when it's using shot form directory +# +seq=`basename $0` +seqres=$RESULT_DIR/$seq + +status=1 # failure is the default! + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +# Modify as appropriate. +_supported_fs xfs +_supported_os Linux +_require_scratch + + +_scratch_mkfs >> /dev/null 2>&1 + +rootino=$(_scratch_xfs_get_metadata_field 'rootino' 'sb 0') + +prefix=$(_scratch_get_sfdir_prefix ${rootino} || \ + _fail "Cannot determine sfdir prefix") + + +# Corrupt root inode parent pointer +_scratch_xfs_set_metadata_field "${prefix}.hdr.parent.i4" 0 "inode ${rootino}"\ + >> $seqres.full + +_scratch_xfs_repair >> $seqres.full 2>&1 + +_scratch_xfs_repair -n >> $seqres.full 2>&1 + +if [ $? -eq 1 ] +then + _fail "xfs_repair failed to repair filesystem" +fi + +echo "OK" +status=0 +exit diff --git a/tests/xfs/450.out b/tests/xfs/450.out new file mode 100644 index 0000000..d86bac9 --- /dev/null +++ b/tests/xfs/450.out @@ -0,0 +1 @@ +OK diff --git a/tests/xfs/group b/tests/xfs/group index 932ab90..3a1db0b 100644 --- a/tests/xfs/group +++ b/tests/xfs/group @@ -447,3 +447,4 @@ 447 auto mount 448 auto quick fuzzers 449 auto quick +450 auto quick -- 2.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-18 17:44 ` [PATCH 2/2] xfstests: Test root inode parent pointer repairing Marco Benatto @ 2018-06-18 19:26 ` Darrick J. Wong 2018-06-20 1:48 ` Dave Chinner 2018-06-22 2:37 ` Eryu Guan 2 siblings, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2018-06-18 19:26 UTC (permalink / raw) To: Marco Benatto; +Cc: fstests, linux-xfs, sandeen On Mon, Jun 18, 2018 at 02:44:33PM -0300, Marco Benatto wrote: > Recently we found out xfs_repair were not repairing > root inode parent pointer when root inode is on short-form > and parent points to an invalid inode number (refer to: > "xfs_repair: Fix root inode's parent when it's bogus for sf > directory" on xfs-devel list). > > This test checks if xfs_repair successfully repair the > filesystem in the scenario mentioned before. > > Signed-off-by: Marco Benatto <mbenatto@redhat.com> Looks ok, I think... Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/450.out | 1 + > tests/xfs/group | 1 + > 3 files changed, 55 insertions(+) > create mode 100755 tests/xfs/450 > create mode 100644 tests/xfs/450.out > > diff --git a/tests/xfs/450 b/tests/xfs/450 > new file mode 100755 > index 0000000..dc7f244 > --- /dev/null > +++ b/tests/xfs/450 > @@ -0,0 +1,53 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. > +# > +# FS QA Test 450 > +# > +# Make sure xfs_repair can repair root inode parent's pointer > +# when it contains a bogus ino when it's using shot form directory > +# > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > + > +status=1 # failure is the default! > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > + > +# Modify as appropriate. > +_supported_fs xfs > +_supported_os Linux > +_require_scratch > + > + > +_scratch_mkfs >> /dev/null 2>&1 > + > +rootino=$(_scratch_xfs_get_metadata_field 'rootino' 'sb 0') > + > +prefix=$(_scratch_get_sfdir_prefix ${rootino} || \ > + _fail "Cannot determine sfdir prefix") > + > + > +# Corrupt root inode parent pointer > +_scratch_xfs_set_metadata_field "${prefix}.hdr.parent.i4" 0 "inode ${rootino}"\ > + >> $seqres.full > + > +_scratch_xfs_repair >> $seqres.full 2>&1 > + > +_scratch_xfs_repair -n >> $seqres.full 2>&1 > + > +if [ $? -eq 1 ] > +then > + _fail "xfs_repair failed to repair filesystem" > +fi > + > +echo "OK" > +status=0 > +exit > diff --git a/tests/xfs/450.out b/tests/xfs/450.out > new file mode 100644 > index 0000000..d86bac9 > --- /dev/null > +++ b/tests/xfs/450.out > @@ -0,0 +1 @@ > +OK > diff --git a/tests/xfs/group b/tests/xfs/group > index 932ab90..3a1db0b 100644 > --- a/tests/xfs/group > +++ b/tests/xfs/group > @@ -447,3 +447,4 @@ > 447 auto mount > 448 auto quick fuzzers > 449 auto quick > +450 auto quick > -- > 2.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-18 17:44 ` [PATCH 2/2] xfstests: Test root inode parent pointer repairing Marco Benatto 2018-06-18 19:26 ` Darrick J. Wong @ 2018-06-20 1:48 ` Dave Chinner 2018-06-22 2:37 ` Eryu Guan 2 siblings, 0 replies; 10+ messages in thread From: Dave Chinner @ 2018-06-20 1:48 UTC (permalink / raw) To: Marco Benatto; +Cc: fstests, linux-xfs, darrick.wong, sandeen On Mon, Jun 18, 2018 at 02:44:33PM -0300, Marco Benatto wrote: > Recently we found out xfs_repair were not repairing > root inode parent pointer when root inode is on short-form > and parent points to an invalid inode number (refer to: > "xfs_repair: Fix root inode's parent when it's bogus for sf > directory" on xfs-devel list). > > This test checks if xfs_repair successfully repair the > filesystem in the scenario mentioned before. > > Signed-off-by: Marco Benatto <mbenatto@redhat.com> > --- > tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/450.out | 1 + > tests/xfs/group | 1 + > 3 files changed, 55 insertions(+) > create mode 100755 tests/xfs/450 > create mode 100644 tests/xfs/450.out > > diff --git a/tests/xfs/450 b/tests/xfs/450 > new file mode 100755 > index 0000000..dc7f244 > --- /dev/null > +++ b/tests/xfs/450 > @@ -0,0 +1,53 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. > +# > +# FS QA Test 450 > +# > +# Make sure xfs_repair can repair root inode parent's pointer > +# when it contains a bogus ino when it's using shot form directory > +# > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > + > +status=1 # failure is the default! > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > + > +# Modify as appropriate. > +_supported_fs xfs > +_supported_os Linux > +_require_scratch > + > + > +_scratch_mkfs >> /dev/null 2>&1 > + > +rootino=$(_scratch_xfs_get_metadata_field 'rootino' 'sb 0') > + > +prefix=$(_scratch_get_sfdir_prefix ${rootino} || \ > + _fail "Cannot determine sfdir prefix") > + > + > +# Corrupt root inode parent pointer > +_scratch_xfs_set_metadata_field "${prefix}.hdr.parent.i4" 0 "inode ${rootino}"\ > + >> $seqres.full > + > +_scratch_xfs_repair >> $seqres.full 2>&1 > + > +_scratch_xfs_repair -n >> $seqres.full 2>&1 > + > +if [ $? -eq 1 ] > +then > + _fail "xfs_repair failed to repair filesystem" > +fi Isn't this last "filesystem is not corrupt" check run by the harness via _check_scratch_fs()? (Also, "if [ .. ]; then" is the preferred fstests style) > + > +echo "OK" There's no test output, so the normal thing to do is this after setting up the test: echo "Silence is golden" We do that early on so that when something goes wrong the bad output file immediately tells you that no output was expected. > @@ -447,3 +447,4 @@ > 447 auto mount > 448 auto quick fuzzers > 449 auto quick > +450 auto quick also the metadata and repair groups should be added here. Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-18 17:44 ` [PATCH 2/2] xfstests: Test root inode parent pointer repairing Marco Benatto 2018-06-18 19:26 ` Darrick J. Wong 2018-06-20 1:48 ` Dave Chinner @ 2018-06-22 2:37 ` Eryu Guan 2018-06-22 2:58 ` Eric Sandeen 2 siblings, 1 reply; 10+ messages in thread From: Eryu Guan @ 2018-06-22 2:37 UTC (permalink / raw) To: Marco Benatto; +Cc: fstests, linux-xfs, darrick.wong, sandeen On Mon, Jun 18, 2018 at 02:44:33PM -0300, Marco Benatto wrote: > Recently we found out xfs_repair were not repairing > root inode parent pointer when root inode is on short-form > and parent points to an invalid inode number (refer to: > "xfs_repair: Fix root inode's parent when it's bogus for sf > directory" on xfs-devel list). > > This test checks if xfs_repair successfully repair the > filesystem in the scenario mentioned before. > > Signed-off-by: Marco Benatto <mbenatto@redhat.com> > --- > tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/xfs/450.out | 1 + > tests/xfs/group | 1 + > 3 files changed, 55 insertions(+) > create mode 100755 tests/xfs/450 > create mode 100644 tests/xfs/450.out > > diff --git a/tests/xfs/450 b/tests/xfs/450 > new file mode 100755 > index 0000000..dc7f244 > --- /dev/null > +++ b/tests/xfs/450 > @@ -0,0 +1,53 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. > +# > +# FS QA Test 450 > +# > +# Make sure xfs_repair can repair root inode parent's pointer > +# when it contains a bogus ino when it's using shot form directory > +# > +seq=`basename $0` > +seqres=$RESULT_DIR/$seq > + > +status=1 # failure is the default! Apart from Dave's comments, there're some common definitions missing too, e.g. 'tmp' and 'here', please use './new xfs' to generate new test template. BTW, I've applied the first patch, no need to resend it in v2. Thanks, Eryu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-22 2:37 ` Eryu Guan @ 2018-06-22 2:58 ` Eric Sandeen 2018-06-22 3:54 ` Dave Chinner 0 siblings, 1 reply; 10+ messages in thread From: Eric Sandeen @ 2018-06-22 2:58 UTC (permalink / raw) To: Eryu Guan, Marco Benatto; +Cc: fstests, linux-xfs, darrick.wong On 6/21/18 9:37 PM, Eryu Guan wrote: > On Mon, Jun 18, 2018 at 02:44:33PM -0300, Marco Benatto wrote: >> Recently we found out xfs_repair were not repairing >> root inode parent pointer when root inode is on short-form >> and parent points to an invalid inode number (refer to: >> "xfs_repair: Fix root inode's parent when it's bogus for sf >> directory" on xfs-devel list). >> >> This test checks if xfs_repair successfully repair the >> filesystem in the scenario mentioned before. >> >> Signed-off-by: Marco Benatto <mbenatto@redhat.com> >> --- >> tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> tests/xfs/450.out | 1 + >> tests/xfs/group | 1 + >> 3 files changed, 55 insertions(+) >> create mode 100755 tests/xfs/450 >> create mode 100644 tests/xfs/450.out >> >> diff --git a/tests/xfs/450 b/tests/xfs/450 >> new file mode 100755 >> index 0000000..dc7f244 >> --- /dev/null >> +++ b/tests/xfs/450 >> @@ -0,0 +1,53 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. >> +# >> +# FS QA Test 450 >> +# >> +# Make sure xfs_repair can repair root inode parent's pointer >> +# when it contains a bogus ino when it's using shot form directory >> +# >> +seq=`basename $0` >> +seqres=$RESULT_DIR/$seq >> + >> +status=1 # failure is the default! > > Apart from Dave's comments, there're some common definitions missing > too, e.g. 'tmp' and 'here', please use './new xfs' to generate new test > template. Eryu, I think I suggested that Marco could remove the "tmp" definition because it's not used in the script. Is there some reason to keep it? The script did start out with a "./new xfs" generation template. Oh, I bet some helpers depend on it... sorry, my mistake. Oops. (though maybe tmp should just get hoisted to the harness and not be required in every script, but I digress ...) Thanks, -Eric > BTW, I've applied the first patch, no need to resend it in v2. > > Thanks, > Eryu > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-22 2:58 ` Eric Sandeen @ 2018-06-22 3:54 ` Dave Chinner 2018-06-22 15:17 ` Marco Benatto 0 siblings, 1 reply; 10+ messages in thread From: Dave Chinner @ 2018-06-22 3:54 UTC (permalink / raw) To: Eric Sandeen; +Cc: Eryu Guan, Marco Benatto, fstests, linux-xfs, darrick.wong On Thu, Jun 21, 2018 at 09:58:25PM -0500, Eric Sandeen wrote: > On 6/21/18 9:37 PM, Eryu Guan wrote: > > On Mon, Jun 18, 2018 at 02:44:33PM -0300, Marco Benatto wrote: > >> Recently we found out xfs_repair were not repairing > >> root inode parent pointer when root inode is on short-form > >> and parent points to an invalid inode number (refer to: > >> "xfs_repair: Fix root inode's parent when it's bogus for sf > >> directory" on xfs-devel list). > >> > >> This test checks if xfs_repair successfully repair the > >> filesystem in the scenario mentioned before. > >> > >> Signed-off-by: Marco Benatto <mbenatto@redhat.com> > >> --- > >> tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> tests/xfs/450.out | 1 + > >> tests/xfs/group | 1 + > >> 3 files changed, 55 insertions(+) > >> create mode 100755 tests/xfs/450 > >> create mode 100644 tests/xfs/450.out > >> > >> diff --git a/tests/xfs/450 b/tests/xfs/450 > >> new file mode 100755 > >> index 0000000..dc7f244 > >> --- /dev/null > >> +++ b/tests/xfs/450 > >> @@ -0,0 +1,53 @@ > >> +#! /bin/bash > >> +# SPDX-License-Identifier: GPL-2.0 > >> +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. > >> +# > >> +# FS QA Test 450 > >> +# > >> +# Make sure xfs_repair can repair root inode parent's pointer > >> +# when it contains a bogus ino when it's using shot form directory > >> +# > >> +seq=`basename $0` > >> +seqres=$RESULT_DIR/$seq > >> + > >> +status=1 # failure is the default! > > > > Apart from Dave's comments, there're some common definitions missing > > too, e.g. 'tmp' and 'here', please use './new xfs' to generate new test > > template. > > Eryu, I think I suggested that Marco could remove the "tmp" definition > because it's not used in the script. Is there some reason to keep it? > The script did start out with a "./new xfs" generation template. > > Oh, I bet some helpers depend on it... sorry, my mistake. Oops. > > (though maybe tmp should just get hoisted to the harness and > not be required in every script, but I digress ...) You mean like I proposed a couple of weeks ago along with the spdx license tag updates? https://www.spinics.net/lists/fstests/msg09849.html that's next on my list of "Big cleanups for fstests To Do" list. Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-22 3:54 ` Dave Chinner @ 2018-06-22 15:17 ` Marco Benatto 2018-06-22 18:04 ` Marco Benatto 0 siblings, 1 reply; 10+ messages in thread From: Marco Benatto @ 2018-06-22 15:17 UTC (permalink / raw) To: Dave Chinner; +Cc: Eric Sandeen, Eryu Guan, fstests, linux-xfs, Darrick J. Wong Hi Eryuy et al, firstly thanks for the reviews. OK, I'll resend just the test itself (without the one which hoist the helper function for sfdir format). Given Eric's and Dave's updates, should I re-include the common definitions even those being unused within this test scope? If those are used by helpers or any other flow on xfstest I do agree with Eric and Dave about hoist it. Thanks, On Fri, Jun 22, 2018 at 12:54 AM, Dave Chinner <david@fromorbit.com> wrote: > On Thu, Jun 21, 2018 at 09:58:25PM -0500, Eric Sandeen wrote: >> On 6/21/18 9:37 PM, Eryu Guan wrote: >> > On Mon, Jun 18, 2018 at 02:44:33PM -0300, Marco Benatto wrote: >> >> Recently we found out xfs_repair were not repairing >> >> root inode parent pointer when root inode is on short-form >> >> and parent points to an invalid inode number (refer to: >> >> "xfs_repair: Fix root inode's parent when it's bogus for sf >> >> directory" on xfs-devel list). >> >> >> >> This test checks if xfs_repair successfully repair the >> >> filesystem in the scenario mentioned before. >> >> >> >> Signed-off-by: Marco Benatto <mbenatto@redhat.com> >> >> --- >> >> tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> >> tests/xfs/450.out | 1 + >> >> tests/xfs/group | 1 + >> >> 3 files changed, 55 insertions(+) >> >> create mode 100755 tests/xfs/450 >> >> create mode 100644 tests/xfs/450.out >> >> >> >> diff --git a/tests/xfs/450 b/tests/xfs/450 >> >> new file mode 100755 >> >> index 0000000..dc7f244 >> >> --- /dev/null >> >> +++ b/tests/xfs/450 >> >> @@ -0,0 +1,53 @@ >> >> +#! /bin/bash >> >> +# SPDX-License-Identifier: GPL-2.0 >> >> +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. >> >> +# >> >> +# FS QA Test 450 >> >> +# >> >> +# Make sure xfs_repair can repair root inode parent's pointer >> >> +# when it contains a bogus ino when it's using shot form directory >> >> +# >> >> +seq=`basename $0` >> >> +seqres=$RESULT_DIR/$seq >> >> + >> >> +status=1 # failure is the default! >> > >> > Apart from Dave's comments, there're some common definitions missing >> > too, e.g. 'tmp' and 'here', please use './new xfs' to generate new test >> > template. >> >> Eryu, I think I suggested that Marco could remove the "tmp" definition >> because it's not used in the script. Is there some reason to keep it? >> The script did start out with a "./new xfs" generation template. >> >> Oh, I bet some helpers depend on it... sorry, my mistake. Oops. >> >> (though maybe tmp should just get hoisted to the harness and >> not be required in every script, but I digress ...) > > You mean like I proposed a couple of weeks ago along with the > spdx license tag updates? > > https://www.spinics.net/lists/fstests/msg09849.html > > that's next on my list of "Big cleanups for fstests To Do" list. > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com -- Marco Benatto Senior Software Maintenance Engineer | Red Hat Brasil T: +55 11 35246161 | M: +55 41 9 88504051 Av. Brigadeiro Faria Lima 3900, 8° Andar. São Paulo, Brasil. RED HAT | TRIED. TESTED. TRUSTED. Saiba porque em redhat.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing 2018-06-22 15:17 ` Marco Benatto @ 2018-06-22 18:04 ` Marco Benatto 0 siblings, 0 replies; 10+ messages in thread From: Marco Benatto @ 2018-06-22 18:04 UTC (permalink / raw) To: Dave Chinner; +Cc: Eric Sandeen, Eryu Guan, fstests, linux-xfs, Darrick J. Wong Hello all, just FYI just sent out v2 covering Dave's and Eryu's suggestions. Thanks, On Fri, Jun 22, 2018 at 12:17 PM, Marco Benatto <mbenatto@redhat.com> wrote: > Hi Eryuy et al, > > firstly thanks for the reviews. > > OK, I'll resend just the test itself (without the one which hoist the > helper function for sfdir format). > > > Given Eric's and Dave's updates, should I re-include the common > definitions even those being unused within > this test scope? > > If those are used by helpers or any other flow on xfstest I do agree > with Eric and Dave about hoist it. > > Thanks, > > > On Fri, Jun 22, 2018 at 12:54 AM, Dave Chinner <david@fromorbit.com> wrote: >> On Thu, Jun 21, 2018 at 09:58:25PM -0500, Eric Sandeen wrote: >>> On 6/21/18 9:37 PM, Eryu Guan wrote: >>> > On Mon, Jun 18, 2018 at 02:44:33PM -0300, Marco Benatto wrote: >>> >> Recently we found out xfs_repair were not repairing >>> >> root inode parent pointer when root inode is on short-form >>> >> and parent points to an invalid inode number (refer to: >>> >> "xfs_repair: Fix root inode's parent when it's bogus for sf >>> >> directory" on xfs-devel list). >>> >> >>> >> This test checks if xfs_repair successfully repair the >>> >> filesystem in the scenario mentioned before. >>> >> >>> >> Signed-off-by: Marco Benatto <mbenatto@redhat.com> >>> >> --- >>> >> tests/xfs/450 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> >> tests/xfs/450.out | 1 + >>> >> tests/xfs/group | 1 + >>> >> 3 files changed, 55 insertions(+) >>> >> create mode 100755 tests/xfs/450 >>> >> create mode 100644 tests/xfs/450.out >>> >> >>> >> diff --git a/tests/xfs/450 b/tests/xfs/450 >>> >> new file mode 100755 >>> >> index 0000000..dc7f244 >>> >> --- /dev/null >>> >> +++ b/tests/xfs/450 >>> >> @@ -0,0 +1,53 @@ >>> >> +#! /bin/bash >>> >> +# SPDX-License-Identifier: GPL-2.0 >>> >> +# Copyright (c) 2018 Red Hat Inc. All Rights Reserved. >>> >> +# >>> >> +# FS QA Test 450 >>> >> +# >>> >> +# Make sure xfs_repair can repair root inode parent's pointer >>> >> +# when it contains a bogus ino when it's using shot form directory >>> >> +# >>> >> +seq=`basename $0` >>> >> +seqres=$RESULT_DIR/$seq >>> >> + >>> >> +status=1 # failure is the default! >>> > >>> > Apart from Dave's comments, there're some common definitions missing >>> > too, e.g. 'tmp' and 'here', please use './new xfs' to generate new test >>> > template. >>> >>> Eryu, I think I suggested that Marco could remove the "tmp" definition >>> because it's not used in the script. Is there some reason to keep it? >>> The script did start out with a "./new xfs" generation template. >>> >>> Oh, I bet some helpers depend on it... sorry, my mistake. Oops. >>> >>> (though maybe tmp should just get hoisted to the harness and >>> not be required in every script, but I digress ...) >> >> You mean like I proposed a couple of weeks ago along with the >> spdx license tag updates? >> >> https://www.spinics.net/lists/fstests/msg09849.html >> >> that's next on my list of "Big cleanups for fstests To Do" list. >> >> Cheers, >> >> Dave. >> -- >> Dave Chinner >> david@fromorbit.com > > > > -- > Marco Benatto > Senior Software Maintenance Engineer | Red Hat Brasil > T: +55 11 35246161 | M: +55 41 9 88504051 > Av. Brigadeiro Faria Lima 3900, 8° Andar. São Paulo, Brasil. RED HAT | > TRIED. TESTED. TRUSTED. Saiba porque em redhat.com -- Marco Benatto Senior Software Maintenance Engineer | Red Hat Brasil T: +55 11 35246161 | M: +55 41 9 88504051 Av. Brigadeiro Faria Lima 3900, 8° Andar. São Paulo, Brasil. RED HAT | TRIED. TESTED. TRUSTED. Saiba porque em redhat.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function 2018-06-18 17:44 [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function Marco Benatto 2018-06-18 17:44 ` [PATCH 2/2] xfstests: Test root inode parent pointer repairing Marco Benatto @ 2018-06-18 19:11 ` Darrick J. Wong 1 sibling, 0 replies; 10+ messages in thread From: Darrick J. Wong @ 2018-06-18 19:11 UTC (permalink / raw) To: Marco Benatto; +Cc: fstests, linux-xfs, sandeen On Mon, Jun 18, 2018 at 02:44:32PM -0300, Marco Benatto wrote: > Move get_sfdir_prefix function from xfs/278 to commom/xfs > and rename it to _scratch_get_sfdir_prefix so it can be > used in other xfs tests. > > This commit also changes xfs/278 to make use of > _scratch_get_sfdir_prefix instead previous one. > > Signed-off-by: Marco Benatto <mbenatto@redhat.com> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > common/xfs | 15 +++++++++++++++ > tests/xfs/278 | 16 +--------------- > 2 files changed, 16 insertions(+), 15 deletions(-) > > diff --git a/common/xfs b/common/xfs > index 55cfa94..ecf54bb 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -760,3 +760,18 @@ _require_xfs_spaceman_command() > _notrun "xfs_spaceman $command doesn't support $param" > fi > } > + > +_scratch_get_sfdir_prefix() { > + local dir_ino="$1" > + > + for prefix in "u.sfdir3" "u.sfdir2" "u3.sfdir3"; do > + if [ -n "$(_scratch_xfs_get_metadata_field \ > + "${prefix}.hdr.parent.i4" \ > + "inode ${dir_ino}")" ]; then > + echo "${prefix}" > + return 0 > + fi > + done > + _scratch_xfs_db -c "inode ${dir_ino}" -c 'p' >> $seqres.full > + return 1 > +} > diff --git a/tests/xfs/278 b/tests/xfs/278 > index 3d2a846..c208e61 100755 > --- a/tests/xfs/278 > +++ b/tests/xfs/278 > @@ -46,25 +46,11 @@ _scratch_unmount > > echo "Silence is goodness..." > > -get_sfdir_prefix() { > - local dir_ino="$1" > - > - for prefix in "u.sfdir3" "u.sfdir2" "u3.sfdir3"; do > - if [ -n "$(_scratch_xfs_get_metadata_field \ > - "${prefix}.hdr.parent.i4" \ > - "inode ${dir_ino}")" ]; then > - echo "${prefix}" > - return 0 > - fi > - done > - _scratch_xfs_db -c "inode ${dir_ino}" -c 'p' >> $seqres.full > - return 1 > -} > set_ifield() { > _scratch_xfs_set_metadata_field "$1" 0 "inode $2" >> $seqres.full > } > > -sfdir_prefix="$(get_sfdir_prefix "$DIR_INO" || \ > +sfdir_prefix="$(_scratch_get_sfdir_prefix "$DIR_INO" || \ > _fail "Cannot determine sfdir prefix")" > > # Corrupt DIR > -- > 2.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-06-22 18:04 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-18 17:44 [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function Marco Benatto 2018-06-18 17:44 ` [PATCH 2/2] xfstests: Test root inode parent pointer repairing Marco Benatto 2018-06-18 19:26 ` Darrick J. Wong 2018-06-20 1:48 ` Dave Chinner 2018-06-22 2:37 ` Eryu Guan 2018-06-22 2:58 ` Eric Sandeen 2018-06-22 3:54 ` Dave Chinner 2018-06-22 15:17 ` Marco Benatto 2018-06-22 18:04 ` Marco Benatto 2018-06-18 19:11 ` [PATCH 1/2] common/xfs: Add _scratch_get_sfdir_prefix function Darrick J. Wong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).