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@xxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxx> >>> >> --- >>> >> 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@xxxxxxxxxxxxx > > > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html