linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chandan Rajendra <chandan@linux.vnet.ibm.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 00/42] mkfs: factor the crap out of the code
Date: Mon, 04 Sep 2017 18:01:02 +0530	[thread overview]
Message-ID: <3637079.ALR5oY5V5m@localhost.localdomain> (raw)
In-Reply-To: <20170829235052.21050-1-david@fromorbit.com>

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,

I am trying to test the "mkfs refactor" patchset on a ppc64 machine. But "git
am ~/mkfs-xfs-rewrite/\[PATCH\ 03_42\]\ mkfs_introduce\ a\ structure\ to\
hold\ CLI\ options.mbox" fails to apply cleanly on top of xfsprogs/master
(with patches 01 and 02 already applied) branch. Can you please let me know
the id of the topmost commit on which the "mkfs refactor" patches are applied? 

-- 
chandan


  parent reply	other threads:[~2017-09-04 12:30 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-29 23:50 [PATCH 00/42] mkfs: factor the crap out of the code Dave Chinner
2017-08-29 23:50 ` [PATCH 01/42] mkfs: can't specify sector size of internal log Dave Chinner
2017-08-29 23:50 ` [PATCH 02/42] mkfs: make subopt table const Dave Chinner
2017-08-29 23:50 ` [PATCH 03/42] mkfs: introduce a structure to hold CLI options Dave Chinner
2017-08-29 23:50 ` [PATCH 04/42] mkfs: add generic subopt parsing table Dave Chinner
2017-08-29 23:50 ` [PATCH 05/42] mkfs: factor block subopts parser Dave Chinner
2017-08-29 23:50 ` [PATCH 06/42] mkfs: factor data " Dave Chinner
2017-08-29 23:50 ` [PATCH 07/42] mkfs: factor inode " Dave Chinner
2017-08-29 23:50 ` [PATCH 08/42] mkfs: factor log " Dave Chinner
2017-08-29 23:50 ` [PATCH 09/42] mkfs: factor meta " Dave Chinner
2017-08-29 23:50 ` [PATCH 10/42] mkfs: factor naming " Dave Chinner
2017-08-29 23:50 ` [PATCH 11/42] mkfs: factor rt " Dave Chinner
2017-08-29 23:50 ` [PATCH 12/42] mkfs: factor sector " Dave Chinner
2017-08-29 23:50 ` [PATCH 13/42] mkfs: Introduce mkfs configuration structure Dave Chinner
2017-08-29 23:50 ` [PATCH 14/42] mkfs: factor printing of mkfs config Dave Chinner
2017-08-29 23:50 ` [PATCH 15/42] mkfs: factor in memory superblock setup Dave Chinner
2017-08-29 23:50 ` [PATCH 16/42] mkfs: factor out device preparation Dave Chinner
2017-08-29 23:50 ` [PATCH 17/42] mkfs: factor writing AG headers Dave Chinner
2017-08-29 23:50 ` [PATCH 18/42] mkfs: factor secondary superblock updates Dave Chinner
2017-08-29 23:50 ` [PATCH 19/42] mkfs: introduce default configuration structure Dave Chinner
2017-08-29 23:50 ` [PATCH 20/42] mkfs: rename top level CLI parameters Dave Chinner
2017-08-29 23:50 ` [PATCH 21/42] mkfs: factor sectorsize validation Dave Chinner
2017-08-29 23:50 ` [PATCH 22/42] mkfs: factor blocksize validation Dave Chinner
2017-08-29 23:50 ` [PATCH 23/42] mkfs: factor log sector size validation Dave Chinner
2017-08-29 23:50 ` [PATCH 24/42] mkfs: factor superblock feature validation Dave Chinner
2017-08-29 23:50 ` [PATCH 25/42] mkfs: factor directory blocksize validation Dave Chinner
2017-08-29 23:50 ` [PATCH 26/42] mkfs: factor inode size validation Dave Chinner
2017-08-29 23:50 ` [PATCH 27/42] mkfs: factor out device size calculations Dave Chinner
2017-08-29 23:50 ` [PATCH 28/42] mkfs: fix hidden parameter in DTOBT() Dave Chinner
2017-08-29 23:50 ` [PATCH 29/42] mkfs: factor rtdev extent size validation Dave Chinner
2017-08-29 23:50 ` [PATCH 30/42] mkfs: rework stripe calculations Dave Chinner
2017-08-29 23:50 ` [PATCH 31/42] mkfs: factor device opening Dave Chinner
2017-08-29 23:50 ` [PATCH 32/42] mkfs: factor data device validation Dave Chinner
2017-08-29 23:50 ` [PATCH 33/42] mkfs: factor log " Dave Chinner
2017-08-29 23:50 ` [PATCH 34/42] mkfs: factor rt " Dave Chinner
2017-08-29 23:50 ` [PATCH 35/42] mkfs: factor AG geometry calculations Dave Chinner
2017-08-29 23:50 ` [PATCH 36/42] mkfs: factor AG alignment Dave Chinner
2017-08-30 23:44   ` Dave Chinner
2017-08-29 23:50 ` [PATCH 37/42] mkfs: rework imaxpct calculation Dave Chinner
2017-08-29 23:50 ` [PATCH 38/42] mkfs: factor initial mount setup Dave Chinner
2017-08-29 23:50 ` [PATCH 39/42] mkfs: factor log size calculations Dave Chinner
2017-09-05  5:23   ` Dave Chinner
2017-08-29 23:50 ` [PATCH 40/42] mkfs: cleanup redundant temporary code Dave Chinner
2017-08-29 23:50 ` [PATCH 41/42] mkfs: move error functions Dave Chinner
2017-08-29 23:50 ` [PATCH 42/42] mkfs: tidy up definitions Dave Chinner
2017-08-30  1:23 ` [PATCH 00/42] mkfs: factor the crap out of the code Darrick J. Wong
2017-08-30  1:57   ` Dave Chinner
2017-08-30  4:16 ` Luis R. Rodriguez
2017-08-30  5:44   ` Dave Chinner
2017-08-30 22:10     ` Luis R. Rodriguez
2017-08-30 23:22       ` Dave Chinner
2017-08-31  0:05         ` Luis R. Rodriguez
2017-08-31 16:23     ` Jan Tulak
2017-08-30  7:44 ` Martin Steigerwald
2017-09-04 12:31 ` Chandan Rajendra [this message]
2017-09-04 15:34   ` Eric Sandeen
2017-09-04 22:40   ` Dave Chinner
2017-09-07 10:31 ` Chandan Rajendra
2017-09-07 23:38   ` Dave Chinner
2017-09-09 10:24 ` Chandan Rajendra
2017-09-15  9:42 ` Jan Tulak
2017-09-16 11:29   ` Dave Chinner
2017-10-24  3:00 ` Eric Sandeen
2017-10-25  0:59   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3637079.ALR5oY5V5m@localhost.localdomain \
    --to=chandan@linux.vnet.ibm.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).