[Cluster-devel] [PATCH 0/5] mkfs.gfs2 improvements (groundwork)

[Cluster-devel] [PATCH 0/5] mkfs.gfs2 improvements (groundwork)

[Andrew Price]

This patchset lays the groundwork for the RAID stripe alignment (etc.)
enhancements I'm working on. Notably it reduces the peak heap usage (from >3GB
to ~25MB for a 50TB fs with default options) and also improves performance
slightly. It also adds more tests to the test suite (run 'make check').

Andrew Price (5):
  libgfs2: Rework blk_alloc_i
  libgfs2: Make gfs2_rgrp_out accept char buffers
  mkfs.gfs2: Reduce memory usage
  gfs2-utils: Make the tool tests script more useful
  mkfs.gfs2: Separate user options from file system params

 .gitignore                  |   2 +
 gfs2/convert/gfs2_convert.c |  10 +-
 gfs2/edit/hexedit.c         |   4 +-
 gfs2/fsck/initialize.c      |   4 +-
 gfs2/fsck/metawalk.c        |   4 +-
 gfs2/fsck/pass5.c           |   2 +-
 gfs2/fsck/rgrepair.c        |   4 +-
 gfs2/libgfs2/fs_geometry.c  |   4 +-
 gfs2/libgfs2/fs_ops.c       |  99 +++++---
 gfs2/libgfs2/gfs1.c         |   6 +-
 gfs2/libgfs2/libgfs2.h      |  10 +-
 gfs2/libgfs2/ondisk.c       |  32 ++-
 gfs2/libgfs2/structures.c   |   2 +-
 gfs2/mkfs/main_jadd.c       |   2 +-
 gfs2/mkfs/main_mkfs.c       | 593 +++++++++++++++++++++++---------------------
 tests/tool_tests.sh         |  56 ++++-
 16 files changed, 476 insertions(+), 358 deletions(-)

-- 
1.8.1.2

[Cluster-devel] [PATCH 1/5] libgfs2: Rework blk_alloc_i

[Andrew Price]

blk_alloc_i previously chose any resource group with a free block and
assumed that the bitmaps were already available in the rgrp_tree. This
patch allows us to allocate blocks in a resource group with a given
number of free blocks in order to plan ahead and reads in the bitmap
data on demand. It also removes some exit()s from blk_alloc_i.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/fs_ops.c  | 79 ++++++++++++++++++++++++++++++++++++++------------
 gfs2/libgfs2/libgfs2.h |  1 +
 2 files changed, 61 insertions(+), 19 deletions(-)

diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index 3d027e8..b6c268d 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -116,32 +116,24 @@ void inode_put(struct gfs2_inode **ip_in)
 	*ip_in = NULL; /* make sure the memory isn't accessed again */
 }
 
-static uint64_t blk_alloc_i(struct gfs2_sbd *sdp, unsigned int type)
+static int blk_alloc_in_rg(struct gfs2_sbd *sdp, unsigned int type, struct rgrp_tree *rl, uint64_t *blkno)
 {
-	struct osi_node *n, *next = NULL;
-	struct rgrp_tree *rl = NULL;
 	struct gfs2_rindex *ri;
 	struct gfs2_rgrp *rg;
 	unsigned int block, bn = 0, x = 0, y = 0;
 	unsigned int state;
 	struct gfs2_buffer_head *bh;
 
-	memset(&rg, 0, sizeof(rg));
-	for (n = osi_first(&sdp->rgtree); n; n = next) {
-		next = osi_next(n);
-		rl = (struct rgrp_tree *)n;
-		if (rl->rg.rg_free)
-			break;
+	if (rl == NULL || rl->rg.rg_free == 0) {
+		errno = ENOSPC;
+		return -1;
 	}
 
-	if (n == NULL) {
-		fprintf(stderr, "Not enough space available on device\n");
-		exit(1);
-	}
+	if (rl->bh[0] == NULL && gfs2_rgrp_read(sdp, rl) != 0)
+		return -1;
 
 	ri = &rl->ri;
 	rg = &rl->rg;
-
 	for (block = 0; block < ri->ri_length; block++) {
 		bh = rl->bh[block];
 		x = (block) ? sizeof(struct gfs2_meta_header) : sizeof(struct gfs2_rgrp);
@@ -157,7 +149,7 @@ static uint64_t blk_alloc_i(struct gfs2_sbd *sdp, unsigned int type)
 
 	fprintf(stderr, "allocation is broken (1): %"PRIu64" %u\n",
 	    (uint64_t)rl->ri.ri_addr, rl->rg.rg_free);
-	exit(1);
+	return -1;
 
 found:
 	if (bn >= ri->ri_bitbytes * GFS2_NBBY) {
@@ -165,7 +157,7 @@ found:
 		    " (0x%" PRIx64 ") Free:%u\n",
 		    bn, ri->ri_bitbytes * GFS2_NBBY, (uint64_t)rl->ri.ri_addr,
 		    (uint64_t)rl->ri.ri_addr, rl->rg.rg_free);
-		exit(1);
+		return -1;
 	}
 
 	switch (type) {
@@ -179,7 +171,7 @@ found:
 		break;
 	default:
 		fprintf(stderr, "bad state\n");
-		exit(1);
+		return -1;
 	}
 
 	bh->b_data[x] &= ~(0x03 << (GFS2_BIT_SIZE * y));
@@ -193,7 +185,27 @@ found:
 		gfs2_rgrp_out(rg, rl->bh[0]);
 
 	sdp->blks_alloced++;
-	return ri->ri_data0 + bn;
+	*blkno = ri->ri_data0 + bn;
+	return 0;
+}
+
+/**
+ * Do not use this function, it's only here until we can kill it.
+ * Use blk_alloc_in_rg directly instead.
+ */
+static uint64_t blk_alloc_i(struct gfs2_sbd *sdp, unsigned int type)
+{
+	int ret;
+	uint64_t blkno = 0;
+	struct osi_node *n = NULL;
+	for (n = osi_first(&sdp->rgtree); n; n = osi_next(n)) {
+		if (((struct rgrp_tree *)n)->rg.rg_free)
+			break;
+	}
+	ret = blk_alloc_in_rg(sdp, type, (struct rgrp_tree *)n, &blkno);
+	if (ret != 0) /* Do what the old blk_alloc_i did */
+		exit(1);
+	return blkno;
 }
 
 uint64_t data_alloc(struct gfs2_inode *ip)
@@ -220,6 +232,33 @@ uint64_t dinode_alloc(struct gfs2_sbd *sdp)
 	return blk_alloc_i(sdp, DINODE);
 }
 
