[Cluster-devel] [PATCH 0/8] gfs2-utils: Add support for new resource group fields

[Cluster-devel] [PATCH 0/8] gfs2-utils: Add support for new resource group fields

[Andrew Price]

These patches add basic support for the new fields in struct gfs2_rgrp as described in the previous posting.

Andrew Price (8):
  gfs2-utils configure: Check for rg_skip
  libgfs2: Add rgrp_skip support
  mkfs.gfs2: Pull place_journals() out of place_rgrps()
  mkfs.gfs2: Set the rg_skip field in new rgrps
  gfs2-utils configure: Check for rg_data0, rg_data and rg_bitbytes
  libgfs2: Add support for rg_data0, rg_data and rg_bitbytes
  mkfs.gfs2: Set the rg_data0, rg_data and rg_bitbytes fields
  libgfs2: Add support for rg_crc

 configure.ac             |  4 +++
 gfs2/libgfs2/libgfs2.h   |  4 ++-
 gfs2/libgfs2/meta.c      | 10 +++++++
 gfs2/libgfs2/ondisk.c    | 42 +++++++++++++++++++++++++++--
 gfs2/libgfs2/rgrp.c      | 70 +++++++++++++++++++++++++++++++++++++++++-------
 gfs2/mkfs/main_grow.c    |  9 ++++---
 gfs2/mkfs/main_mkfs.c    | 35 +++++++++++-------------
 tests/check_rgrp.c       |  2 +-
 tests/mkfs.at            | 32 ++++++++++++++++++++++
 tests/rgrifieldscheck.sh | 20 ++++++++++++++
 tests/rgskipcheck.sh     | 19 +++++++++++++
 11 files changed, 212 insertions(+), 35 deletions(-)
 create mode 100755 tests/rgrifieldscheck.sh
 create mode 100755 tests/rgskipcheck.sh

-- 
2.13.6

[Cluster-devel] [PATCH 1/8] gfs2-utils configure: Check for rg_skip

[Andrew Price]

