On Tue, Oct 03, 2017 at 07:06:07PM +1100, Dave Chinner wrote: > Hi Eric, > > I've put the latest mkfs refactor code that I have up in place > you can pull it from. I've rebased it against the current for-next > tree (4.13.1 release) and fixed all the problems that xfstests > exposes. The only thing I haven't fixed is xfs/191 that does mkfs > command line behaviour verification because the refactored version > fixes several problems that the old mkfs didn't handle correctly > (e.g. being able to specify certain things like agsize in blocks or > sectors). > > There's a small filter patch needed for xfstests that I'll post in > a reply to this pull request that will filter out the new "defaults > sourced from ..." output and so prevent spurious xfstests failures. > > If you want I can tag the branch with a signed tag for you to pull > from (same process as Linus prefers) rather than just a branch in a > tree. If you'd prefer that I post this as patches instead, then let > me know and I'll bomb the list instead. I had a look at mkfs-refactor. It looks ok to me (I defer to Eric on the question of pull req. vs. patchbomb) though I have one question: calculate_log_size calls max_trans_res, and max_trans_res assembles a fake struct xfs_mount in order to call libxfs_log_calc_minimum_size. I've fixed a few mkfs bugs over the past couple of years that all stem from us forgetting to propagate superblock settings from the configuration we're building in main() into the fake xfs_mount->m_sb that we use to calculate the minimum log size, which results in a disagreement between the kernel and mkfs as to what is the minimum log size for a given fs configuration. This disagreement pops up in the form of a freshly mkfs'd 500MB filesystem immediately failing to mount. With this branch applied it looks like we've nearly finished filling out the real xfs_mount->m_sb when we call calculate_log_size, so could we refactor setup_superblock to set all the non-log superblock fields in the real m_sb and then pass that directly into max_trans_res so that we can memcpy the real superblock settings into the fake struct xfs_mount? Doing that will eliminate a whole class of "we forgot that we have to set sb_newfield in setup_superblock /and/ in max_trans_res and now mkfs creates broken filesystems" bugs. Even now there are small discrepancies between (for example) tr_itruncate.tr_logres in the kernel and in mkfs, which make me nervous. AFAICT the discrepancies result in mkfs using a minimum log size that is larger than what the kernel calculates, so there's no user-visible badness. --D > > Cheers, > > Dave. > > The following changes since commit d4a36331dc383c7c7747e244b3ae20155ae92c98: > > xfsprogs: Release v4.13.1 (2017-09-26 20:45:05 -0500) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/dgc/xfsprogs-dev mkfs-refactor > > for you to fetch changes up to a4bc6d3c7bb5babc51f7341039dafcff5fcc6c7e: > > mkfs: tidy up definitions (2017-09-29 08:44:30 +1000) > > ---------------------------------------------------------------- > Dave Chinner (42): > mkfs: can't specify sector size of internal log > mkfs: make subopt table const > mkfs: introduce a structure to hold CLI options > mkfs: add generic subopt parsing table > mkfs: factor block subopts parser > mkfs: factor data subopts parser > mkfs: factor inode subopts parser > mkfs: factor log subopts parser > mkfs: factor meta subopts parser > mkfs: factor naming subopts parser > mkfs: factor rt subopts parser > mkfs: factor sector subopts parser > mkfs: Introduce mkfs configuration structure > mkfs: factor printing of mkfs config > mkfs: factor in memory superblock setup > mkfs: factor out device preparation > mkfs: factor writing AG headers > mkfs: factor secondary superblock updates > mkfs: introduce default configuration structure > mkfs: rename top level CLI parameters > mkfs: factor sectorsize validation > mkfs: factor blocksize validation > mkfs: factor log sector size validation > mkfs: factor superblock feature validation > mkfs: factor directory blocksize validation > mkfs: factor inode size validation > mkfs: factor out device size calculations > mkfs: fix hidden parameter in DTOBT() > mkfs: factor rtdev extent size validation > mkfs: rework stripe calculations > mkfs: factor device opening > mkfs: factor data device validation > mkfs: factor log device validation > mkfs: factor rt device validation > mkfs: factor AG geometry calculations > mkfs: factor AG alignment > mkfs: rework imaxpct calculation > mkfs: factor initial mount setup > mkfs: factor log size calculations > mkfs: cleanup redundant temporary code > mkfs: move error functions > mkfs: tidy up definitions > > include/libxfs.h | 2 +- > mkfs/xfs_mkfs.c | 4645 ++++++++++++++++++++++++++++++------------------------ > 2 files changed, 2602 insertions(+), 2045 deletions(-) > -- > Dave Chinner > david@xxxxxxxxxxxxx > -- > 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 -- 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