+/**
+ * Allocate a dinode block in a bitmap. In order to plan ahead we look for a
+ * resource group with blksreq free blocks but only allocate the one dinode block.
+ * Returns 0 on success with the allocated block number in *blkno or non-zero otherwise.
+ */
+int lgfs2_dinode_alloc(struct gfs2_sbd *sdp, const uint64_t blksreq, uint64_t *blkno)
+{
+	int ret;
+	struct rgrp_tree *rgt;
+	struct osi_node *n = NULL;
+	for (n = osi_first(&sdp->rgtree); n; n = osi_next(n)) {
+		rgt = (struct rgrp_tree *)n;
+		if (rgt->rg.rg_free >= blksreq)
+			break;
+	}
+	if (rgt == NULL)
+		return -1;
+
+	ret = blk_alloc_in_rg(sdp, DINODE, rgt, blkno);
+	gfs2_rgrp_relse(rgt);
+
+	if (ret == 0)
+		sdp->dinodes_alloced++;
+
+	return ret;
+}
+
 static __inline__ void buffer_clear_tail(struct gfs2_sbd *sdp,
 					 struct gfs2_buffer_head *bh, int head)
 {
@@ -1380,7 +1419,9 @@ static struct gfs2_inode *__createi(struct gfs2_inode *dip,
 
 	gfs2_lookupi(dip, filename, strlen(filename), &ip);
 	if (!ip) {
-		bn = dinode_alloc(sdp);
+		err = lgfs2_dinode_alloc(sdp, 1, &bn);
+		if (err != 0)
+			return NULL;
 
 		if (if_gfs1)
 			inum.no_formal_ino = bn;
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index db31a6c..c415745 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -440,6 +440,7 @@ extern void inode_put(struct gfs2_inode **ip);
 extern uint64_t data_alloc(struct gfs2_inode *ip);
 extern uint64_t meta_alloc(struct gfs2_inode *ip);
 extern uint64_t dinode_alloc(struct gfs2_sbd *sdp);
+extern int lgfs2_dinode_alloc(struct gfs2_sbd *sdp, const uint64_t blksreq, uint64_t *blkno);
 extern int gfs2_readi(struct gfs2_inode *ip, void *buf, uint64_t offset,
 		      unsigned int size);
 #define gfs2_writei(ip, buf, offset, size) \
-- 
1.8.1.2

[Cluster-devel] [PATCH 2/5] libgfs2: Make gfs2_rgrp_out accept char buffers

[Andrew Price]

In order to prepare for handling resource groups separately from buffer
heads in mkfs.gfs2, change gfs2_rgrp_out to copy rgrps to char buffers
instead of buffer heads. Copying to bh's is now done in
gfs2_rgrp_out_bh.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/convert/gfs2_convert.c | 10 +++++-----
 gfs2/edit/hexedit.c         |  4 ++--
 gfs2/fsck/initialize.c      |  4 ++--
 gfs2/fsck/metawalk.c        |  4 ++--
 gfs2/fsck/pass5.c           |  2 +-
 gfs2/fsck/rgrepair.c        |  4 ++--
 gfs2/libgfs2/fs_geometry.c  |  4 ++--
 gfs2/libgfs2/fs_ops.c       | 20 ++++++++++----------
 gfs2/libgfs2/gfs1.c         |  6 +++---
 gfs2/libgfs2/libgfs2.h      |  8 +++++---
 gfs2/libgfs2/ondisk.c       | 32 +++++++++++++++++++++-----------
 gfs2/libgfs2/structures.c   |  2 +-
 gfs2/mkfs/main_jadd.c       |  2 +-
 13 files changed, 57 insertions(+), 45 deletions(-)

diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index 72c974f..235c95c 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -237,7 +237,7 @@ static int convert_rgs(struct gfs2_sbd *sbp)
 		sbp->dinodes_alloced += rgd1->rg_useddi;
 		convert_bitmaps(sbp, rgd);
 		/* Write the updated rgrp to the gfs2 buffer */
-		gfs2_rgrp_out(&rgd->rg, rgd->bh[0]);
+		gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 		rgs++;
 		if (rgs % 100 == 0) {
 			printf(".");
@@ -359,7 +359,7 @@ static void fix_metatree(struct gfs2_sbd *sbp, struct gfs2_inode *ip,
 			bh = bread(sbp, block);
 			if (new)
 				memset(bh->b_data, 0, sbp->bsize);
-			gfs2_meta_header_out(&mh, bh);
+			gfs2_meta_header_out_bh(&mh, bh);
 		}
 
 		hdrsize = blk->height ? sizeof(struct gfs2_meta_header) :
@@ -517,7 +517,7 @@ static void fix_jdatatree(struct gfs2_sbd *sbp, struct gfs2_inode *ip,
 			if (new)
 				memset(bh->b_data, 0, sbp->bsize);
 			if (h < (blk->height - 1))
-				gfs2_meta_header_out(&mh, bh);
+				gfs2_meta_header_out_bh(&mh, bh);
 		}
 
 		if (amount > sbp->bsize - ptramt)
@@ -1855,9 +1855,9 @@ static int journ_space_to_rg(struct gfs2_sbd *sdp)
 		convert_bitmaps(sdp, rgd);
 		for (x = 0; x < rgd->ri.ri_length; x++) {
 			if (x)
-				gfs2_meta_header_out(&mh, rgd->bh[x]);
+				gfs2_meta_header_out_bh(&mh, rgd->bh[x]);
 			else
-				gfs2_rgrp_out(&rgd->rg, rgd->bh[x]);
+				gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[x]);
 		}
 	} /* for each journal */
 	return error;
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 931b3c3..4856181 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1106,7 +1106,7 @@ static void set_rgrp_flags(int rgnum, uint32_t new_flags, int modify, int full)
 		if (sbd.gfs1)
 			gfs_rgrp_out(&rg.rg1, rbh);
 		else
-			gfs2_rgrp_out(&rg.rg2, rbh);
+			gfs2_rgrp_out_bh(&rg.rg2, rbh);
 		brelse(rbh);
 	} else {
 		if (full) {
@@ -2125,7 +2125,7 @@ static void process_field(const char *field, const char *nstr)
 		gfs2_rgrp_in(&rg, rbh);
 		if (setval) {
 			gfs2_rgrp_assignval(&rg, field, newval);
-			gfs2_rgrp_out(&rg, rbh);
+			gfs2_rgrp_out_bh(&rg, rbh);
 			if (!termlines)
 				gfs2_rgrp_printval(&rg, field);
 		} else {
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index e64ab3a..7d64b0a 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -314,7 +314,7 @@ static void check_rgrp_integrity(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
 		if (sdp->gfs1)
 			gfs_rgrp_out((struct gfs_rgrp *)&rgd->rg, rgd->bh[0]);
 		else
-			gfs2_rgrp_out(&rgd->rg, rgd->bh[0]);
+			gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 		*this_rg_cleaned = 1;
 		log_info( _("The rgrp at %lld (0x%llx) was cleaned of %d "
 			    "free metadata blocks.\n"),
@@ -336,7 +336,7 @@ static void check_rgrp_integrity(struct gfs2_sbd *sdp, struct rgrp_tree *rgd,
 				gfs_rgrp_out((struct gfs_rgrp *)&rgd->rg,
 					     rgd->bh[0]);
 			else
-				gfs2_rgrp_out(&rgd->rg, rgd->bh[0]);
+				gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 			*this_rg_fixed = 1;
 			log_err( _("The rgrp was fixed.\n"));
 		} else
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index d8193c5..5838fba 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -83,14 +83,14 @@ int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk,
 					gfs_rgrp_out((struct gfs_rgrp *)
 						     &rgd->rg, rgd->bh[0]);
 				else
-					gfs2_rgrp_out(&rgd->rg, rgd->bh[0]);
+					gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 			} else if (old_bitmap_state == GFS2_BLKST_FREE) {
 				rgd->rg.rg_free--;
 				if (sdp->gfs1)
 					gfs_rgrp_out((struct gfs_rgrp *)
 						     &rgd->rg, rgd->bh[0]);
 				else
-					gfs2_rgrp_out(&rgd->rg, rgd->bh[0]);
+					gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 			}
 			log_err( _("The bitmap was fixed.\n"));
 		} else {
diff --git a/gfs2/fsck/pass5.c b/gfs2/fsck/pass5.c
index 6c08e13..92861a1 100644
--- a/gfs2/fsck/pass5.c
+++ b/gfs2/fsck/pass5.c
@@ -274,7 +274,7 @@ static void update_rgrp(struct gfs2_sbd *sdp, struct rgrp_tree *rgp,
 			if (sdp->gfs1)
 				gfs_rgrp_out(gfs1rg, rgp->bh[0]);
 			else
-				gfs2_rgrp_out(&rgp->rg, rgp->bh[0]);
+				gfs2_rgrp_out_bh(&rgp->rg, rgp->bh[0]);
 		} else
 			log_err( _("Resource group counts left inconsistent\n"));
 	}
diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index c3467ba..27368a2 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -684,7 +684,7 @@ static int rewrite_rg_block(struct gfs2_sbd *sdp, struct rgrp_tree *rg,
 			mh.mh_magic = GFS2_MAGIC;
 			mh.mh_type = GFS2_METATYPE_RB;
 			mh.mh_format = GFS2_FORMAT_RB;
-			gfs2_meta_header_out(&mh, rg->bh[x]);
+			gfs2_meta_header_out_bh(&mh, rg->bh[x]);
 		} else {
 			if (sdp->gfs1)
 				memset(&rg->rg, 0, sizeof(struct gfs_rgrp));
@@ -698,7 +698,7 @@ static int rewrite_rg_block(struct gfs2_sbd *sdp, struct rgrp_tree *rg,
 				gfs_rgrp_out((struct gfs_rgrp *)&rg->rg,
 					     rg->bh[x]);
 			else
-				gfs2_rgrp_out(&rg->rg, rg->bh[x]);
+				gfs2_rgrp_out_bh(&rg->rg, rg->bh[x]);
 		}
 		brelse(rg->bh[x]);
 		rg->bh[x] = NULL;
diff --git a/gfs2/libgfs2/fs_geometry.c b/gfs2/libgfs2/fs_geometry.c
index e248f7c..e716127 100644
--- a/gfs2/libgfs2/fs_geometry.c
+++ b/gfs2/libgfs2/fs_geometry.c
@@ -217,9 +217,9 @@ void build_rgrps(struct gfs2_sbd *sdp, int do_write)
 			for (x = 0; x < bitblocks; x++) {
 				rl->bh[x] = bget(sdp, rl->start + x);
 				if (x)
-					gfs2_meta_header_out(&mh, rl->bh[x]);
+					gfs2_meta_header_out_bh(&mh, rl->bh[x]);
 				else
-					gfs2_rgrp_out(&rl->rg, rl->bh[x]);
+					gfs2_rgrp_out_bh(&rl->rg, rl->bh[x]);
 			}
 		}
 
diff --git a/gfs2/libgfs2/fs_ops.c b/gfs2/libgfs2/fs_ops.c
index b6c268d..4ea6dd3 100644
--- a/gfs2/libgfs2/fs_ops.c
+++ b/gfs2/libgfs2/fs_ops.c
@@ -182,7 +182,7 @@ found:
 	if (sdp->gfs1)
 		gfs_rgrp_out((struct gfs_rgrp *)rg, rl->bh[0]);
 	else
-		gfs2_rgrp_out(rg, rl->bh[0]);
+		gfs2_rgrp_out_bh(rg, rl->bh[0]);
 
 	sdp->blks_alloced++;
 	*blkno = ri->ri_data0 + bn;
@@ -294,7 +294,7 @@ void unstuff_dinode(struct gfs2_inode *ip)
 			mh.mh_magic = GFS2_MAGIC;
 			mh.mh_type = GFS2_METATYPE_JD;
 			mh.mh_format = GFS2_FORMAT_JD;
-			gfs2_meta_header_out(&mh, bh);
+			gfs2_meta_header_out_bh(&mh, bh);
 
 			buffer_copy_tail(sdp, bh,
 					 sizeof(struct gfs2_meta_header),
@@ -371,7 +371,7 @@ void build_height(struct gfs2_inode *ip, int height)
 			mh.mh_magic = GFS2_MAGIC;
 			mh.mh_type = GFS2_METATYPE_IN;
 			mh.mh_format = GFS2_FORMAT_IN;
-			gfs2_meta_header_out(&mh, bh);
+			gfs2_meta_header_out_bh(&mh, bh);
 			buffer_copy_tail(sdp, bh,
 					 sizeof(struct gfs2_meta_header),
 					 ip->i_bh, sizeof(struct gfs2_dinode));
@@ -494,7 +494,7 @@ void block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 			mh.mh_magic = GFS2_MAGIC;
 			mh.mh_type = GFS2_METATYPE_IN;
 			mh.mh_format = GFS2_FORMAT_IN;
-			gfs2_meta_header_out(&mh, bh);
+			gfs2_meta_header_out_bh(&mh, bh);
 		} else {
 			if (*dblock == ip->i_di.di_num.no_addr)
 				bh = ip->i_bh;
@@ -688,7 +688,7 @@ int __gfs2_writei(struct gfs2_inode *ip, void *buf,
 				mh.mh_magic = GFS2_MAGIC;
 				mh.mh_type = GFS2_METATYPE_JD;
 				mh.mh_format = GFS2_FORMAT_JD;
-				gfs2_meta_header_out(&mh, bh);
+				gfs2_meta_header_out_bh(&mh, bh);
 			}
 		} else {
 			if (dblock == ip->i_di.di_num.no_addr)
@@ -935,7 +935,7 @@ static void dir_split_leaf(struct gfs2_inode *dip, uint32_t lindex,
 		mh.mh_magic = GFS2_MAGIC;
 		mh.mh_type = GFS2_METATYPE_LF;
 		mh.mh_format = GFS2_FORMAT_LF;
-		gfs2_meta_header_out(&mh, nbh);
+		gfs2_meta_header_out_bh(&mh, nbh);
 		buffer_clear_tail(dip->i_sbd, nbh,
 				  sizeof(struct gfs2_meta_header));
 	}
@@ -1197,7 +1197,7 @@ restart:
 				mh.mh_magic = GFS2_MAGIC;
 				mh.mh_type = GFS2_METATYPE_LF;
 				mh.mh_format = GFS2_FORMAT_LF;
-				gfs2_meta_header_out(&mh, nbh);
+				gfs2_meta_header_out_bh(&mh, nbh);
 
 				leaf->lf_next = cpu_to_be64(bn);
 
@@ -1248,7 +1248,7 @@ static void dir_make_exhash(struct gfs2_inode *dip)
 		mh.mh_magic = GFS2_MAGIC;
 		mh.mh_type = GFS2_METATYPE_LF;
 		mh.mh_format = GFS2_FORMAT_LF;
-		gfs2_meta_header_out(&mh, bh);
+		gfs2_meta_header_out_bh(&mh, bh);
 	}
 
 	leaf = (struct gfs2_leaf *)bh->b_data;
@@ -1821,7 +1821,7 @@ void gfs2_free_block(struct gfs2_sbd *sdp, uint64_t block)
 		if (sdp->gfs1)
 			gfs_rgrp_out((struct gfs_rgrp *)&rgd->rg, rgd->bh[0]);
 		else
-			gfs2_rgrp_out(&rgd->rg, rgd->bh[0]);
+			gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 		sdp->blks_alloced--;
 	}
 }
@@ -1892,7 +1892,7 @@ int gfs2_freedi(struct gfs2_sbd *sdp, uint64_t diblock)
 	if (sdp->gfs1)
 		gfs_rgrp_out((struct gfs_rgrp *)&rgd->rg, rgd->bh[0]);
 	else
-		gfs2_rgrp_out(&rgd->rg, rgd->bh[0]);
+		gfs2_rgrp_out_bh(&rgd->rg, rgd->bh[0]);
 	sdp->dinodes_alloced--;
 	return 0;
 }
diff --git a/gfs2/libgfs2/gfs1.c b/gfs2/libgfs2/gfs1.c
index 06fa98f..89fb898 100644
--- a/gfs2/libgfs2/gfs1.c
+++ b/gfs2/libgfs2/gfs1.c
@@ -126,7 +126,7 @@ void gfs1_block_map(struct gfs2_inode *ip, uint64_t lblock, int *new,
 			mh.mh_magic = GFS2_MAGIC;
 			mh.mh_type = GFS2_METATYPE_IN;
 			mh.mh_format = GFS2_FORMAT_IN;
-			gfs2_meta_header_out(&mh, bh);
+			gfs2_meta_header_out_bh(&mh, bh);
 		} else {
 			if (*dblock == ip->i_di.di_num.no_addr)
 				bh = ip->i_bh;
@@ -225,7 +225,7 @@ int gfs1_writei(struct gfs2_inode *ip, char *buf, uint64_t offset,
 			mh.mh_magic = GFS2_MAGIC;
 			mh.mh_type = GFS2_METATYPE_JD;
 			mh.mh_format = GFS2_FORMAT_JD;
-			gfs2_meta_header_out(&mh, bh);
+			gfs2_meta_header_out_bh(&mh, bh);
 		}
 
 		memcpy(bh->b_data + offset, buf + copied, amount);
@@ -381,7 +381,7 @@ void gfs_rgrp_out(struct gfs_rgrp *rgrp, struct gfs2_buffer_head *rbh)
 {
 	struct gfs_rgrp *str = (struct gfs_rgrp *)rbh->b_data;
 
-	gfs2_meta_header_out(&rgrp->rg_header, rbh);
+	gfs2_meta_header_out_bh(&rgrp->rg_header, rbh);
 	str->rg_flags = cpu_to_be32(rgrp->rg_flags);
 	str->rg_free = cpu_to_be32(rgrp->rg_free);
 	str->rg_useddi = cpu_to_be32(rgrp->rg_useddi);
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index c415745..1475227 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -785,14 +785,16 @@ extern void gfs2_inum_in(struct gfs2_inum *no, char *buf);
 extern void gfs2_inum_out(struct gfs2_inum *no, char *buf);
 extern void gfs2_meta_header_in(struct gfs2_meta_header *mh,
 				struct gfs2_buffer_head *bh);
-extern void gfs2_meta_header_out(struct gfs2_meta_header *mh,
-				 struct gfs2_buffer_head *bh);
+extern void gfs2_meta_header_out(const struct gfs2_meta_header *mh, char *buf);
+extern void gfs2_meta_header_out_bh(const struct gfs2_meta_header *mh,
+                                    struct gfs2_buffer_head *bh);
 extern void gfs2_sb_in(struct gfs2_sb *sb, struct gfs2_buffer_head *bh);
 extern void gfs2_sb_out(struct gfs2_sb *sb, struct gfs2_buffer_head *bh);
 extern void gfs2_rindex_in(struct gfs2_rindex *ri, char *buf);
 extern void gfs2_rindex_out(struct gfs2_rindex *ri, char *buf);
 extern void gfs2_rgrp_in(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh);
-extern void gfs2_rgrp_out(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh);
+extern void gfs2_rgrp_out(const struct gfs2_rgrp *rg, char *buf);
+extern void gfs2_rgrp_out_bh(const struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh);
 extern void gfs2_quota_in(struct gfs2_quota *qu, char *buf);
 extern void gfs2_quota_out(struct gfs2_quota *qu, char *buf);
 extern void gfs2_dinode_in(struct gfs2_dinode *di,
diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
index 301d92d..4baacc7 100644
--- a/gfs2/libgfs2/ondisk.c
+++ b/gfs2/libgfs2/ondisk.c
@@ -72,16 +72,21 @@ void gfs2_meta_header_in(struct gfs2_meta_header *mh,
 	CPIN_32(mh, str, mh_format);
 }
 
-void gfs2_meta_header_out(struct gfs2_meta_header *mh,
-			  struct gfs2_buffer_head *bh)
+void gfs2_meta_header_out(const struct gfs2_meta_header *mh, char *buf)
 {
-	struct gfs2_meta_header *str = (struct gfs2_meta_header *)bh->b_data;
+	struct gfs2_meta_header *str = (struct gfs2_meta_header *)buf;
 
 	CPOUT_32(mh, str, mh_magic);
 	CPOUT_32(mh, str, mh_type);
 	CPOUT_32(mh, str, mh_format);
 	str->__pad0 = 0;
 	str->__pad1 = 0;
+}
+
+void gfs2_meta_header_out_bh(const struct gfs2_meta_header *mh,
+                             struct gfs2_buffer_head *bh)
+{
+	gfs2_meta_header_out(mh, bh->iov.iov_base);
 	bmodified(bh);
 }
 
@@ -123,7 +128,7 @@ void gfs2_sb_out(struct gfs2_sb *sb, struct gfs2_buffer_head *bh)
 {
 	struct gfs2_sb *str = (struct gfs2_sb *)bh->b_data;
 
-	gfs2_meta_header_out(&sb->sb_header, bh);
+	gfs2_meta_header_out_bh(&sb->sb_header, bh);
 
 	CPOUT_32(sb, str, sb_fs_format);
 	CPOUT_32(sb, str, sb_multihost_format);
@@ -243,16 +248,21 @@ void gfs2_rgrp_in(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
 	CPIN_08(rg, str, rg_reserved, 80);
 }
 
-void gfs2_rgrp_out(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
+void gfs2_rgrp_out(const struct gfs2_rgrp *rg, char *buf)
 {
-	struct gfs2_rgrp *str = (struct gfs2_rgrp *)bh->b_data;
+	struct gfs2_rgrp *str = (struct gfs2_rgrp *)buf;
 
-	gfs2_meta_header_out(&rg->rg_header, bh);
+	gfs2_meta_header_out(&rg->rg_header, buf);
 	CPOUT_32(rg, str, rg_flags);
 	CPOUT_32(rg, str, rg_free);
 	CPOUT_32(rg, str, rg_dinodes);
 
 	CPOUT_08(rg, str, rg_reserved, 80);
+}
+
+void gfs2_rgrp_out_bh(const struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
+{
+	gfs2_rgrp_out(rg, bh->iov.iov_base);
 	bmodified(bh);
 }
 
@@ -330,7 +340,7 @@ void gfs2_dinode_out(struct gfs2_dinode *di, struct gfs2_buffer_head *bh)
 {
 	struct gfs2_dinode *str = (struct gfs2_dinode *)bh->b_data;
 
-	gfs2_meta_header_out(&di->di_header, bh);
+	gfs2_meta_header_out_bh(&di->di_header, bh);
 	gfs2_inum_out(&di->di_num, (char *)&str->di_num);
 
 	CPOUT_32(di, str, di_mode);
@@ -432,7 +442,7 @@ void gfs2_leaf_out(struct gfs2_leaf *lf, struct gfs2_buffer_head *bh)
 {
 	struct gfs2_leaf *str = (struct gfs2_leaf *)bh->b_data;
 
-	gfs2_meta_header_out(&lf->lf_header, bh);
+	gfs2_meta_header_out_bh(&lf->lf_header, bh);
 	CPOUT_16(lf, str, lf_depth);
 	CPOUT_16(lf, str, lf_entries);
 	CPOUT_32(lf, str, lf_dirent_format);
@@ -497,7 +507,7 @@ void gfs2_log_header_out(struct gfs2_log_header *lh,
 {
 	struct gfs2_log_header *str = (struct gfs2_log_header *)bh->b_data;
 
-	gfs2_meta_header_out(&lh->lh_header, bh);
+	gfs2_meta_header_out_bh(&lh->lh_header, bh);
 	CPOUT_64(lh, str, lh_sequence);
 	CPOUT_32(lh, str, lh_flags);
 	CPOUT_32(lh, str, lh_tail);
@@ -535,7 +545,7 @@ void gfs2_log_descriptor_out(struct gfs2_log_descriptor *ld,
 {
 	struct gfs2_log_descriptor *str = (struct gfs2_log_descriptor *)bh->b_data;
 
-	gfs2_meta_header_out(&ld->ld_header, bh);
+	gfs2_meta_header_out_bh(&ld->ld_header, bh);
 	CPOUT_32(ld, str, ld_type);
 	CPOUT_32(ld, str, ld_length);
 	CPOUT_32(ld, str, ld_data1);
diff --git a/gfs2/libgfs2/structures.c b/gfs2/libgfs2/structures.c
index f9231ca..c60f3e4 100644
--- a/gfs2/libgfs2/structures.c
+++ b/gfs2/libgfs2/structures.c
@@ -256,7 +256,7 @@ static int build_quota_change(struct gfs2_inode *per_node, unsigned int j)
 			return -1;
 
 		memset(bh->b_data, 0, sdp->bsize);
-		gfs2_meta_header_out(&mh, bh);
+		gfs2_meta_header_out_bh(&mh, bh);
 		brelse(bh);
 	}
 
diff --git a/gfs2/mkfs/main_jadd.c b/gfs2/mkfs/main_jadd.c
index a4c6ca7..223dce7 100644
--- a/gfs2/mkfs/main_jadd.c
+++ b/gfs2/mkfs/main_jadd.c
@@ -342,7 +342,7 @@ add_qc(struct gfs2_sbd *sdp)
 		mh.mh_magic = GFS2_MAGIC;
 		mh.mh_type = GFS2_METATYPE_QC;
 		mh.mh_format = GFS2_FORMAT_QC;
-		gfs2_meta_header_out(&mh, &dummy_bh);
+		gfs2_meta_header_out_bh(&mh, &dummy_bh);
 
 		for (x=0; x<blocks; x++) {
 			if (write(fd, buf, sdp->bsize) != sdp->bsize) {
-- 
1.8.1.2

[Cluster-devel] [PATCH 3/5] mkfs.gfs2: Reduce memory usage

[Andrew Price]

This folds much of the resource group allocation and writing into two
new functions so that we allocate memory for each entire header at once
and free it once we're done with it.

Testing on a 50TB volume with default options, this brings the peak heap
usage down from 3.03GB to 24MB while also reducing the run time
slightly due to fewer writes and allocations.

There are still memory efficiency gains to be made here, notably we
still use gfs2_compute_bitstructs which allocates bitmap buffers and an
array of bh pointers for each rgrp.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 110 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 94 insertions(+), 16 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 2ed17d2..b6aa5dd 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -567,6 +567,93 @@ static int is_symlink(char *path, char **abspath)
 	return 1;
 }
 
+static int writerg(int fd, const struct rgrp_tree *rgt, const unsigned bsize)
+{
+	ssize_t ret = 0;
+	unsigned int i;
+	const struct gfs2_meta_header bmh = {
+		.mh_magic = GFS2_MAGIC,
+		.mh_type = GFS2_METATYPE_RB,
+		.mh_format = GFS2_FORMAT_RB,
+	};
+	struct iovec iov = {
+		.iov_len = rgt->ri.ri_length * bsize,
+		.iov_base = calloc(rgt->ri.ri_length, bsize),
+	};
+	if (iov.iov_base == NULL)
+		return -1;
+
+	gfs2_rgrp_out(&rgt->rg, iov.iov_base);
+	for (i = 1; i < rgt->ri.ri_length; i++)
+		gfs2_meta_header_out(&bmh, (char *)iov.iov_base + (i * bsize));
+
+	ret = pwritev(fd, &iov, 1, rgt->ri.ri_addr * bsize);
+	if (ret != iov.iov_len) {
+		free(iov.iov_base);
+		return -1;
+	}
+
+	free(iov.iov_base);
+	return 0;
+}
+
+static int place_rgrps(struct gfs2_sbd *sdp, int rgsize_specified)
+{
+	struct rgrp_tree *rgt = NULL;
+	uint64_t rgaddr = 0;
+	unsigned int i = 0;
+	int err = 0;
+
+	sdp->device.length -= sdp->sb_addr + 1;
+	sdp->new_rgrps = how_many_rgrps(sdp, &sdp->device, rgsize_specified);
+	rgaddr = sdp->sb_addr + 1;
+
+	for (i = 0; i < sdp->new_rgrps; i++) {
+		/* TODO: align to RAID stripes, etc. */
+		rgt = rgrp_insert(&sdp->rgtree, rgaddr);
+		if (rgt == NULL)
+			return -1;
+		if (i == 0)
+			rgt->length = sdp->device.length - ((sdp->new_rgrps - 1) * (sdp->device.length / sdp->new_rgrps));
+		else
+			rgt->length = sdp->device.length / sdp->new_rgrps;
+
+		/* Build the rindex entry */
+		rgt->ri.ri_length = rgblocks2bitblocks(sdp->bsize, rgt->length, &rgt->ri.ri_data);
+		rgt->ri.ri_addr = rgaddr;
+		rgt->ri.ri_data0 = rgaddr + rgt->ri.ri_length;
+		rgt->ri.ri_bitbytes = rgt->ri.ri_data / GFS2_NBBY;
+
+		/* Build the rgrp header */
+		memset(&rgt->rg, 0, sizeof(rgt->rg));
+		rgt->rg.rg_header.mh_magic = GFS2_MAGIC;
+		rgt->rg.rg_header.mh_type = GFS2_METATYPE_RG;
+		rgt->rg.rg_header.mh_format = GFS2_FORMAT_RG;
+		rgt->rg.rg_free = rgt->ri.ri_data;
+
+		/* TODO: This call allocates buffer heads and bitmap pointers
+		 * in rgt. We really shouldn't need to do that. */
+		err = gfs2_compute_bitstructs(sdp, rgt);
+		if (err != 0) {
+			fprintf(stderr, _("Could not compute bitmaps. "
+			        "Check resource group and block size options.\n"));
+			return -1;
+		}
+
+		err = writerg(sdp->device_fd, rgt, sdp->bsize);
+		if (err != 0) {
+			perror(_("Failed to write resource group"));
+			return -1;
+		}
+		sdp->blks_total += rgt->ri.ri_data;
+		rgaddr += rgt->length;
+	}
+
+	sdp->rgrps = sdp->new_rgrps;
+	sdp->fssize = rgt->ri.ri_data0 + rgt->ri.ri_data;
+	return 0;
+}
+
 /**
  * main_mkfs - do everything
  * @argc:
@@ -668,20 +755,13 @@ void main_mkfs(int argc, char *argv[])
 	if (!S_ISREG(sdp->dinfo.stat.st_mode) && discard)
 		discard_blocks(sdp);
 
-	/* Compute the resource group layouts */
-
-	compute_rgrp_layout(sdp, &sdp->rgtree, rgsize_specified);
-	debug_print_rgrps(sdp, &sdp->rgtree);
-
-	/* Generate a random uuid */
-	get_random_bytes(uuid, sizeof(uuid));
-
-	/* Build ondisk structures */
-
-	build_rgrps(sdp, TRUE);
+	error = place_rgrps(sdp, rgsize_specified);
+	if (error) {
+		fprintf(stderr, _("Failed to build resource groups\n"));
+		exit(1);
+	}
 	build_root(sdp);
 	build_master(sdp);
-	build_sb(sdp, uuid);
 	error = build_jindex(sdp);
 	if (error) {
 		fprintf(stderr, _("Error building '%s': %s\n"), "jindex", strerror(errno));
@@ -714,12 +794,12 @@ void main_mkfs(int argc, char *argv[])
 		fprintf(stderr, _("Error building '%s': %s\n"), "quota", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
+	get_random_bytes(uuid, sizeof(uuid));
+	build_sb(sdp, uuid);
 
 	do_init_inum(sdp);
 	do_init_statfs(sdp);
 
-	/* Cleanup */
-
 	inode_put(&sdp->md.rooti);
 	inode_put(&sdp->master_dir);
 	inode_put(&sdp->md.inum);
@@ -727,14 +807,12 @@ void main_mkfs(int argc, char *argv[])
 
 	gfs2_rgrp_free(&sdp->rgtree);
 	error = fsync(sdp->device_fd);
-
 	if (error){
 		perror(sdp->device_name);
 		exit(EXIT_FAILURE);
 	}
 
 	error = close(sdp->device_fd);
-
 	if (error){
 		perror(sdp->device_name);
 		exit(EXIT_FAILURE);
-- 
1.8.1.2

[Cluster-devel] [PATCH 4/5] gfs2-utils: Make the tool tests script more useful

[Andrew Price]

This makes the tool test script log command output to a file and only
remove the test file (now named testvol by default) on success so that
we can analyze it on failure.

Also adds some tests to create and check file systems with various
resource group sizes.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 .gitignore          |  2 ++
 tests/tool_tests.sh | 28 +++++++++++++++++++---------
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/.gitignore b/.gitignore
index bd47927..a1eadd0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -41,6 +41,8 @@ gfs2/fsck/fsck.gfs2
 gfs2/mkfs/mkfs.gfs2
 gfs2/tune/tunegfs2
 tests/check_libgfs2
+tests/testvol
+tests/tests.log
 group/gfs_control/gfs_control
 group/gfs_controld/gfs_controld
 ABOUT-NLS
diff --git a/tests/tool_tests.sh b/tests/tool_tests.sh
index 791b071..50be31a 100755
--- a/tests/tool_tests.sh
+++ b/tests/tool_tests.sh
@@ -11,7 +11,10 @@ MKFS="${TOPBUILDDIR}/gfs2/mkfs/mkfs.gfs2 -qO"
 FSCK="${TOPBUILDDIR}/gfs2/fsck/fsck.gfs2 -qn"
 
 # Name of the sparse file we'll use for testing
-TEST_TARGET=${TEST_TARGET:-test_sparse}
+TEST_TARGET=${TEST_TARGET:-testvol}
+# Log of test output
+TEST_LOG=${TEST_LOG:-tests.log}
+truncate -cs0 "${TEST_LOG}"
 # Size, in GB, of the sparse file we'll create to run the tests
 TEST_TARGET_SZ=${TEST_TARGET_SZ:-10}
 [ $TEST_TARGET_SZ -gt 0 ] || { echo "Target size (in GB) must be greater than 0" >&2; exit 1; }
@@ -22,17 +25,17 @@ fn_test()
 {
 	local expected="$1"
 	local cmd="$2"
-	echo -n "Running '$cmd' - (Exp: $expected Got: "
-	$cmd &> /dev/null;
+	echo -n "** Test '$cmd'" | tee -a "${TEST_LOG}"
+	$cmd &>> "${TEST_LOG}";
 	local ret=$?
-	echo -n "$ret) "
+	echo -n " (exp: $expected got: $ret) " | tee -a "${TEST_LOG}"
 	if [ "$ret" != "$expected" ];
 	then
-		echo "FAIL"
+		echo "FAIL" | tee -a "${TEST_LOG}"
 		TEST_RET=1
 		TEST_GRP_RET=1
 	else
-		echo "PASS"
+		echo "PASS" | tee -a "${TEST_LOG}"
 	fi
 }
 
@@ -44,7 +47,7 @@ fn_rm_target()
 fn_recreate_target()
 {
 	fn_rm_target
-	fn_test 0 "dd if=/dev/null of=$TEST_TARGET bs=1 count=0 seek=${TEST_TARGET_SZ}G"
+	fn_test 0 "truncate -s ${TEST_TARGET_SZ}G ${TEST_TARGET}"
 }
 
 
@@ -54,9 +57,16 @@ fn_test 0 "$MKFS -p lock_nolock $TEST_TARGET"
 fn_test 0 "$MKFS -p lock_dlm -t foo:bar $TEST_TARGET"
 fn_test 255 "$MKFS -p badprotocol $TEST_TARGET"
 fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 255 "$MKFS -p lock_nolock -r 31 $TEST_TARGET"
+fn_test 255 "$MKFS -p lock_nolock -r 2049 $TEST_TARGET"
+fn_test 0 "$MKFS -p lock_nolock -r 32 $TEST_TARGET"
+fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 0 "$MKFS -p lock_nolock -r 2048 $TEST_TARGET"
+fn_test 0 "$FSCK $TEST_TARGET"
 
 # Tests end here
 
-# Clean up
-fn_test 0 "rm -f $TEST_TARGET"
+# Only remove the test file on success
+[ "$TEST_RET" = "0" ] && fn_test 0 "rm -f $TEST_TARGET"
+echo "Tool test output written to ${TEST_LOG}"
 exit $TEST_RET
-- 
1.8.1.2

[Cluster-devel] [PATCH 5/5] mkfs.gfs2: Separate user options from file system params

[Andrew Price]

Previously the user-supplied options were tangled up with the calculated
file system parameters in the gfs2_sbd. This patch adds a mkfs_opts
structure to create a separation between the two and rearranges the code
to make that separation more apparent. It also adds some more tool tests
to be run with 'make check' and removes the now-unneeded orig_fssize
field from the gfs2_sbd definition.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/libgfs2.h |   1 -
 gfs2/mkfs/main_mkfs.c  | 493 ++++++++++++++++++++++---------------------------
 tests/tool_tests.sh    |  30 ++-
 3 files changed, 249 insertions(+), 275 deletions(-)

diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 1475227..b2457c4 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -295,7 +295,6 @@ struct gfs2_sbd {
 
 	uint64_t sb_addr;
 
-	uint64_t orig_fssize;
 	uint64_t fssize;
 	uint64_t blks_total;
 	uint64_t blks_alloced;
diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index b6aa5dd..610a641 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -26,8 +26,6 @@
 #include "libgfs2.h"
 #include "gfs2_mkfs.h"
 
-int discard = 1;
-
 /**
  * This function is for libgfs2's sake.
  */
@@ -41,13 +39,7 @@ void print_it(const char *label, const char *fmt, const char *fmt2, ...)
 	va_end(args);
 }
 
-/**
- * print_usage - print out usage information
- * @prog_name: The name of this program
- */
-
-static void
-print_usage(const char *prog_name)
+static void print_usage(const char *prog_name)
 {
 	int i;
 	const char *option, *param, *desc;
@@ -85,178 +77,157 @@ print_usage(const char *prog_name)
 	}
 }
 
+struct mkfs_opts {
+	unsigned bsize;
+	unsigned qcsize;
+	unsigned jsize;
+	unsigned rgsize;
+	uint64_t fssize;
+	uint32_t journals;
+	const char *lockproto;
+	const char *locktable;
+	const char *device;
+	unsigned discard:1;
+
+	unsigned got_bsize:1;
+	unsigned got_qcsize:1;
+	unsigned got_jsize:1;
+	unsigned got_rgsize:1;
+	unsigned got_fssize:1;
+	unsigned got_journals:1;
+	unsigned got_lockproto:1;
+	unsigned got_locktable:1;
+	unsigned got_device:1;
+
+	unsigned override:1;
+	unsigned quiet:1;
+	unsigned expert:1;
+	unsigned debug:1;
+	unsigned confirm:1;
+};
+
+static void opts_init(struct mkfs_opts *opts)
+{
+	memset(opts, 0, sizeof(*opts));
+	opts->discard = 1;
+	opts->journals = 1;
+	opts->bsize = GFS2_DEFAULT_BSIZE;
+	opts->jsize = GFS2_DEFAULT_JSIZE;
+	opts->qcsize = GFS2_DEFAULT_QCSIZE;
+	opts->rgsize = GFS2_DEFAULT_RGSIZE;
+	opts->lockproto = "lock_dlm";
+	opts->locktable = "";
+}
+
 #ifndef BLKDISCARD
 #define BLKDISCARD      _IO(0x12,119)
 #endif
 
-static int discard_blocks(struct gfs2_sbd *sdp)
+static int discard_blocks(int fd, uint64_t len, int debug)
 {
         __uint64_t range[2];
 
 	range[0] = 0;
-	range[1] = sdp->device.length * sdp->bsize;
-	if (sdp->debug)
+	range[1] = len;
+	if (debug)
 		/* Translators: "discard" is a request sent to a storage device to
 		 * discard a range of blocks. */
 		printf(_("Issuing discard request: range: %llu - %llu..."),
 		       (unsigned long long)range[0],
 		       (unsigned long long)range[1]);
-	if (ioctl(sdp->device_fd, BLKDISCARD, &range) < 0) {
-		if (sdp->debug)
+	if (ioctl(fd, BLKDISCARD, &range) < 0) {
+		if (debug)
 			printf("%s = %d\n", _("error"), errno);
 		return errno;
 	}
-	if (sdp->debug)
+	if (debug)
 		printf(_("Successful.\n"));
         return 0;
 }
 
-/**
- * decode_arguments - decode command line arguments and fill in the struct gfs2_sbd
- * @argc:
- * @argv:
- * @sdp: the decoded command line arguments
- *
- */
-
-static void decode_arguments(int argc, char *argv[], struct gfs2_sbd *sdp)
+static void opts_get(int argc, char *argv[], struct mkfs_opts *opts)
 {
-	int cont = TRUE;
-	int optchar;
-
-	memset(sdp->device_name, 0, sizeof(sdp->device_name));
-	sdp->md.journals = 1;
-	sdp->orig_fssize = 0;
-
-	while (cont) {
-		optchar = getopt(argc, argv, "-b:c:DhJ:j:KOp:qr:t:VX");
+	int c;
+	while (1) {
+		c = getopt(argc, argv, "-b:c:DhJ:j:KOp:qr:t:VX");
+		if (c == -1)
+			break;
 
-		switch (optchar) {
+		switch (c) {
 		case 'b':
-			sdp->bsize = atoi(optarg);
+			opts->bsize = atoi(optarg);
+			opts->got_bsize = 1;
 			break;
-
 		case 'c':
-			sdp->qcsize = atoi(optarg);
+			opts->qcsize = atoi(optarg);
+			opts->got_qcsize = 1;
 			break;
-
 		case 'D':
-			sdp->debug = TRUE;
+			opts->debug = 1;
 			break;
-
 		case 'h':
 			print_usage(argv[0]);
 			exit(0);
-			break;
-
 		case 'J':
-			sdp->jsize = atoi(optarg);
+			opts->jsize = atoi(optarg);
+			opts->got_jsize = 1;
 			break;
-
 		case 'j':
-			sdp->md.journals = atoi(optarg);
+			opts->journals = atoi(optarg);
+			opts->got_journals = 1;
 			break;
-
 		case 'K':
-			discard = 0;
+			opts->discard = 0;
 			break;
-
 		case 'O':
-			sdp->override = TRUE;
+			opts->override = 1;
 			break;
-
 		case 'p':
-			if (strlen(optarg) >= GFS2_LOCKNAME_LEN)
-				die( _("lock protocol name '%s' is too long\n"),
-				    optarg);
-			strcpy(sdp->lockproto, optarg);
+			opts->lockproto = optarg;
+			opts->got_lockproto = 1;
+			break;
+		case 't':
+			opts->locktable = optarg;
+			opts->got_locktable = 1;
 			break;
-
 		case 'q':
-			sdp->quiet = TRUE;
+			opts->quiet = 1;
 			break;
-
 		case 'r':
-			sdp->rgsize = atoi(optarg);
-			break;
-
-		case 't':
-			if (strlen(optarg) >= GFS2_LOCKNAME_LEN)
-				die( _("lock table name '%s' is too long\n"), optarg);
-			strcpy(sdp->locktable, optarg);
+			opts->rgsize = atoi(optarg);
+			opts->got_rgsize = 1;
 			break;
-
 		case 'V':
 			printf("mkfs.gfs2 %s (built %s %s)\n", VERSION,
 			       __DATE__, __TIME__);
 			printf(REDHAT_COPYRIGHT "\n");
 			exit(EXIT_SUCCESS);
 			break;
-
 		case 'X':
-			sdp->expert = TRUE;
+			opts->expert = 1;
 			break;
-
 		case ':':
 		case '?':
 			fprintf(stderr, _("Please use '-h' for help.\n"));
 			exit(EXIT_FAILURE);
 			break;
-
-		case EOF:
-			cont = FALSE;
-			break;
-
 		case 1:
 			if (strcmp(optarg, "gfs2") == 0)
 				continue;
-			if (!sdp->device_name[0])
-				strcpy(sdp->device_name, optarg);
-			else if (!sdp->orig_fssize &&
-				 isdigit(optarg[0]))
-				sdp->orig_fssize = atol(optarg);
-			else
+			if (!opts->got_device) {
+				opts->device = optarg;
+				opts->got_device = 1;
+			} else if (!opts->got_fssize && isdigit(optarg[0])) {
+				opts->fssize = atol(optarg);
+				opts->got_fssize = 1;
+			} else
 				die( _("More than one device specified (try -h for help)\n"));
 			break;
-
 		default:
-			die( _("Invalid option: %c\n"), optchar);
+			die( _("Invalid option: %c\n"), c);
 			break;
 		};
 	}
-
-	if ((sdp->device_name[0] == 0) && (optind < argc))
-		strcpy(sdp->device_name, argv[optind++]);
-
-	if (sdp->device_name[0] == '\0')
-		die( _("No device specified. Please use '-h' for help.\n"));
-
-	if (optind < argc)
-		sdp->orig_fssize = atol(argv[optind++]);
-
-	if (optind < argc)
-		die( _("Unrecognized argument: %s\n"), argv[optind]);
-
-	if (sdp->debug) {
-		printf( _("Command Line Arguments:\n"));
-		if (sdp->bsize != -1)
-			printf("  bsize = %u\n", sdp->bsize);
-		printf("  qcsize = %u\n", sdp->qcsize);
-		printf("  jsize = %u\n", sdp->jsize);
-		printf("  journals = %u\n", sdp->md.journals);
-		printf("  override = %d\n", sdp->override);
-		printf("  proto = %s\n", sdp->lockproto);
-		printf("  quiet = %d\n", sdp->quiet);
-		if (sdp->rgsize == (unsigned int)-1)
-			printf("  rgsize = %s\n", _("Optimize for best performance"));
-		else
-			printf("  rgsize = %u\n", sdp->rgsize);
-		printf("  table = %s\n", sdp->locktable);
-		printf("  device = %s\n", sdp->device_name);
-		if (sdp->orig_fssize)
-			printf("  block-count = %llu\n",
-			       (unsigned long long)sdp->orig_fssize);
-	}
 }
 
 /**
@@ -266,9 +237,9 @@ static void decode_arguments(int argc, char *argv[], struct gfs2_sbd *sdp)
  *
  */
 
-static void test_locking(char *lockproto, char *locktable)
+static void test_locking(const char *lockproto, const char *locktable)
 {
-	char *c;
+	const char *c;
 	/* Translators: A lock table is a string identifying a gfs2 file system
 	 * in a cluster, e.g. cluster_name:fs_name */
 	const char *errprefix = _("Invalid lock table:");
@@ -307,12 +278,6 @@ static void test_locking(char *lockproto, char *locktable)
 	}
 }
 
-/**
- * are_you_sure - protect lusers from themselves
- * @sdp: the command line
- *
- */
-
 static void are_you_sure(void)
 {
 	char *line = NULL;
@@ -345,59 +310,73 @@ static void are_you_sure(void)
 		free(line);
 }
 
-static void verify_bsize(struct gfs2_sbd *sdp)
+static unsigned choose_blocksize(struct mkfs_opts *opts, struct lgfs2_dev_info *dinfo)
 {
 	unsigned int x;
+	unsigned int bsize = opts->bsize;
 
-	/* Block sizes must be a power of two from 512 to 65536 */
+	if (!opts->got_bsize) {
+		if (S_ISREG(dinfo->stat.st_mode))
+			bsize = GFS2_DEFAULT_BSIZE;
+		/* See if optimal_io_size (the biggest I/O we can submit
+		   without incurring a penalty) is a suitable block size. */
+		else if (dinfo->io_optimal_size <= getpagesize() && dinfo->io_optimal_size >= dinfo->io_min_size)
+			bsize = dinfo->io_optimal_size;
+		/* See if physical_block_size (the smallest unit we can write
+		   without incurring read-modify-write penalty) is suitable. */
+		else if (dinfo->physical_block_size <= getpagesize() && dinfo->physical_block_size >= GFS2_DEFAULT_BSIZE)
+			bsize = dinfo->physical_block_size;
+		else
+			bsize = GFS2_DEFAULT_BSIZE;
 
+	}
+
+	/* Block sizes must be a power of two from 512 to 65536 */
 	for (x = 512; x; x <<= 1)
-		if (x == sdp->bsize)
+		if (x == bsize)
 			break;
 
-	if (!x || sdp->bsize > getpagesize())
+	if (!x || bsize > getpagesize())
 		die( _("Block size must be a power of two between 512 and %d\n"),
 		       getpagesize());
 
-	if (sdp->bsize < sdp->dinfo.logical_block_size) {
+	if (bsize < dinfo->logical_block_size) {
 		die( _("Error: Block size %d is less than minimum logical "
-		       "block size (%d).\n"), sdp->bsize,
-		     sdp->dinfo.logical_block_size);
+		       "block size (%d).\n"), bsize, dinfo->logical_block_size);
 	}
 
-	if (sdp->bsize < sdp->dinfo.physical_block_size) {
-		printf( _("WARNING: Block size %d is inefficient because it "
+	if (bsize < dinfo->physical_block_size) {
+		printf( _("Warning: Block size %d is inefficient because it "
 			  "is less than the physical block size (%d).\n"),
-			  sdp->bsize, sdp->dinfo.physical_block_size);
-		if (sdp->override)
-			return;
-
-		are_you_sure();
+			  bsize, dinfo->physical_block_size);
+		opts->confirm = 1;
 	}
+	return bsize;
 }
 
-static void verify_arguments(struct gfs2_sbd *sdp)
+static void opts_check(struct mkfs_opts *opts)
 {
-	if (!sdp->expert)
-		test_locking(sdp->lockproto, sdp->locktable);
-	if (sdp->expert) {
-		if (GFS2_EXP_MIN_RGSIZE > sdp->rgsize || sdp->rgsize > GFS2_MAX_RGSIZE)
+	if (!opts->expert)
+		test_locking(opts->lockproto, opts->locktable);
+	if (opts->expert) {
+		if (GFS2_EXP_MIN_RGSIZE > opts->rgsize || opts->rgsize > GFS2_MAX_RGSIZE)
 			/* Translators: gfs2 file systems are split into equal sized chunks called
 			   resource groups. We're checking that the user gave a valid size for them. */
 			die( _("bad resource group size\n"));
 	} else {
-		if (GFS2_MIN_RGSIZE > sdp->rgsize || sdp->rgsize > GFS2_MAX_RGSIZE)
+		if (GFS2_MIN_RGSIZE > opts->rgsize || opts->rgsize > GFS2_MAX_RGSIZE)
 			die( _("bad resource group size\n"));
 	}
 
-	if (!sdp->md.journals)
+	if (!opts->journals)
 		die( _("no journals specified\n"));
 
-	if (sdp->jsize < 8 || sdp->jsize > 1024)
+	if (opts->jsize < 8 || opts->jsize > 1024)
 		die( _("bad journal size\n"));
 
-	if (!sdp->qcsize || sdp->qcsize > 64)
+	if (!opts->qcsize || opts->qcsize > 64)
 		die( _("bad quota change size\n"));
+
 }
 
 static int get_file_output(int fd, char *buffer, size_t buflen)
@@ -499,27 +478,10 @@ fail:
 	exit(execv(args[0], args));
 }
 
-
-/**
- * print_results - print out summary information
- * @sdp: the command line
- *
- */
-
-static void
-print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
-	      unsigned char uuid[16])
+static void print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
+                          struct mkfs_opts *opts, unsigned char uuid[16])
 {
-	if (sdp->debug)
-		printf("\n");
-	else if (sdp->quiet)
-		return;
-
-	if (sdp->expert)
-		printf("%-27s%s\n", _("Expert mode:"), _("on"));
-
-	printf("%-27s%s\n", _("Device:"), sdp->device_name);
-
+	printf("%-27s%s\n", _("Device:"), opts->device);
 	printf("%-27s%u\n", _("Block size:"), sdp->bsize);
 	printf("%-27s%.2f %s (%llu %s)\n", _("Device size:"),
 	       /* Translators: "GB" here means "gigabytes" */
@@ -533,11 +495,11 @@ print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
 	printf("%-27s\"%s\"\n", _("Locking protocol:"), sdp->lockproto);
 	printf("%-27s\"%s\"\n", _("Lock table:"), sdp->locktable);
 
-	if (sdp->debug) {
-		printf("\n%-27s%u\n", _("Writes:"), sdp->writes);
+	if (opts->debug) {
+		printf("%-27s%u\n", _("Writes:"), sdp->writes);
 	}
 	/* Translators: "UUID" = universally unique identifier. */
-	printf("%-27s%s\n\n", _("UUID:"), str_uuid(uuid));
+	printf("%-27s%s\n", _("UUID:"), str_uuid(uuid));
 }
 
 
@@ -546,7 +508,7 @@ print_results(struct gfs2_sbd *sdp, uint64_t real_device_size,
  * otherwise return 0. Exit on errors. The caller must free the memory pointed
  * to by *abspath.
  */
-static int is_symlink(char *path, char **abspath)
+static int is_symlink(const char *path, char **abspath)
 {
 	struct stat lnkstat;
 
@@ -597,7 +559,7 @@ static int writerg(int fd, const struct rgrp_tree *rgt, const unsigned bsize)
 	return 0;
 }
 
-static int place_rgrps(struct gfs2_sbd *sdp, int rgsize_specified)
+static int place_rgrps(struct gfs2_sbd *sdp, const struct mkfs_opts *opts)
 {
 	struct rgrp_tree *rgt = NULL;
 	uint64_t rgaddr = 0;
@@ -605,7 +567,7 @@ static int place_rgrps(struct gfs2_sbd *sdp, int rgsize_specified)
 	int err = 0;
 
 	sdp->device.length -= sdp->sb_addr + 1;
-	sdp->new_rgrps = how_many_rgrps(sdp, &sdp->device, rgsize_specified);
+	sdp->new_rgrps = how_many_rgrps(sdp, &sdp->device, opts->got_rgsize);
 	rgaddr = sdp->sb_addr + 1;
 
 	for (i = 0; i < sdp->new_rgrps; i++) {
@@ -654,169 +616,160 @@ static int place_rgrps(struct gfs2_sbd *sdp, int rgsize_specified)
 	return 0;
 }
 
-/**
- * main_mkfs - do everything
- * @argc:
- * @argv:
- *
- */
+static void sbd_init(struct gfs2_sbd *sdp, struct mkfs_opts *opts, struct lgfs2_dev_info *dinfo, int fd)
+{
+	memset(sdp, 0, sizeof(struct gfs2_sbd));
+	sdp->time = time(NULL);
+	sdp->rgtree.osi_node = NULL;
+	sdp->rgsize = opts->rgsize;
+	sdp->qcsize = opts->qcsize;
+	sdp->jsize = opts->jsize;
+	sdp->md.journals = opts->journals;
+	sdp->device_fd = fd;
+	sdp->bsize = choose_blocksize(opts, dinfo);
+
+	if (compute_constants(sdp)) {
+		perror(_("Failed to compute file system constants"));
+		exit(1);
+	}
+	sdp->device.length = dinfo->size / sdp->bsize;
+	if (opts->got_fssize) {
+		if (opts->fssize > sdp->device.length) {
+			fprintf(stderr, _("Specified size is bigger than the device."));
+			die("%s %.2f %s (%llu %s)\n", _("Device size:"),
+			       dinfo->size / ((float)(1 << 30)), _("GB"),
+			       (unsigned long long)dinfo->size / sdp->bsize, _("blocks"));
+		}
+		/* TODO: Check if the fssize is too small, somehow */
+		sdp->device.length = opts->fssize;
+	}
+	strcpy(sdp->lockproto, opts->lockproto);
+	strcpy(sdp->locktable, opts->locktable);
+	if (opts->debug) {
+		printf(_("Calculated file system options:\n"));
+		printf("  bsize = %u\n", sdp->bsize);
+		printf("  qcsize = %u\n", sdp->qcsize);
+		printf("  jsize = %u\n", sdp->jsize);
+		printf("  journals = %u\n", sdp->md.journals);
+		printf("  proto = %s\n", sdp->lockproto);
+		printf("  rgsize = %u\n", sdp->rgsize);
+		printf("  table = %s\n", sdp->locktable);
+		printf("  fssize = %"PRIu64"\n", opts->fssize);
+	}
+}
+
 void main_mkfs(int argc, char *argv[])
 {
-	struct gfs2_sbd sbd, *sdp = &sbd;
+	struct gfs2_sbd sbd;
+	struct mkfs_opts opts;
+	struct lgfs2_dev_info dinfo;
 	int error;
-	int rgsize_specified = 0;
 	unsigned char uuid[16];
 	char *absname = NULL;
 	char *fdpath = NULL;
 	int islnk = 0;
+	int fd;
 
-	memset(sdp, 0, sizeof(struct gfs2_sbd));
-	sdp->bsize = -1;
-	sdp->jsize = GFS2_DEFAULT_JSIZE;
-	sdp->rgsize = -1;
-	sdp->qcsize = GFS2_DEFAULT_QCSIZE;
-	strcpy(sdp->lockproto, GFS2_DEFAULT_LOCKPROTO);
-	sdp->time = time(NULL);
-	sdp->rgtree.osi_node = NULL;
-
-	decode_arguments(argc, argv, sdp);
-	if (sdp->rgsize == -1)                 /* if rg size not specified */
-		sdp->rgsize = GFS2_DEFAULT_RGSIZE; /* default it for now */
-	else
-		rgsize_specified = TRUE;
-
-	verify_arguments(sdp);
+	opts_init(&opts);
+	opts_get(argc, argv, &opts);
 
-	sdp->device_fd = open(sdp->device_name, O_RDWR | O_CLOEXEC);
-	if (sdp->device_fd < 0){
-		perror(sdp->device_name);
+	fd = open(opts.device, O_RDWR | O_CLOEXEC);
+	if (fd < 0){
+		perror(opts.device);
 		exit(EXIT_FAILURE);
 	}
 
-	if (lgfs2_get_dev_info(sdp->device_fd, &sdp->dinfo) < 0) {
-		perror(sdp->device_name);
+	if (lgfs2_get_dev_info(fd, &dinfo) < 0) {
+		perror(opts.device);
 		exit(EXIT_FAILURE);
 	}
 
-	if (asprintf(&fdpath, "/proc/%d/fd/%d", getpid(), sdp->device_fd) < 0) {
+	opts_check(&opts);
+	sbd_init(&sbd, &opts, &dinfo, fd);
+
+	if (asprintf(&fdpath, "/proc/%d/fd/%d", getpid(), fd) < 0) {
 		perror(_("Failed to build string"));
 		exit(EXIT_FAILURE);
 	}
 
-	if (!sdp->override) {
-		islnk = is_symlink(sdp->device_name, &absname);
-		printf(_("This will destroy any data on %s.\n"), islnk ? absname : sdp->device_name);
+	if (!opts.override) {
+		islnk = is_symlink(opts.device, &absname);
+		printf(_("This will destroy any data on %s.\n"), islnk ? absname : opts.device);
 		free(absname);
 		check_dev_content(fdpath);
-		are_you_sure();
+		opts.confirm = 1;
 	}
 	free(fdpath);
 
-	if (sdp->bsize == -1) {
-		if (S_ISREG(sdp->dinfo.stat.st_mode))
-			sdp->bsize = GFS2_DEFAULT_BSIZE;
-		/* See if optimal_io_size (the biggest I/O we can submit
-		   without incurring a penalty) is a suitable block size. */
-		else if (sdp->dinfo.io_optimal_size <= getpagesize() &&
-		    sdp->dinfo.io_optimal_size >= sdp->dinfo.io_min_size)
-			sdp->bsize = sdp->dinfo.io_optimal_size;
-		/* See if physical_block_size (the smallest unit we can write
-		   without incurring read-modify-write penalty) is suitable. */
-		else if (sdp->dinfo.physical_block_size <= getpagesize() &&
-			 sdp->dinfo.physical_block_size >= GFS2_DEFAULT_BSIZE)
-			sdp->bsize = sdp->dinfo.physical_block_size;
-		else
-			sdp->bsize = GFS2_DEFAULT_BSIZE;
-
-		if (sdp->debug)
-			printf("\n%s %u\n", _("Using block size:"), sdp->bsize);
-	}
-	verify_bsize(sdp);
-
-	if (compute_constants(sdp)) {
-		perror(_("Failed to compute file system constants"));
-		exit(EXIT_FAILURE);
-	}
+	if (opts.confirm && !opts.override)
+		are_you_sure();
 
-	/* Convert optional block-count to basic blocks */
-	if (sdp->orig_fssize) {
-		sdp->orig_fssize *= sdp->bsize;
-		sdp->orig_fssize >>= GFS2_BASIC_BLOCK_SHIFT;
-		if (sdp->orig_fssize > sdp->device.length) {
-			fprintf(stderr, "%s:%s\n", argv[0],
-			        _("Specified size is bigger than the device."));
-			die("%s %.2f %s (%llu %s)\n", _("Device size:"),
-			       sdp->dinfo.size / ((float)(1 << 30)), _("GB"),
-			       (unsigned long long)sdp->dinfo.size / sdp->bsize, _("blocks"));
-		}
-		sdp->device.length = sdp->orig_fssize;
-	}
-	fix_device_geometry(sdp);
-	if (!S_ISREG(sdp->dinfo.stat.st_mode) && discard)
-		discard_blocks(sdp);
+	if (!S_ISREG(dinfo.stat.st_mode) && opts.discard)
+		discard_blocks(fd, sbd.bsize * sbd.device.length, opts.debug);
 
-	error = place_rgrps(sdp, rgsize_specified);
+	error = place_rgrps(&sbd, &opts);
 	if (error) {
 		fprintf(stderr, _("Failed to build resource groups\n"));
 		exit(1);
 	}
-	build_root(sdp);
-	build_master(sdp);
-	error = build_jindex(sdp);
+	build_root(&sbd);
+	build_master(&sbd);
+	error = build_jindex(&sbd);
 	if (error) {
 		fprintf(stderr, _("Error building '%s': %s\n"), "jindex", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
-	error = build_per_node(sdp);
+	error = build_per_node(&sbd);
 	if (error) {
 		fprintf(stderr, _("Error building '%s': %s\n"), "per_node", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
-	error = build_inum(sdp);
+	error = build_inum(&sbd);
 	if (error) {
 		fprintf(stderr, _("Error building '%s': %s\n"), "inum", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
-	gfs2_lookupi(sdp->master_dir, "inum", 4, &sdp->md.inum);
-	error = build_statfs(sdp);
+	gfs2_lookupi(sbd.master_dir, "inum", 4, &sbd.md.inum);
+	error = build_statfs(&sbd);
 	if (error) {
 		fprintf(stderr, _("Error building '%s': %s\n"), "statfs", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
-	gfs2_lookupi(sdp->master_dir, "statfs", 6, &sdp->md.statfs);
-	error = build_rindex(sdp);
+	gfs2_lookupi(sbd.master_dir, "statfs", 6, &sbd.md.statfs);
+	error = build_rindex(&sbd);
 	if (error) {
 		fprintf(stderr, _("Error building '%s': %s\n"), "rindex", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
-	error = build_quota(sdp);
+	error = build_quota(&sbd);
 	if (error) {
 		fprintf(stderr, _("Error building '%s': %s\n"), "quota", strerror(errno));
 		exit(EXIT_FAILURE);
 	}
 	get_random_bytes(uuid, sizeof(uuid));
-	build_sb(sdp, uuid);
+	build_sb(&sbd, uuid);
 
-	do_init_inum(sdp);
-	do_init_statfs(sdp);
+	do_init_inum(&sbd);
+	do_init_statfs(&sbd);
 
-	inode_put(&sdp->md.rooti);
-	inode_put(&sdp->master_dir);
-	inode_put(&sdp->md.inum);
-	inode_put(&sdp->md.statfs);
+	inode_put(&sbd.md.rooti);
+	inode_put(&sbd.master_dir);
+	inode_put(&sbd.md.inum);
+	inode_put(&sbd.md.statfs);
 
-	gfs2_rgrp_free(&sdp->rgtree);
-	error = fsync(sdp->device_fd);
+	gfs2_rgrp_free(&sbd.rgtree);
+	error = fsync(fd);
 	if (error){
-		perror(sdp->device_name);
+		perror(opts.device);
 		exit(EXIT_FAILURE);
 	}
 
-	error = close(sdp->device_fd);
+	error = close(fd);
 	if (error){
-		perror(sdp->device_name);
+		perror(opts.device);
 		exit(EXIT_FAILURE);
 	}
 
-	print_results(sdp, sdp->dinfo.size, uuid);
+	if (!opts.quiet)
+		print_results(&sbd, dinfo.size, &opts, uuid);
 }
diff --git a/tests/tool_tests.sh b/tests/tool_tests.sh
index 50be31a..7b9a5c3 100755
--- a/tests/tool_tests.sh
+++ b/tests/tool_tests.sh
@@ -5,10 +5,10 @@
 # of the whole script will be non-zero but the tests will continue to be run. The
 # sparse file which is used as the target of the tests can be configured by
 # setting the environment variables TEST_TARGET (the filename) and TEST_TARGET_SZ
-# (its apparent size in gigabytes). Defaults to "test_sparse" and 10GB.
+# (its apparent size in gigabytes). Defaults to "testvol" and 10GB.
 
-MKFS="${TOPBUILDDIR}/gfs2/mkfs/mkfs.gfs2 -qO"
-FSCK="${TOPBUILDDIR}/gfs2/fsck/fsck.gfs2 -qn"
+MKFS="${TOPBUILDDIR}/gfs2/mkfs/mkfs.gfs2 -O"
+FSCK="${TOPBUILDDIR}/gfs2/fsck/fsck.gfs2 -n"
 
 # Name of the sparse file we'll use for testing
 TEST_TARGET=${TEST_TARGET:-testvol}
@@ -50,19 +50,41 @@ fn_recreate_target()
 	fn_test 0 "truncate -s ${TEST_TARGET_SZ}G ${TEST_TARGET}"
 }
 
+util_max_blocks()
+{
+	printf $((TEST_TARGET_SZ*1073741824/$1))
+}
 
 # Tests start here
 fn_recreate_target
 fn_test 0 "$MKFS -p lock_nolock $TEST_TARGET"
 fn_test 0 "$MKFS -p lock_dlm -t foo:bar $TEST_TARGET"
-fn_test 255 "$MKFS -p badprotocol $TEST_TARGET"
 fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 255 "$MKFS -p badprotocol $TEST_TARGET"
 fn_test 255 "$MKFS -p lock_nolock -r 31 $TEST_TARGET"
 fn_test 255 "$MKFS -p lock_nolock -r 2049 $TEST_TARGET"
 fn_test 0 "$MKFS -p lock_nolock -r 32 $TEST_TARGET"
 fn_test 0 "$FSCK $TEST_TARGET"
 fn_test 0 "$MKFS -p lock_nolock -r 2048 $TEST_TARGET"
 fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 255 "$MKFS -p lock_nolock -b 512 $TEST_TARGET $(($(util_max_blocks 512)+1))"
+fn_test 255 "$MKFS -p lock_nolock -b 4096 $TEST_TARGET $(($(util_max_blocks 4096)+1))"
+fn_test 0 "$MKFS -p lock_nolock -b 512 $TEST_TARGET $(util_max_blocks 512)"
+fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 0 "$MKFS -p lock_nolock -b 4096 $TEST_TARGET $(util_max_blocks 4096)"
+fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 255 "$MKFS -p lock_nolock -J 7 $TEST_TARGET"
+fn_test 255 "$MKFS -p lock_nolock -J 1025 $TEST_TARGET"
+fn_test 0 "$MKFS -p lock_nolock -J 1024 $TEST_TARGET"
+fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 0 "$MKFS -p lock_nolock -J 8 $TEST_TARGET"
+fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 255 "$MKFS -p lock_nolock -c 0 $TEST_TARGET"
+fn_test 255 "$MKFS -p lock_nolock -c 65 $TEST_TARGET"
+fn_test 0 "$MKFS -p lock_nolock -c 1 $TEST_TARGET"
+fn_test 0 "$FSCK $TEST_TARGET"
+fn_test 0 "$MKFS -p lock_nolock -c 64 $TEST_TARGET"
+fn_test 0 "$FSCK $TEST_TARGET"
 
 # Tests end here
 
-- 
1.8.1.2