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