Re: [PATCH 2/2] xfstests: Test root inode parent pointer repairing

[Dave Chinner <david@fromorbit.com>]

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