On Thu, Sep 07, 2017 at 04:01:57PM +0530, Chandan Rajendra wrote:
> On Wednesday, August 30, 2017 5:20:10 AM IST Dave Chinner wrote:
> > Everyone who tries to modify mkfs quickly learns that it is a pile
> > of spaghetti, the only difference in opinion is whether it is a
> > steaming, cold or rotten pile. This patchset attempts to untangle
> > the ball of pasta and turn it into a set of clear, obvious
> > operations that lead to a filesystem being formatted correctly.
> >
> > The patch series is really in three parts, splitting the code up
> > into roughly three modules. The first part introduces a mkfs
> > parameters structure and factors the on-disk formatting code to use
> > only information in that structure. The second part introduces a
> > command line input structure and factors the input parsing to use
> > it. This requires a bunch of temporary code to keep the rest of
> > the code working. The third part is factoring the input validation
> > and geometry calculation code to use the input structure and put
> > the output into the mkfs parameter structure and to remove all the
> > temporary support code.
> >
> > The result is three modules - input parsing, validation+calculations
> > and formatting - with well defined data flow between them. This also
> > paves the way to supporting config files to set defaults via a
> > separate (new) module. The overall data flow now looks like this:
> >
> > Build defaults --\
> > ---> mkfs_default_params -> CLI -> mkfs_params
> > config file -----/
> >
> > It is not worth spending a lot of time reviewing the temporary code
> > that is added - it gets removed before the end of the series. No
> > attempt has been made to ensure that mkfs works 100% correctly after
> > each patch is applied - the only guarantee is that it will build
> > cleanly. It /should/ work if a bisect lands in the middle of the
> > series, but trying to exhaustively test each patch is OK would take
> > more effort than it is worth. As such, testing has only been
> > performed on the whole series.
> >
> > The new output from mkfs to indicate where it has sourced the
> > defaults from causes xfstests to have conniptions. This requires
> > some updates to the mkfs output filters that are already in place
> > but it is a fairly trivial update. Test xfs/191 has a couple of new
> > failures, but that is because the new code now correctly parses
> > things like agsize so that block and sector size based
> > specifications work with default mkfs values. This will require test
> > updates.
> >
> > Future work will be to split the xfs_mkfs.c file into a file per
> > module (i.e. seperate files for CLI parsing, mkfs formating,
> > validation+calculation and, finally, one for config file support),
> > but otherwise the majority of the factoring work is now complete.
> >
> > Comments, flames, etc all welcome.
> >
>
> Hi Dave,
>
> For 4k blocksized xfs filesystem on ppc64, xfs/058 fails with mkfs refactor
> patchset applied.
>
> [root@localhost xfstests-dev]# ./check xfs/058
> FSTYP -- xfs (debug)
> PLATFORM -- Linux/ppc64 localhost 4.13.0-next-20170905-00001-g9730219
> MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1
> MOUNT_OPTIONS -- /dev/loop1 /mnt/btrfs-xfstest-scratch
>
> xfs/058 2s ... - output mismatch (see /root/repos/xfstests-dev/results//xfs/058.out.bad)
> --- tests/xfs/058.out 2017-09-03 02:23:13.432063287 -0500
> +++ /root/repos/xfstests-dev/results//xfs/058.out.bad 2017-09-07 05:07:49.850183977 -0500
> @@ -13,16 +13,16 @@
> fdblocks = 9223372036854775807
> Test verb middlebit
> Allowing fuzz of corrupted data with good CRC
> -fdblocks = 9223372034707292159
> +fdblocks = 9223372036854775807
> Test verb lastbit
> Allowing fuzz of corrupted data with good CRC
> ...
> (Run 'diff -u tests/xfs/058.out /root/repos/xfstests-dev/results//xfs/058.out.bad' to see the entire diff)
> Ran: xfs/058
> Failures: xfs/058
> Failed 1 of 1 tests
That's a bug in xfs_db, not my patchset. Fixed by commit
592c10154f99 ("xfs_db: bit fuzzing should read the right bit when
flipping").
> Also, xfs/206 fails because the line
> "Default configuration sourced from package build definitions" isn't getting
> filtered out.
Lots of tests fail that way. I have a local xfstests patch that
filters these out that I haven't sent out yet. Attached below.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
---
common/xfs | 3 ++-
tests/xfs/206 | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/common/xfs b/common/xfs
index 2c34e0887747..3969582866cf 100644
--- a/common/xfs
+++ b/common/xfs
@@ -81,7 +81,8 @@ _scratch_mkfs_xfs()
{
local mkfs_cmd="`_scratch_mkfs_xfs_opts`"
local mkfs_filter="sed -e '/less than device physical sector/d' \
- -e '/switching to logical sector/d'"
+ -e '/switching to logical sector/d' \
+ -e '/Default configuration/d'"
local tmp=`mktemp -u`
local mkfs_status
diff --git a/tests/xfs/206 b/tests/xfs/206
index 70997e3fe83e..01782b7b93a9 100755
--- a/tests/xfs/206
+++ b/tests/xfs/206
@@ -84,7 +84,8 @@ mkfs_filter()
-e "s/\(sectsz\)\(=[0-9]* *\)/\1=512 /" \
-e "s/\(sunit=\)\([0-9]* blks,\)/\10 blks,/" \
-e "s/, lazy-count=[0-9]//" \
- -e "/.*crc=/d"
+ -e "/.*crc=/d" \
+ -e "/^Default configuration/d"
}
# mkfs slightly smaller than that, small log for speed.
--
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