Define GFS2_HAS_RG_SKIP if the new rg_skip field is found in struct
gfs2_rgrp.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index f1d83513..7f8bcca9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -137,6 +137,8 @@ AC_CHECK_MEMBER([struct gfs2_dirent.de_rahead],[AC_DEFINE([GFS2_HAS_DE_RAHEAD],[
                 [], [[#include <linux/gfs2_ondisk.h>]])
 AC_CHECK_MEMBER([struct gfs2_dirent.de_cookie],[AC_DEFINE([GFS2_HAS_DE_COOKIE],[],[Dirent cookie field])],
                 [], [[#include <linux/gfs2_ondisk.h>]])
+AC_CHECK_MEMBER([struct gfs2_rgrp.rg_skip],[AC_DEFINE([GFS2_HAS_RG_SKIP],[],[Next resource group pointer])],
+                [], [[#include <linux/gfs2_ondisk.h>]])
 
 # libuuid is only required if struct gfs2_sb.sb_uuid exists
 if test "$sb_has_uuid" = "yes" -a "$have_uuid" = "no"; then
-- 
2.13.6

[Cluster-devel] [PATCH 2/8] libgfs2: Add rgrp_skip support

[Andrew Price]

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/meta.c   |  4 ++++
 gfs2/libgfs2/ondisk.c | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/gfs2/libgfs2/meta.c b/gfs2/libgfs2/meta.c
index 500757d9..be91f0cc 100644
--- a/gfs2/libgfs2/meta.c
+++ b/gfs2/libgfs2/meta.c
@@ -201,7 +201,11 @@ MH(rg_header)
 F(rg_flags)
 F(rg_free, .flags = LGFS2_MFF_FSBLOCKS)
 F(rg_dinodes, .flags = LGFS2_MFF_FSBLOCKS)
+#ifdef GFS2_HAS_RG_SKIP
+FP(rg_skip, .points_to = (1 << LGFS2_MT_GFS2_RGRP))
+#else
 RF(__pad)
+#endif
 F(rg_igeneration)
 RF(rg_reserved)
 };
diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
index acedd3ea..b5c62114 100644
--- a/gfs2/libgfs2/ondisk.c
+++ b/gfs2/libgfs2/ondisk.c
@@ -227,6 +227,12 @@ void gfs2_rgrp_in(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
 	CPIN_32(rg, str, rg_flags);
 	CPIN_32(rg, str, rg_free);
 	CPIN_32(rg, str, rg_dinodes);
+#ifdef GFS2_HAS_RG_SKIP
+	CPIN_32(rg, str, rg_skip);
+#else
+	CPIN_32(rg, str, __pad);
+#endif
+	CPIN_64(rg, str, rg_igeneration);
 
 	CPIN_08(rg, str, rg_reserved, 80);
 }
@@ -239,6 +245,12 @@ void gfs2_rgrp_out(const struct gfs2_rgrp *rg, char *buf)
 	CPOUT_32(rg, str, rg_flags);
 	CPOUT_32(rg, str, rg_free);
 	CPOUT_32(rg, str, rg_dinodes);
+#ifdef GFS2_HAS_RG_SKIP
+	CPOUT_32(rg, str, rg_skip);
+#else
+	CPOUT_32(rg, str, __pad);
+#endif
+	CPOUT_64(rg, str, rg_igeneration);
 
 	CPOUT_08(rg, str, rg_reserved, 80);
 }
@@ -255,6 +267,12 @@ void gfs2_rgrp_print(const struct gfs2_rgrp *rg)
 	pv(rg, rg_flags, "%u", "0x%x");
 	pv(rg, rg_free, "%u", "0x%x");
 	pv(rg, rg_dinodes, "%u", "0x%x");
+#ifdef GFS2_HAS_RG_SKIP
+	pv(rg, rg_skip, "%u", "0x%x");
+#else
+	pv(rg, __pad, "%u", "0x%x");
+#endif
+	pv(rg, rg_igeneration, "%llu", "0x%llx");
 }
 
 void gfs2_quota_in(struct gfs2_quota *qu, char *buf)
-- 
2.13.6

[Cluster-devel] [PATCH 3/8] mkfs.gfs2: Pull place_journals() out of place_rgrps()

[Andrew Price]

Removes some clutter from place_rgrps() and makes main() a little more
informative.

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

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 2e08bc62..3b774115 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -713,7 +713,6 @@ static int place_journals(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_o
 	uint32_t rgsize = lgfs2_rgsize_for_data(jfsize, sdp->bsize);
 	unsigned j;
 
-	/* Initialise a progress bar for resource group creation. */
 	gfs2_progress_init(&progress, opts->journals, _("Adding journals: "), opts->quiet);
 
 	/* We'll build the jindex later so remember where we put the journals */
@@ -728,7 +727,6 @@ static int place_journals(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_o
 		lgfs2_rgrp_t rg;
 		struct gfs2_inode in = {0};
 
-		/* Update progress bar for journal creation. */
 		gfs2_progress_update(&progress, (j + 1));
 
 		if (opts->debug)
@@ -783,26 +781,19 @@ static int place_journals(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_o
 	return 0;
 }
 
-static int place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_opts *opts)
+static int place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, uint64_t *rgaddr, struct mkfs_opts *opts)
 {
 	struct gfs2_progress_bar progress;
-	uint64_t rgaddr = lgfs2_rgrp_align_addr(rgs, sdp->sb_addr + 1);
 	uint32_t rgblks = ((opts->rgsize << 20) / sdp->bsize);
 	uint32_t rgnum;
 	int result;
 
-	result = place_journals(sdp, rgs, opts, &rgaddr);
-	if (result != 0)
-		return result;
-
-	rgnum = lgfs2_rgrps_plan(rgs, sdp->device.length - rgaddr, rgblks);
-
-	/* Initialise a progress bar for resource group creation (after journal creation). */
+	rgnum = lgfs2_rgrps_plan(rgs, sdp->device.length - *rgaddr, rgblks);
 	gfs2_progress_init(&progress, (rgnum + opts->journals), _("Building resource groups: "), opts->quiet);
 
 	while (1) {
 		lgfs2_rgrp_t rg;
-		result = add_rgrp(rgs, &rgaddr, 0, &rg);
+		result = add_rgrp(rgs, rgaddr, 0, &rg);
 		if (result > 0)
 			break;
 		else if (result < 0)
@@ -814,7 +805,6 @@ static int place_rgrps(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs, struct mkfs_opts
 			return result;
 		}
 
-		/* Update progress bar with resource group address. */
 		gfs2_progress_update(&progress, (sdp->rgrps));
 	}
 	gfs2_progress_close(&progress, _("Done\n"));
@@ -944,6 +934,7 @@ int main(int argc, char *argv[])
 	struct gfs2_sb sb;
 	struct mkfs_opts opts;
 	lgfs2_rgrps_t rgs;
+	uint64_t rgaddr;
 	int error;
 	unsigned bsize;
 
@@ -993,8 +984,13 @@ int main(int argc, char *argv[])
 		if (!opts.quiet)
 			printf("%s", _("Done\n"));
 	}
-
-	error = place_rgrps(&sbd, rgs, &opts);
+	rgaddr = lgfs2_rgrp_align_addr(rgs, sbd.sb_addr + 1);
+	error = place_journals(&sbd, rgs, &opts, &rgaddr);
+	if (error != 0) {
+		fprintf(stderr, _("Failed to create journals\n"));
+		exit(1);
+	}
+	error = place_rgrps(&sbd, rgs, &rgaddr, &opts);
 	if (error) {
 		fprintf(stderr, _("Failed to build resource groups\n"));
 		exit(1);
-- 
2.13.6

[Cluster-devel] [PATCH 4/8] mkfs.gfs2: Set the rg_skip field in new rgrps

[Andrew Price]

This field is the difference between the block address of a rgrp and the
next one. Includes a test to check the rg_skip field against the rindex.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/libgfs2.h |  2 +-
 gfs2/libgfs2/rgrp.c    | 10 +++++++---
 gfs2/mkfs/main_grow.c  |  9 ++++++---
 gfs2/mkfs/main_mkfs.c  |  9 +++++----
 tests/check_rgrp.c     |  2 +-
 tests/mkfs.at          | 16 ++++++++++++++++
 tests/rgskipcheck.sh   | 19 +++++++++++++++++++
 7 files changed, 55 insertions(+), 12 deletions(-)
 create mode 100755 tests/rgskipcheck.sh

diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 2e29d332..50dc3f56 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -202,7 +202,7 @@ extern uint64_t lgfs2_rgrp_align_addr(const lgfs2_rgrps_t rgs, uint64_t addr);
 extern uint32_t lgfs2_rgrp_align_len(const lgfs2_rgrps_t rgs, uint32_t len);
 extern unsigned lgfs2_rgsize_for_data(uint64_t blksreq, unsigned bsize);
 extern uint32_t lgfs2_rgrps_plan(const lgfs2_rgrps_t rgs, uint64_t space, uint32_t tgtsize);
-extern lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry);
+extern lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, uint32_t rg_skip);
 extern int lgfs2_rgrp_bitbuf_alloc(lgfs2_rgrp_t rg);
 extern void lgfs2_rgrp_bitbuf_free(lgfs2_rgrp_t rg);
 extern int lgfs2_rgrp_write(int fd, lgfs2_rgrp_t rg);
diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index bb0776aa..4a2b1973 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -430,7 +430,7 @@ unsigned lgfs2_rindex_read_fd(int fd, lgfs2_rgrps_t rgs)
 			return 0;
 
 		gfs2_rindex_in(&ri, buf);
-		rg = lgfs2_rgrps_append(rgs, &ri);
+		rg = lgfs2_rgrps_append(rgs, &ri, 0);
 		if (rg == NULL)
 			return 0;
 		count++;
@@ -463,7 +463,7 @@ const struct gfs2_rindex *lgfs2_rindex_read_one(struct gfs2_inode *rip, lgfs2_rg
 		return NULL;
 
 	gfs2_rindex_in(&ri, buf);
-	rg = lgfs2_rgrps_append(rgs, &ri);
+	rg = lgfs2_rgrps_append(rgs, &ri, 0);
 	if (rg == NULL)
 		return NULL;
 
@@ -582,9 +582,10 @@ struct osi_node *lgfs2_rgrps_root(lgfs2_rgrps_t rgs)
  * Insert a new resource group after the last resource group in a set.
  * rgs: The set of resource groups
  * entry: The entry to be added
+ * rg_skip: The value to be used for this resource group's rg_skip field
  * Returns the new resource group on success or NULL on failure with errno set.
  */
-lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry)
+lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, uint32_t rg_skip)
 {
 	lgfs2_rgrp_t rg;
 	struct osi_node **link = &rgs->root.osi_node;
@@ -615,6 +616,9 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry)
 	rg->rg.rg_header.mh_type = GFS2_METATYPE_RG;
 	rg->rg.rg_header.mh_format = GFS2_FORMAT_RG;
 	rg->rg.rg_free = rg->ri.ri_data;
+#ifdef GFS2_HAS_RG_SKIP
+	rg->rg.rg_skip = rg_skip;
+#endif
 
 	compute_bitmaps(rg, rgs->sdp->bsize);
 	rg->rgrps = rgs;
diff --git a/gfs2/mkfs/main_grow.c b/gfs2/mkfs/main_grow.c
index dbee4bc6..ad80a8d9 100644
--- a/gfs2/mkfs/main_grow.c
+++ b/gfs2/mkfs/main_grow.c
@@ -213,14 +213,17 @@ static unsigned initialize_new_portion(struct gfs2_sbd *sdp, lgfs2_rgrps_t rgs)
 		int err = 0;
 		lgfs2_rgrp_t rg;
 		struct gfs2_rindex ri;
-		rgaddr = lgfs2_rindex_entry_new(rgs, &ri, rgaddr, 0);
-		if (rgaddr == 0)
+		uint64_t nextaddr;
+
+		nextaddr = lgfs2_rindex_entry_new(rgs, &ri, rgaddr, 0);
+		if (nextaddr == 0)
 			break;
-		rg = lgfs2_rgrps_append(rgs, &ri);
+		rg = lgfs2_rgrps_append(rgs, &ri, nextaddr - rgaddr);
 		if (rg == NULL) {
 			perror(_("Failed to create resource group"));
 			return 0;
 		}
+		rgaddr = nextaddr;
 		if (metafs_interrupted)
 			return 0;
 		if (!test)
diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 3b774115..54ff2db6 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -685,11 +685,12 @@ static int place_rgrp(struct gfs2_sbd *sdp, lgfs2_rgrp_t rg, int debug)
 static int add_rgrp(lgfs2_rgrps_t rgs, uint64_t *addr, uint32_t len, lgfs2_rgrp_t *rg)
 {
 	struct gfs2_rindex ri;
+	uint64_t nextaddr;
 
 	/* When we get to the end of the device, it's only an error if we have
 	   more structures left to write, i.e. when len is != 0. */
-	*addr = lgfs2_rindex_entry_new(rgs, &ri, *addr, len);
-	if (*addr == 0) {
+	nextaddr = lgfs2_rindex_entry_new(rgs, &ri, *addr, len);
+	if (nextaddr == 0) {
 		if (len != 0) {
 			perror(_("Failed to create resource group index entry"));
 			return -1;
@@ -697,12 +698,12 @@ static int add_rgrp(lgfs2_rgrps_t rgs, uint64_t *addr, uint32_t len, lgfs2_rgrp_
 			return 1;
 		}
 	}
-
-	*rg = lgfs2_rgrps_append(rgs, &ri);
+	*rg = lgfs2_rgrps_append(rgs, &ri, nextaddr - *addr);
 	if (*rg == NULL) {
 		perror(_("Failed to create resource group"));
 		return -1;
 	}
+	*addr = nextaddr;
 	return 0;
 }
 
diff --git a/tests/check_rgrp.c b/tests/check_rgrp.c
index ce22c387..0360d3c8 100644
--- a/tests/check_rgrp.c
+++ b/tests/check_rgrp.c
@@ -31,7 +31,7 @@ static lgfs2_rgrps_t mockup_rgrp(void)
 	addr = lgfs2_rindex_entry_new(rgs, &ri, 16, rgsize);
 	ck_assert(addr != 0);
 
-	rg = lgfs2_rgrps_append(rgs, &ri);
+	rg = lgfs2_rgrps_append(rgs, &ri, 0);
 	fail_unless(rg != NULL);
 
 	for (i = 0; i < rg->ri.ri_length; i++) {
diff --git a/tests/mkfs.at b/tests/mkfs.at
index 274a81db..b90d55e2 100644
--- a/tests/mkfs.at
+++ b/tests/mkfs.at
@@ -123,3 +123,19 @@ AT_CHECK([$GFS_MKFS -p lock_nolock -o test_topology=0:512:65536:393216:512 $GFS_
 # Check rgrp alignment to minimum_io_size: 65536 / 4096 == 16
 AT_CHECK([gfs2_edit -p rindex $GFS_TGT | grep ri_addr | awk '{print $2, $2 % 16; if ($2 % 16 != 0) { exit 1 }}'], 0, [ignore], [ignore])
 AT_CLEANUP
+
+AT_SETUP([Values of rg_skip])
+AT_KEYWORDS(mkfs.gfs2 mkfs)
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 2048 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 1024 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 512 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 219 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 32 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -o test_topology=0:512:65536:393216:512 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CLEANUP
diff --git a/tests/rgskipcheck.sh b/tests/rgskipcheck.sh
new file mode 100755
index 00000000..4a8f93fe
--- /dev/null
+++ b/tests/rgskipcheck.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+dev=$1
+rgcount=$(gfs2_edit -p rgcount $dev | cut -f1 -d' ')
+prevaddr=$(gfs2_edit -p rg 0 $dev | grep ^RG | awk '{print $5}')
+prevskip=0
+for i in `seq 0 $(($rgcount - 1))`; do
+	addr=$(gfs2_edit -p rg $i $dev | grep ^RG | awk '{print $5}')
+	expected=$(($addr - $prevaddr))
+
+	if test $prevskip != $expected; then
+		echo "Bad rg_skip in rg $(($i - 1)): $prevskip (expected: $expected)" >&2
+		exit 1
+	fi
+
+	prevskip=$(gfs2_edit -p rg $i $dev | grep rg_skip | awk '{print $2}')
+	prevaddr=$addr
+done
+
-- 
2.13.6

[Cluster-devel] [PATCH 4/8] mkfs.gfs2: Set the rg_skip field in new rgrps

[Bob Peterson]

----- Original Message -----
| This field is the difference between the block address of a rgrp and the
| next one. Includes a test to check the rg_skip field against the rindex.
| 
| Signed-off-by: Andrew Price <anprice@redhat.com>
| ---
(snip)
| @@ -615,6 +616,9 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct
| gfs2_rindex *entry)
|  	rg->rg.rg_header.mh_type = GFS2_METATYPE_RG;
|  	rg->rg.rg_header.mh_format = GFS2_FORMAT_RG;
|  	rg->rg.rg_free = rg->ri.ri_data;
| +#ifdef GFS2_HAS_RG_SKIP
| +	rg->rg.rg_skip = rg_skip;
| +#endif

We may still want to initialize the value of rg_skip if the #ifdef is false.

The rest looked good.

Bob Peterson

[Cluster-devel] [PATCH 4/8] mkfs.gfs2: Set the rg_skip field in new rgrps

[Andrew Price]

On 07/12/17 13:29, Bob Peterson wrote:
> ----- Original Message -----
> | This field is the difference between the block address of a rgrp and the
> | next one. Includes a test to check the rg_skip field against the rindex.
> |
> | Signed-off-by: Andrew Price <anprice@redhat.com>
> | ---
> (snip)
> | @@ -615,6 +616,9 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct
> | gfs2_rindex *entry)
> |  	rg->rg.rg_header.mh_type = GFS2_METATYPE_RG;
> |  	rg->rg.rg_header.mh_format = GFS2_FORMAT_RG;
> |  	rg->rg.rg_free = rg->ri.ri_data;
> | +#ifdef GFS2_HAS_RG_SKIP
> | +	rg->rg.rg_skip = rg_skip;
> | +#endif
> 
> We may still want to initialize the value of rg_skip if the #ifdef is false.

The structure is allocated and zeroed with calloc() in that function and 
rg_skip doesn't exist outside of the #ifdef so we're ok here.

> The rest looked good.

Cheers,
Andy

[Cluster-devel] [PATCH 5/8] gfs2-utils configure: Check for rg_data0, rg_data and rg_bitbytes

[Andrew Price]

Define GFS2_HAS_RG_RI_FIELDS if the new rg_data0 field is found in
struct gfs2_rgrp. This implies that rg_data and rg_bitbytes are also
present as they were added at the same time.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index 7f8bcca9..4e8518cb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -139,6 +139,8 @@ AC_CHECK_MEMBER([struct gfs2_dirent.de_cookie],[AC_DEFINE([GFS2_HAS_DE_COOKIE],[
                 [], [[#include <linux/gfs2_ondisk.h>]])
 AC_CHECK_MEMBER([struct gfs2_rgrp.rg_skip],[AC_DEFINE([GFS2_HAS_RG_SKIP],[],[Next resource group pointer])],
                 [], [[#include <linux/gfs2_ondisk.h>]])
+AC_CHECK_MEMBER([struct gfs2_rgrp.rg_data0],[AC_DEFINE([GFS2_HAS_RG_RI_FIELDS],[],[Resource group fields duplicated from the rindex])],
+                [], [[#include <linux/gfs2_ondisk.h>]])
 
 # libuuid is only required if struct gfs2_sb.sb_uuid exists
 if test "$sb_has_uuid" = "yes" -a "$have_uuid" = "no"; then
-- 
2.13.6

[Cluster-devel] [PATCH 5/8] gfs2-utils configure: Check for rg_data0, rg_data and rg_bitbytes

[Bob Peterson]

----- Original Message -----
| Define GFS2_HAS_RG_RI_FIELDS if the new rg_data0 field is found in
| struct gfs2_rgrp. This implies that rg_data and rg_bitbytes are also
| present as they were added at the same time.
| 
| Signed-off-by: Andrew Price <anprice@redhat.com>
| ---

ACK

Bob Peterson

[Cluster-devel] [PATCH 6/8] libgfs2: Add support for rg_data0, rg_data and rg_bitbytes

[Andrew Price]

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/meta.c   |  5 +++++
 gfs2/libgfs2/ondisk.c | 21 +++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/gfs2/libgfs2/meta.c b/gfs2/libgfs2/meta.c
index be91f0cc..1b771f30 100644
--- a/gfs2/libgfs2/meta.c
+++ b/gfs2/libgfs2/meta.c
@@ -207,6 +207,11 @@ FP(rg_skip, .points_to = (1 << LGFS2_MT_GFS2_RGRP))
 RF(__pad)
 #endif
 F(rg_igeneration)
+#ifdef GFS2_HAS_RG_RI_FIELDS
+FP(rg_data0, .points_to = ANY_GFS2_BLOCK|(1 << LGFS2_MT_FREE))
+F(rg_data, .flags = LGFS2_MFF_FSBLOCKS)
+F(rg_bitbytes, .flags = LGFS2_MFF_BYTES)
+#endif
 RF(rg_reserved)
 };
 
diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
index b5c62114..fd25a860 100644
--- a/gfs2/libgfs2/ondisk.c
+++ b/gfs2/libgfs2/ondisk.c
@@ -233,8 +233,14 @@ void gfs2_rgrp_in(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
 	CPIN_32(rg, str, __pad);
 #endif
 	CPIN_64(rg, str, rg_igeneration);
-
+#ifdef GFS2_HAS_RG_RI_FIELDS
+	CPIN_64(rg, str, rg_data0);
+	CPIN_32(rg, str, rg_data);
+	CPIN_32(rg, str, rg_bitbytes);
+	CPIN_08(rg, str, rg_reserved, 64);
+#else
 	CPIN_08(rg, str, rg_reserved, 80);
+#endif
 }
 
 void gfs2_rgrp_out(const struct gfs2_rgrp *rg, char *buf)
@@ -251,8 +257,14 @@ void gfs2_rgrp_out(const struct gfs2_rgrp *rg, char *buf)
 	CPOUT_32(rg, str, __pad);
 #endif
 	CPOUT_64(rg, str, rg_igeneration);
-
+#ifdef GFS2_HAS_RG_RI_FIELDS
+	CPOUT_64(rg, str, rg_data0);
+	CPOUT_32(rg, str, rg_data);
+	CPOUT_32(rg, str, rg_bitbytes);
+	CPOUT_08(rg, str, rg_reserved, 64);
+#else
 	CPOUT_08(rg, str, rg_reserved, 80);
+#endif
 }
 
 void gfs2_rgrp_out_bh(const struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
@@ -273,6 +285,11 @@ void gfs2_rgrp_print(const struct gfs2_rgrp *rg)
 	pv(rg, __pad, "%u", "0x%x");
 #endif
 	pv(rg, rg_igeneration, "%llu", "0x%llx");
+#ifdef GFS2_HAS_RG_RI_FIELDS
+	pv(rg, rg_data0, "%llu", "0x%llx");
+	pv(rg, rg_data, "%u", "0x%x");
+	pv(rg, rg_bitbytes, "%u", "0x%x");
+#endif
 }
 
 void gfs2_quota_in(struct gfs2_quota *qu, char *buf)
-- 
2.13.6

[Cluster-devel] [PATCH 6/8] libgfs2: Add support for rg_data0, rg_data and rg_bitbytes

[Bob Peterson]

----- Original Message -----
| Signed-off-by: Andrew Price <anprice@redhat.com>
| ---
|  gfs2/libgfs2/meta.c   |  5 +++++
|  gfs2/libgfs2/ondisk.c | 21 +++++++++++++++++++--
|  2 files changed, 24 insertions(+), 2 deletions(-)

ACK

Bob Peterson

[Cluster-devel] [PATCH 7/8] mkfs.gfs2: Set the rg_data0, rg_data and rg_bitbytes fields

[Andrew Price]

These fields are duplicated from the rindex entry associated with the
rgrp in order to reduce dependency on the rindex.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/rgrp.c      |  6 +++++-
 tests/mkfs.at            | 16 ++++++++++++++++
 tests/rgrifieldscheck.sh | 17 +++++++++++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100755 tests/rgrifieldscheck.sh

diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index 4a2b1973..17178bd9 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -619,7 +619,11 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, ui
 #ifdef GFS2_HAS_RG_SKIP
 	rg->rg.rg_skip = rg_skip;
 #endif
-
+#ifdef GFS2_HAS_RG_RI_FIELDS
+	rg->rg.rg_data0 = rg->ri.ri_data0;
+	rg->rg.rg_data = rg->ri.ri_data;
+	rg->rg.rg_bitbytes = rg->ri.ri_bitbytes;
+#endif
 	compute_bitmaps(rg, rgs->sdp->bsize);
 	rg->rgrps = rgs;
 	return rg;
diff --git a/tests/mkfs.at b/tests/mkfs.at
index b90d55e2..be888171 100644
--- a/tests/mkfs.at
+++ b/tests/mkfs.at
@@ -139,3 +139,19 @@ AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
 AT_CHECK([$GFS_MKFS -p lock_nolock -o test_topology=0:512:65536:393216:512 $GFS_TGT], 0, [ignore], [ignore])
 AT_CHECK([rgskipcheck.sh $GFS_TGT], 0, [ignore], [ignore])
 AT_CLEANUP
+
+AT_SETUP([Values of rg_data0, rg_data, rg_bitbytes])
+AT_KEYWORDS(mkfs.gfs2 mkfs)
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 2048 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgrifieldscheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 1024 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgrifieldscheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 512 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgrifieldscheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 219 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgrifieldscheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -r 32 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgrifieldscheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -o test_topology=0:512:65536:393216:512 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([rgrifieldscheck.sh $GFS_TGT], 0, [ignore], [ignore])
+AT_CLEANUP
diff --git a/tests/rgrifieldscheck.sh b/tests/rgrifieldscheck.sh
new file mode 100755
index 00000000..c99470a9
--- /dev/null
+++ b/tests/rgrifieldscheck.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+dev=$1
+i=0
+gfs2_edit -p rindex $dev | while read field rival unused
+do
+	test $field = ri_data0 -o $field = ri_data -o $field = ri_bitbytes || continue
+	rgfield=$(echo $field | sed 's/ri/rg/')
+	rgval=$(gfs2_edit -p rg $i $dev | grep " $rgfield " | awk '{print $2}')
+
+	if test "$rival" != "$rgval"
+	then
+		echo "Bad $rgfield in rg $i: $rgval (expected: $rival)" >&2
+		exit 1
+	fi
+
+	test $field = ri_bitbytes && let i++
+done
-- 
2.13.6

[Cluster-devel] [PATCH 7/8] mkfs.gfs2: Set the rg_data0, rg_data and rg_bitbytes fields

[Bob Peterson]

----- Original Message -----
| These fields are duplicated from the rindex entry associated with the
| rgrp in order to reduce dependency on the rindex.
| 
| Signed-off-by: Andrew Price <anprice@redhat.com>
| ---

ACK

Bob Peterson

[Cluster-devel] [PATCH 8/8] libgfs2: Add support for rg_crc

[Andrew Price]

This new field in the resource group headers is set on write and checked
on read so that fsck.gfs2 now considers a resource group with a bad,
non-zero crc to be corrupted. gfs2_grow, gfs2_edit, and mkfs.gfs2 all
now support the rg_crc field, including save/restoremeta.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/libgfs2/libgfs2.h   |  2 ++
 gfs2/libgfs2/meta.c      |  1 +
 gfs2/libgfs2/ondisk.c    |  7 +++++--
 gfs2/libgfs2/rgrp.c      | 54 +++++++++++++++++++++++++++++++++++++++++++-----
 tests/rgrifieldscheck.sh |  3 +++
 5 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 50dc3f56..5309e6d6 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -667,6 +667,8 @@ extern int clean_journal(struct gfs2_inode *ip, struct gfs2_log_header *head);
 /* rgrp.c */
 extern int gfs2_compute_bitstructs(const uint32_t bsize, struct rgrp_tree *rgd);
 extern struct rgrp_tree *gfs2_blk2rgrpd(struct gfs2_sbd *sdp, uint64_t blk);
+extern int lgfs2_rgrp_crc_check(const struct gfs2_rgrp *rg, char *buf);
+extern void lgfs2_rgrp_crc_set(char *buf);
 extern uint64_t gfs2_rgrp_read(struct gfs2_sbd *sdp, struct rgrp_tree *rgd);
 extern void gfs2_rgrp_relse(struct rgrp_tree *rgd);
 extern struct rgrp_tree *rgrp_insert(struct osi_root *rgtree,
diff --git a/gfs2/libgfs2/meta.c b/gfs2/libgfs2/meta.c
index 1b771f30..82c8c6be 100644
--- a/gfs2/libgfs2/meta.c
+++ b/gfs2/libgfs2/meta.c
@@ -211,6 +211,7 @@ F(rg_igeneration)
 FP(rg_data0, .points_to = ANY_GFS2_BLOCK|(1 << LGFS2_MT_FREE))
 F(rg_data, .flags = LGFS2_MFF_FSBLOCKS)
 F(rg_bitbytes, .flags = LGFS2_MFF_BYTES)
+F(rg_crc, .flags = LGFS2_MFF_CHECK)
 #endif
 RF(rg_reserved)
 };
diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
index fd25a860..dfb99d14 100644
--- a/gfs2/libgfs2/ondisk.c
+++ b/gfs2/libgfs2/ondisk.c
@@ -237,7 +237,8 @@ void gfs2_rgrp_in(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
 	CPIN_64(rg, str, rg_data0);
 	CPIN_32(rg, str, rg_data);
 	CPIN_32(rg, str, rg_bitbytes);
-	CPIN_08(rg, str, rg_reserved, 64);
+	CPIN_32(rg, str, rg_crc);
+	CPIN_08(rg, str, rg_reserved, 60);
 #else
 	CPIN_08(rg, str, rg_reserved, 80);
 #endif
@@ -261,7 +262,8 @@ void gfs2_rgrp_out(const struct gfs2_rgrp *rg, char *buf)
 	CPOUT_64(rg, str, rg_data0);
 	CPOUT_32(rg, str, rg_data);
 	CPOUT_32(rg, str, rg_bitbytes);
-	CPOUT_08(rg, str, rg_reserved, 64);
+	CPOUT_08(rg, str, rg_reserved, 60);
+	lgfs2_rgrp_crc_set(buf);
 #else
 	CPOUT_08(rg, str, rg_reserved, 80);
 #endif
@@ -289,6 +291,7 @@ void gfs2_rgrp_print(const struct gfs2_rgrp *rg)
 	pv(rg, rg_data0, "%llu", "0x%llx");
 	pv(rg, rg_data, "%u", "0x%x");
 	pv(rg, rg_bitbytes, "%u", "0x%x");
+	pv(rg, rg_crc, "%u", "0x%x");
 #endif
 }
 
diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index 17178bd9..03861d14 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -158,6 +158,42 @@ void lgfs2_rgrp_bitbuf_free(lgfs2_rgrp_t rg)
 }
 
 /**
+ * Check a resource group's crc
+ * Returns 0 on success, non-zero if crc is bad
+ */
+int lgfs2_rgrp_crc_check(const struct gfs2_rgrp *rg, char *buf)
+{
+	int ret = 0;
+#ifdef GFS2_HAS_RG_RI_FIELDS
+	uint32_t crc = rg->rg_crc;
+
+	if (crc == 0)
+		return 0;
+
+	((struct gfs2_rgrp *)buf)->rg_crc = 0;
+	if (crc != gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp)))
+		ret = 1;
+	((struct gfs2_rgrp *)buf)->rg_crc = cpu_to_be32(crc);
+#endif
+	return ret;
+}
+
+/**
+ * Set the crc of an on-disk resource group
+ */
+void lgfs2_rgrp_crc_set(char *buf)
+{
+#ifdef GFS2_HAS_RG_RI_FIELDS
+	struct gfs2_rgrp *rg = (struct gfs2_rgrp *)buf;
+	uint32_t crc;
+
+	rg->rg_crc = 0;
+	crc = gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp));
+	rg->rg_crc = cpu_to_be32(crc);
+#endif
+}
+
+/**
  * gfs2_rgrp_read - read in the resource group information from disk.
  * @rgd - resource group structure
  * returns: 0 if no error, otherwise the block number that failed
@@ -194,11 +230,18 @@ uint64_t gfs2_rgrp_read(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
 			return rgd->ri.ri_addr + err;
 		}
 	}
-	if (x > 0) {
-		if (sdp->gfs1)
-			gfs_rgrp_in((struct gfs_rgrp *)&rgd->rg, rgd->bits[0].bi_bh);
-		else
-			gfs2_rgrp_in(&rgd->rg, rgd->bits[0].bi_bh);
+	if (x <= 0) {
+		free(bhs);
+		return 0;
+	}
+	if (sdp->gfs1)
+		gfs_rgrp_in((struct gfs_rgrp *)&rgd->rg, rgd->bits[0].bi_bh);
+	else {
+		gfs2_rgrp_in(&rgd->rg, rgd->bits[0].bi_bh);
+		if (lgfs2_rgrp_crc_check(&rgd->rg, rgd->bits[0].bi_bh->b_data)) {
+			free(bhs);
+			return rgd->ri.ri_addr;
+		}
 	}
 	free(bhs);
 	return 0;
@@ -623,6 +666,7 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, ui
 	rg->rg.rg_data0 = rg->ri.ri_data0;
 	rg->rg.rg_data = rg->ri.ri_data;
 	rg->rg.rg_bitbytes = rg->ri.ri_bitbytes;
+	rg->rg.rg_crc = 0;
 #endif
 	compute_bitmaps(rg, rgs->sdp->bsize);
 	rg->rgrps = rgs;
diff --git a/tests/rgrifieldscheck.sh b/tests/rgrifieldscheck.sh
index c99470a9..85f6408e 100755
--- a/tests/rgrifieldscheck.sh
+++ b/tests/rgrifieldscheck.sh
@@ -1,6 +1,9 @@
 #!/bin/sh
 dev=$1
 i=0
+gfs2_edit -p rg 0 $dev | grep rg_data0 > /dev/null 2>&1
+# New fields not present in /usr/include/linux/gfs2_ondisk.h
+test $? = 0 || exit 0
 gfs2_edit -p rindex $dev | while read field rival unused
 do
 	test $field = ri_data0 -o $field = ri_data -o $field = ri_bitbytes || continue
-- 
2.13.6

[Cluster-devel] [PATCH 8/8] libgfs2: Add support for rg_crc

[Steven Whitehouse]

Excellent - looks really good :-)

Steve.


On 07/12/17 11:53, Andrew Price wrote:
> This new field in the resource group headers is set on write and checked
> on read so that fsck.gfs2 now considers a resource group with a bad,
> non-zero crc to be corrupted. gfs2_grow, gfs2_edit, and mkfs.gfs2 all
> now support the rg_crc field, including save/restoremeta.
>
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>   gfs2/libgfs2/libgfs2.h   |  2 ++
>   gfs2/libgfs2/meta.c      |  1 +
>   gfs2/libgfs2/ondisk.c    |  7 +++++--
>   gfs2/libgfs2/rgrp.c      | 54 +++++++++++++++++++++++++++++++++++++++++++-----
>   tests/rgrifieldscheck.sh |  3 +++
>   5 files changed, 60 insertions(+), 7 deletions(-)
>
> diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> index 50dc3f56..5309e6d6 100644
> --- a/gfs2/libgfs2/libgfs2.h
> +++ b/gfs2/libgfs2/libgfs2.h
> @@ -667,6 +667,8 @@ extern int clean_journal(struct gfs2_inode *ip, struct gfs2_log_header *head);
>   /* rgrp.c */
>   extern int gfs2_compute_bitstructs(const uint32_t bsize, struct rgrp_tree *rgd);
>   extern struct rgrp_tree *gfs2_blk2rgrpd(struct gfs2_sbd *sdp, uint64_t blk);
> +extern int lgfs2_rgrp_crc_check(const struct gfs2_rgrp *rg, char *buf);
> +extern void lgfs2_rgrp_crc_set(char *buf);
>   extern uint64_t gfs2_rgrp_read(struct gfs2_sbd *sdp, struct rgrp_tree *rgd);
>   extern void gfs2_rgrp_relse(struct rgrp_tree *rgd);
>   extern struct rgrp_tree *rgrp_insert(struct osi_root *rgtree,
> diff --git a/gfs2/libgfs2/meta.c b/gfs2/libgfs2/meta.c
> index 1b771f30..82c8c6be 100644
> --- a/gfs2/libgfs2/meta.c
> +++ b/gfs2/libgfs2/meta.c
> @@ -211,6 +211,7 @@ F(rg_igeneration)
>   FP(rg_data0, .points_to = ANY_GFS2_BLOCK|(1 << LGFS2_MT_FREE))
>   F(rg_data, .flags = LGFS2_MFF_FSBLOCKS)
>   F(rg_bitbytes, .flags = LGFS2_MFF_BYTES)
> +F(rg_crc, .flags = LGFS2_MFF_CHECK)
>   #endif
>   RF(rg_reserved)
>   };
> diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
> index fd25a860..dfb99d14 100644
> --- a/gfs2/libgfs2/ondisk.c
> +++ b/gfs2/libgfs2/ondisk.c
> @@ -237,7 +237,8 @@ void gfs2_rgrp_in(struct gfs2_rgrp *rg, struct gfs2_buffer_head *bh)
>   	CPIN_64(rg, str, rg_data0);
>   	CPIN_32(rg, str, rg_data);
>   	CPIN_32(rg, str, rg_bitbytes);
> -	CPIN_08(rg, str, rg_reserved, 64);
> +	CPIN_32(rg, str, rg_crc);
> +	CPIN_08(rg, str, rg_reserved, 60);
>   #else
>   	CPIN_08(rg, str, rg_reserved, 80);
>   #endif
> @@ -261,7 +262,8 @@ void gfs2_rgrp_out(const struct gfs2_rgrp *rg, char *buf)
>   	CPOUT_64(rg, str, rg_data0);
>   	CPOUT_32(rg, str, rg_data);
>   	CPOUT_32(rg, str, rg_bitbytes);
> -	CPOUT_08(rg, str, rg_reserved, 64);
> +	CPOUT_08(rg, str, rg_reserved, 60);
> +	lgfs2_rgrp_crc_set(buf);
>   #else
>   	CPOUT_08(rg, str, rg_reserved, 80);
>   #endif
> @@ -289,6 +291,7 @@ void gfs2_rgrp_print(const struct gfs2_rgrp *rg)
>   	pv(rg, rg_data0, "%llu", "0x%llx");
>   	pv(rg, rg_data, "%u", "0x%x");
>   	pv(rg, rg_bitbytes, "%u", "0x%x");
> +	pv(rg, rg_crc, "%u", "0x%x");
>   #endif
>   }
>   
> diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
> index 17178bd9..03861d14 100644
> --- a/gfs2/libgfs2/rgrp.c
> +++ b/gfs2/libgfs2/rgrp.c
> @@ -158,6 +158,42 @@ void lgfs2_rgrp_bitbuf_free(lgfs2_rgrp_t rg)
>   }
>   
>   /**
> + * Check a resource group's crc
> + * Returns 0 on success, non-zero if crc is bad
> + */
> +int lgfs2_rgrp_crc_check(const struct gfs2_rgrp *rg, char *buf)
> +{
> +	int ret = 0;
> +#ifdef GFS2_HAS_RG_RI_FIELDS
> +	uint32_t crc = rg->rg_crc;
> +
> +	if (crc == 0)
> +		return 0;
> +
> +	((struct gfs2_rgrp *)buf)->rg_crc = 0;
> +	if (crc != gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp)))
> +		ret = 1;
> +	((struct gfs2_rgrp *)buf)->rg_crc = cpu_to_be32(crc);
> +#endif
> +	return ret;
> +}
> +
> +/**
> + * Set the crc of an on-disk resource group
> + */
> +void lgfs2_rgrp_crc_set(char *buf)
> +{
> +#ifdef GFS2_HAS_RG_RI_FIELDS
> +	struct gfs2_rgrp *rg = (struct gfs2_rgrp *)buf;
> +	uint32_t crc;
> +
> +	rg->rg_crc = 0;
> +	crc = gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp));
> +	rg->rg_crc = cpu_to_be32(crc);
> +#endif
> +}
> +
> +/**
>    * gfs2_rgrp_read - read in the resource group information from disk.
>    * @rgd - resource group structure
>    * returns: 0 if no error, otherwise the block number that failed
> @@ -194,11 +230,18 @@ uint64_t gfs2_rgrp_read(struct gfs2_sbd *sdp, struct rgrp_tree *rgd)
>   			return rgd->ri.ri_addr + err;
>   		}
>   	}
> -	if (x > 0) {
> -		if (sdp->gfs1)
> -			gfs_rgrp_in((struct gfs_rgrp *)&rgd->rg, rgd->bits[0].bi_bh);
> -		else
> -			gfs2_rgrp_in(&rgd->rg, rgd->bits[0].bi_bh);
> +	if (x <= 0) {
> +		free(bhs);
> +		return 0;
> +	}
> +	if (sdp->gfs1)
> +		gfs_rgrp_in((struct gfs_rgrp *)&rgd->rg, rgd->bits[0].bi_bh);
> +	else {
> +		gfs2_rgrp_in(&rgd->rg, rgd->bits[0].bi_bh);
> +		if (lgfs2_rgrp_crc_check(&rgd->rg, rgd->bits[0].bi_bh->b_data)) {
> +			free(bhs);
> +			return rgd->ri.ri_addr;
> +		}
>   	}
>   	free(bhs);
>   	return 0;
> @@ -623,6 +666,7 @@ lgfs2_rgrp_t lgfs2_rgrps_append(lgfs2_rgrps_t rgs, struct gfs2_rindex *entry, ui
>   	rg->rg.rg_data0 = rg->ri.ri_data0;
>   	rg->rg.rg_data = rg->ri.ri_data;
>   	rg->rg.rg_bitbytes = rg->ri.ri_bitbytes;
> +	rg->rg.rg_crc = 0;
>   #endif
>   	compute_bitmaps(rg, rgs->sdp->bsize);
>   	rg->rgrps = rgs;
> diff --git a/tests/rgrifieldscheck.sh b/tests/rgrifieldscheck.sh
> index c99470a9..85f6408e 100755
> --- a/tests/rgrifieldscheck.sh
> +++ b/tests/rgrifieldscheck.sh
> @@ -1,6 +1,9 @@
>   #!/bin/sh
>   dev=$1
>   i=0
> +gfs2_edit -p rg 0 $dev | grep rg_data0 > /dev/null 2>&1
> +# New fields not present in /usr/include/linux/gfs2_ondisk.h
> +test $? = 0 || exit 0
>   gfs2_edit -p rindex $dev | while read field rival unused
>   do
>   	test $field = ri_data0 -o $field = ri_data -o $field = ri_bitbytes || continue

[Cluster-devel] [PATCH 8/8] libgfs2: Add support for rg_crc

[Bob Peterson]

Hi,

----- Original Message -----
| This new field in the resource group headers is set on write and checked
| on read so that fsck.gfs2 now considers a resource group with a bad,
| non-zero crc to be corrupted. gfs2_grow, gfs2_edit, and mkfs.gfs2 all
| now support the rg_crc field, including save/restoremeta.
| 
| Signed-off-by: Andrew Price <anprice@redhat.com>
| ---
(snip)
| diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
| index 17178bd9..03861d14 100644
| --- a/gfs2/libgfs2/rgrp.c
| +++ b/gfs2/libgfs2/rgrp.c
| @@ -158,6 +158,42 @@ void lgfs2_rgrp_bitbuf_free(lgfs2_rgrp_t rg)
|  }
|  
|  /**
| + * Check a resource group's crc
| + * Returns 0 on success, non-zero if crc is bad
| + */
| +int lgfs2_rgrp_crc_check(const struct gfs2_rgrp *rg, char *buf)
| +{
| +	int ret = 0;
| +#ifdef GFS2_HAS_RG_RI_FIELDS
| +	uint32_t crc = rg->rg_crc;
| +
| +	if (crc == 0)
| +		return 0;
| +
| +	((struct gfs2_rgrp *)buf)->rg_crc = 0;
| +	if (crc != gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp)))
| +		ret = 1;
| +	((struct gfs2_rgrp *)buf)->rg_crc = cpu_to_be32(crc);

I didn't trace the code all the way back, so I might be wrong. But:

My concern here is that lgfs2_rgrp_crc_check can modify the contents
of the buffer here. That is exactly what you want in most cases, but
it might be a problem for fsck.gfs2 cases other than -y. For example,
The -n option instructs fsck to not make any changes, and without -y,
it's supposed to ask before making any changes. So hopefully this
buffer is never rewritten without permission, at some other point in
fsck. It's worth checking. It also may be worth testing to make sure
specifying -n detects crc errors, reports them, but does not fix them
with -n.

(snip)
| +/**
| + * Set the crc of an on-disk resource group
| + */
| +void lgfs2_rgrp_crc_set(char *buf)
| +{
| +#ifdef GFS2_HAS_RG_RI_FIELDS
| +	struct gfs2_rgrp *rg = (struct gfs2_rgrp *)buf;
| +	uint32_t crc;
| +
| +	rg->rg_crc = 0;
| +	crc = gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp));
| +	rg->rg_crc = cpu_to_be32(crc);
| +#endif
| +}

You may want to move the initialization of rg_crc (and supporting
code, obviously) outside the #ifdef so it gets initialized to 0.

The rest looked good.

Bob Peterson

[Cluster-devel] [PATCH 8/8] libgfs2: Add support for rg_crc

[Andrew Price]

On 07/12/17 13:53, Bob Peterson wrote:
> Hi,
> 
> ----- Original Message -----
> | This new field in the resource group headers is set on write and checked
> | on read so that fsck.gfs2 now considers a resource group with a bad,
> | non-zero crc to be corrupted. gfs2_grow, gfs2_edit, and mkfs.gfs2 all
> | now support the rg_crc field, including save/restoremeta.
> |
> | Signed-off-by: Andrew Price <anprice@redhat.com>
> | ---
> (snip)
> | diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
> | index 17178bd9..03861d14 100644
> | --- a/gfs2/libgfs2/rgrp.c
> | +++ b/gfs2/libgfs2/rgrp.c
> | @@ -158,6 +158,42 @@ void lgfs2_rgrp_bitbuf_free(lgfs2_rgrp_t rg)
> |  }
> |
> |  /**
> | + * Check a resource group's crc
> | + * Returns 0 on success, non-zero if crc is bad
> | + */
> | +int lgfs2_rgrp_crc_check(const struct gfs2_rgrp *rg, char *buf)
> | +{
> | +	int ret = 0;
> | +#ifdef GFS2_HAS_RG_RI_FIELDS
> | +	uint32_t crc = rg->rg_crc;
> | +
> | +	if (crc == 0)
> | +		return 0;
> | +
> | +	((struct gfs2_rgrp *)buf)->rg_crc = 0;
> | +	if (crc != gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp)))
> | +		ret = 1;
> | +	((struct gfs2_rgrp *)buf)->rg_crc = cpu_to_be32(crc);
> 
> I didn't trace the code all the way back, so I might be wrong. But:
> 
> My concern here is that lgfs2_rgrp_crc_check can modify the contents
> of the buffer here. That is exactly what you want in most cases, but
> it might be a problem for fsck.gfs2 cases other than -y. For example,
> The -n option instructs fsck to not make any changes, and without -y,
> it's supposed to ask before making any changes. So hopefully this
> buffer is never rewritten without permission, at some other point in
> fsck. It's worth checking. It also may be worth testing to make sure
> specifying -n detects crc errors, reports them, but does not fix them
> with -n.

So fsck.gfs2 opens the device read-only with -n but I agree with your 
core concern. Since we can't be sure that the contents of rg and buf 
mirror each other, it would be better to operate on just the buffer and 
make sure the original value is restored once the check is done. I've 
updated it to:

int lgfs2_rgrp_crc_check(char *buf)
{
         int ret = 0;
#ifdef GFS2_HAS_RG_RI_FIELDS
         struct gfs2_rgrp *rg = (struct gfs2_rgrp *)buf;
         uint32_t crc = rg->rg_crc;

         if (crc == 0)
                 return 0;

         rg->rg_crc = 0;
         if (be32_to_cpu(crc) != gfs2_disk_hash(buf, sizeof(struct 
gfs2_rgrp)))
                 ret = 1;
         rg->rg_crc = crc;
#endif
         return ret;
}

> 
> (snip)
> | +/**
> | + * Set the crc of an on-disk resource group
> | + */
> | +void lgfs2_rgrp_crc_set(char *buf)
> | +{
> | +#ifdef GFS2_HAS_RG_RI_FIELDS
> | +	struct gfs2_rgrp *rg = (struct gfs2_rgrp *)buf;
> | +	uint32_t crc;
> | +
> | +	rg->rg_crc = 0;
> | +	crc = gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp));
> | +	rg->rg_crc = cpu_to_be32(crc);
> | +#endif
> | +}
> 
> You may want to move the initialization of rg_crc (and supporting
> code, obviously) outside the #ifdef so it gets initialized to 0.

rg_crc doesn't exist outside of the #ifdef and that rg_reserved chunk 
gets zeroed in libgfs2 so this should be ok too.

Thanks for your reviews, I'll repost the updated patches after some 
extra testing.

Cheers,
Andy