[Cluster-devel] [PATCH 0/2] gfs1 convert/fsck fixes

[Cluster-devel] [PATCH 0/2] gfs1 convert/fsck fixes

[Abhi Das]

These two patches fix bugs identified during recent convert/fsck
tests on gfs1 filesystems.

Abhi Das (2):
  libgfs2: patch to update gfs1 superblock correctly
  gfs2-utils: check and fix bad dinode pointers in gfs1 sb

 gfs2/convert/gfs2_convert.c | 29 ++++++++++++++++++
 gfs2/fsck/initialize.c      | 72 +++++++++++++++++++++++++++++++++++++++++++++
 gfs2/libgfs2/ondisk.c       |  5 ++++
 3 files changed, 106 insertions(+)

-- 
1.8.1.4

[Cluster-devel] [PATCH 1/2] libgfs2: patch to update gfs1 superblock correctly

[Abhi Das]

gfs2-utils uses the gfs2_sb structure as a template to operate on
both gfs1 and gfs2 superblocks. Some fields of the gfs1 superblock
are not in gfs2 and are replaced by padded fields. These padded
fields were not being written out after modification and were
instead being zeroed out, thereby causing corruption.
This patch fixes gfs2_sb_out() to write out the padded fields.

Resolves: rhbz#1053668
Signed-off-by: Abhi Das <adas@redhat.com>
---
 gfs2/libgfs2/ondisk.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
index dcc537e..3fbc63f 100644
--- a/gfs2/libgfs2/ondisk.c
+++ b/gfs2/libgfs2/ondisk.c
@@ -132,15 +132,20 @@ void gfs2_sb_out(const struct gfs2_sb *sb, char *buf)
 
 	CPOUT_32(sb, str, sb_fs_format);
 	CPOUT_32(sb, str, sb_multihost_format);
+	CPOUT_32(sb, str, __pad0);                        /* gfs sb_flags */
 
 	CPOUT_32(sb, str, sb_bsize);
 	CPOUT_32(sb, str, sb_bsize_shift);
+	CPOUT_32(sb, str, __pad1);                        /* gfs sb_seg_size */
 
 	gfs2_inum_out(&sb->sb_master_dir, (char *)&str->sb_master_dir);
 	gfs2_inum_out(&sb->sb_root_dir, (char *)&str->sb_root_dir);
 
 	CPOUT_08(sb, str, sb_lockproto, GFS2_LOCKNAME_LEN);
 	CPOUT_08(sb, str, sb_locktable, GFS2_LOCKNAME_LEN);
+	gfs2_inum_out(&sb->__pad2, (char *)&str->__pad2); /* gfs rindex */
+	gfs2_inum_out(&sb->__pad3, (char *)&str->__pad3); /* gfs quota */
+	gfs2_inum_out(&sb->__pad4, (char *)&str->__pad4); /* gfs license */
 #ifdef GFS2_HAS_UUID
 	memcpy(str->sb_uuid, sb->sb_uuid, 16);
 #endif
-- 
1.8.1.4

[Cluster-devel] [PATCH 1/2] libgfs2: patch to update gfs1 superblock correctly

[Andrew Price]

Hi Abhi,

On 12/03/14 04:14, Abhi Das wrote:
> gfs2-utils uses the gfs2_sb structure as a template to operate on
> both gfs1 and gfs2 superblocks. Some fields of the gfs1 superblock
> are not in gfs2 and are replaced by padded fields. These padded
> fields were not being written out after modification and were
> instead being zeroed out, thereby causing corruption.
> This patch fixes gfs2_sb_out() to write out the padded fields.
>
> Resolves: rhbz#1053668
> Signed-off-by: Abhi Das <adas@redhat.com>

I'll defer to Bob's comments on patch #2 but this one looks good, thanks.

Cheers,
Andy

> ---
>   gfs2/libgfs2/ondisk.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
> index dcc537e..3fbc63f 100644
> --- a/gfs2/libgfs2/ondisk.c
> +++ b/gfs2/libgfs2/ondisk.c
> @@ -132,15 +132,20 @@ void gfs2_sb_out(const struct gfs2_sb *sb, char *buf)
>
>   	CPOUT_32(sb, str, sb_fs_format);
>   	CPOUT_32(sb, str, sb_multihost_format);
> +	CPOUT_32(sb, str, __pad0);                        /* gfs sb_flags */
>
>   	CPOUT_32(sb, str, sb_bsize);
>   	CPOUT_32(sb, str, sb_bsize_shift);
> +	CPOUT_32(sb, str, __pad1);                        /* gfs sb_seg_size */
>
>   	gfs2_inum_out(&sb->sb_master_dir, (char *)&str->sb_master_dir);
>   	gfs2_inum_out(&sb->sb_root_dir, (char *)&str->sb_root_dir);
>
>   	CPOUT_08(sb, str, sb_lockproto, GFS2_LOCKNAME_LEN);
>   	CPOUT_08(sb, str, sb_locktable, GFS2_LOCKNAME_LEN);
> +	gfs2_inum_out(&sb->__pad2, (char *)&str->__pad2); /* gfs rindex */
> +	gfs2_inum_out(&sb->__pad3, (char *)&str->__pad3); /* gfs quota */
> +	gfs2_inum_out(&sb->__pad4, (char *)&str->__pad4); /* gfs license */
>   #ifdef GFS2_HAS_UUID
>   	memcpy(str->sb_uuid, sb->sb_uuid, 16);
>   #endif
>

[Cluster-devel] [PATCH 2/2] gfs2-utils: check and fix bad dinode pointers in gfs1 sb

[Abhi Das]

This patch makes gfs2_convert check for bad values of sb_seg_size,
sb_quota_di and sb_license_di.

In fsck.gfs2, attempts are made to correct these erroneous values.

Resolves: rhbz#1053668
Signed-off-by: Abhi Das <adas@redhat.com>
---
 gfs2/convert/gfs2_convert.c | 29 ++++++++++++++++++
 gfs2/fsck/initialize.c      | 72 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index 7a7d4df..9f7fa17 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -1502,6 +1502,24 @@ static int read_gfs1_jiindex(struct gfs2_sbd *sdp)
 	return -1;
 }
 
+static int sanity_check(struct gfs2_sbd *sdp)
+{
+	int error = 0;
+	if (!raw_gfs1_ondisk_sb.sb_quota_di.no_addr) {
+		log_crit(_("Error: Superblock Quota inode address is NULL\n"));
+		error = 1;
+	}
+	if (!raw_gfs1_ondisk_sb.sb_license_di.no_addr) {
+		log_crit(_("Error: Superblock Statfs inode address is NULL\n"));
+		error = 1;
+	}
+	if (!raw_gfs1_ondisk_sb.sb_seg_size) {
+		log_crit(_("Error: Superblock segment size is zero\n"));
+		error = 1;
+	}
+	return error;
+}
+
 /* ------------------------------------------------------------------------- */
 /* init - initialization code                                                */
 /* Returns: 0 on success, -1 on failure                                      */
@@ -2152,6 +2170,17 @@ int main(int argc, char **argv)
 	process_parameters(argc, argv, &opts);
 	error = init(&sb2);
 
+	/*
+	 * Check for some common fs errors
+	 */
+	if (!error) {
+		if (sanity_check(&sb2)) {
+			log_crit(_("%s is not a clean gfs filesytem. Please use the"
+				   " fsck.gfs2 utility to correct these errors and"
+				   " try again.\n"), device);
+			exit(0);
+		}
+	}
 	/* ---------------------------------------------- */
 	/* Make them seal their fate.                     */
 	/* ---------------------------------------------- */
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 0f33aa6..24e5de2 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -727,6 +727,13 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	}
 
 	if (sdp->gfs1) {
+		/* In gfs1, the license_di is always 3 blocks after the jindex_di */
+		if (sbd1->sb_license_di.no_addr != sbd1->sb_jindex_di.no_addr + 3) {
+			sbd1->sb_license_di.no_addr = sbd1->sb_license_di.no_formal_ino
+				= sbd1->sb_jindex_di.no_addr + 3;
+			log_err(_("Reset statfs inode block address to: %llu\n"),
+				sbd1->sb_license_di.no_addr);
+		}
 		sdp->md.statfs = lgfs2_inode_read(sdp, sbd1->sb_license_di.no_addr);
 		if (sdp->md.statfs == NULL) {
 			log_crit(_("Error reading statfs inode: %s\n"), strerror(errno));
@@ -773,6 +780,14 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	}
 
 	if (sdp->gfs1) {
+		/* In gfs1, the quota_di is always 2 blocks after the jindex_di */
+		if (sbd1->sb_quota_di.no_addr != sbd1->sb_jindex_di.no_addr + 2) {
+			sbd1->sb_quota_di.no_addr = sbd1->sb_quota_di.no_formal_ino
+				= sbd1->sb_jindex_di.no_addr + 2;
+			log_err(_("Reset quota inode block address to: %llu\n"),
+				sbd1->sb_quota_di.no_addr);
+		}
+
 		sdp->md.qinode = lgfs2_inode_read(sdp, sbd1->sb_quota_di.no_addr);
 		if (sdp->md.qinode == NULL) {
 			log_crit(_("Error reading quota inode: %s\n"), strerror(errno));
@@ -1354,6 +1369,57 @@ static int reconstruct_single_journal(struct gfs2_sbd *sdp, int jnum,
 	return 0;
 }
 
+static int reset_journal_seg_size(unsigned int jsize, unsigned int nsegs,
+					     unsigned int bsize)
+{
+	unsigned int seg_size = jsize / (nsegs * bsize);
+	if (!seg_size)
+		seg_size = 16; /* The default with 128MB journal and 4K bsize */
+	if (seg_size != sbd1->sb_seg_size) {
+		sbd1->sb_seg_size = seg_size;
+		if (!query(_("Computed correct journal segment size to %u."
+			     " Reset it? (y/n) "), seg_size)) {
+			log_crit(_("Error: Cannot proceed without a valid journal"
+				   " segment size value.\n"));
+			return -1;
+		}
+		log_err(_("Resetting journal segment size to %u\n"), sbd1->sb_seg_size);
+	}
+	return 0;
+}
+
+static int correct_journal_seg_size(struct gfs2_sbd *sdp)
+{
+	int count;
+	struct gfs_jindex ji_0, ji_1;
+	char buf[sizeof(struct gfs_jindex)];
+	unsigned int jsize = GFS2_DEFAULT_JSIZE * 1024 * 1024;
+
+	count = gfs2_readi(sdp->md.jiinode, buf, 0, sizeof(struct gfs_jindex));
+	if (count != sizeof(struct gfs_jindex))
+		return -1;
+	gfs_jindex_in(&ji_0, buf);
+
+	if (sdp->md.journals == 1 && sbd1->sb_seg_size == 0) {
+		if (!query(_("The gfs2 journal segment size is 0 and a correct value\n"
+			     "cannot be determined in a single-journal filesystem.\n"
+			     "Continue with default? (y/n) "))) {
+			log_crit(_("Error: Cannot proceed without a valid sb_seg_size value.\n"));
+			return -1;
+		}
+		goto out;
+	}
+
+	count = gfs2_readi(sdp->md.jiinode, buf, sizeof(struct gfs_jindex),
+			   sizeof(struct gfs_jindex));
+	if (count != sizeof(struct gfs_jindex))
+		return -1;
+	gfs_jindex_in(&ji_1, buf);
+
+	jsize = (ji_1.ji_addr - ji_0.ji_addr) * sbd1->sb_bsize;
+out:
+	return reset_journal_seg_size(jsize, ji_0.ji_nsegment, sbd1->sb_bsize);
+}
 
 /*
  * reconstruct_journals - write fresh journals for GFS1 only
@@ -1367,6 +1433,12 @@ static int reconstruct_journals(struct gfs2_sbd *sdp)
 	struct gfs_jindex ji;
 	char buf[sizeof(struct gfs_jindex)];
 
+	/* Ensure that sb_seg_size is valid */
+	if (correct_journal_seg_size(sdp)) {
+		log_crit(_("Failed to set correct journal segment size. Cannot continue\n"));
+		return -1;
+	}
+
 	log_err(_("Clearing GFS journals (this may take a while)\n"));
 	for (i = 0; i < sdp->md.journals; i++) {
 		count = gfs2_readi(sdp->md.jiinode, buf,
-- 
1.8.1.4

[Cluster-devel] [PATCH 2/2] gfs2-utils: check and fix bad dinode pointers in gfs1 sb

[Bob Peterson]

----- Original Message -----
| This patch makes gfs2_convert check for bad values of sb_seg_size,
| sb_quota_di and sb_license_di.
| 
| In fsck.gfs2, attempts are made to correct these erroneous values.
| 
| Resolves: rhbz#1053668
| Signed-off-by: Abhi Das <adas@redhat.com>
(snip)
| diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
| index 0f33aa6..24e5de2 100644
| --- a/gfs2/fsck/initialize.c
| +++ b/gfs2/fsck/initialize.c
| @@ -727,6 +727,13 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
|  	}
|  
|  	if (sdp->gfs1) {
| +		/* In gfs1, the license_di is always 3 blocks after the jindex_di */
| +		if (sbd1->sb_license_di.no_addr != sbd1->sb_jindex_di.no_addr + 3) {
| +			sbd1->sb_license_di.no_addr = sbd1->sb_license_di.no_formal_ino
| +				= sbd1->sb_jindex_di.no_addr + 3;
| +			log_err(_("Reset statfs inode block address to: %llu\n"),
| +				sbd1->sb_license_di.no_addr);
| +		}

This resolves the problem in memory, but doesn't fix it on disk.
We should probably fix it on disk and also use function query() to
get their permission. Same for the quota file.

| +static int reset_journal_seg_size(unsigned int jsize, unsigned int nsegs,
| +					     unsigned int bsize)
| +{
| +	unsigned int seg_size = jsize / (nsegs * bsize);
| +	if (!seg_size)
| +		seg_size = 16; /* The default with 128MB journal and 4K bsize */
| +	if (seg_size != sbd1->sb_seg_size) {
| +		sbd1->sb_seg_size = seg_size;
| +		if (!query(_("Computed correct journal segment size to %u."
| +			     " Reset it? (y/n) "), seg_size)) {
| +			log_crit(_("Error: Cannot proceed without a valid journal"
| +				   " segment size value.\n"));
| +			return -1;
| +		}
| +		log_err(_("Resetting journal segment size to %u\n"), sbd1->sb_seg_size);

Is the superblock being rewritten with the new value anywhere?
If the value is only used for fsck purposes, then no problem. If
there are places in gfs2_convert that may reference it (possibly
through libgfs2) then we should fix it on disk as well.

| +static int correct_journal_seg_size(struct gfs2_sbd *sdp)
| +{
| +	int count;
| +	struct gfs_jindex ji_0, ji_1;
| +	char buf[sizeof(struct gfs_jindex)];
| +	unsigned int jsize = GFS2_DEFAULT_JSIZE * 1024 * 1024;
| +
| +	count = gfs2_readi(sdp->md.jiinode, buf, 0, sizeof(struct gfs_jindex));
| +	if (count != sizeof(struct gfs_jindex))
| +		return -1;

We should probably log an error message here.

| +	return reset_journal_seg_size(jsize, ji_0.ji_nsegment, sbd1->sb_bsize);

ALso, make sure to test with a clean file system and "-n" to make sure it
doesn't try to make changes. I think it's okay, but I always check it
(I've been burned too many times in the past!).

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH 2/2] gfs2-utils: check and fix bad dinode pointers in gfs1 sb

[Abhijith Das]

Hi Bob,

Thanks for the review.

> ----- Original Message -----
> | This patch makes gfs2_convert check for bad values of sb_seg_size,
> | sb_quota_di and sb_license_di.
> | 
> | In fsck.gfs2, attempts are made to correct these erroneous values.
> | 
> | Resolves: rhbz#1053668
> | Signed-off-by: Abhi Das <adas@redhat.com>
> (snip)
> | diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> | index 0f33aa6..24e5de2 100644
> | --- a/gfs2/fsck/initialize.c
> | +++ b/gfs2/fsck/initialize.c
> | @@ -727,6 +727,13 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
> |  	}
> |  
> |  	if (sdp->gfs1) {
> | +		/* In gfs1, the license_di is always 3 blocks after the jindex_di */
> | +		if (sbd1->sb_license_di.no_addr != sbd1->sb_jindex_di.no_addr + 3) {
> | +			sbd1->sb_license_di.no_addr = sbd1->sb_license_di.no_formal_ino
> | +				= sbd1->sb_jindex_di.no_addr + 3;
> | +			log_err(_("Reset statfs inode block address to: %llu\n"),
> | +				sbd1->sb_license_di.no_addr);
> | +		}
> 
> This resolves the problem in memory, but doesn't fix it on disk.
> We should probably fix it on disk and also use function query() to
> get their permission. Same for the quota file.

I took a quick look at the code and I only found lgfs2_sb_write() being called in
block_mounters()... which happens before init_system_inodes. But I could've sworn
the changes propagated to disk. I remember seeing it with gfs2_edit after the fsck.

I'll look into this and also add the permission check before changing the value.

> 
> | +static int reset_journal_seg_size(unsigned int jsize, unsigned int nsegs,
> | +					     unsigned int bsize)
> | +{
> | +	unsigned int seg_size = jsize / (nsegs * bsize);
> | +	if (!seg_size)
> | +		seg_size = 16; /* The default with 128MB journal and 4K bsize */
> | +	if (seg_size != sbd1->sb_seg_size) {
> | +		sbd1->sb_seg_size = seg_size;
> | +		if (!query(_("Computed correct journal segment size to %u."
> | +			     " Reset it? (y/n) "), seg_size)) {
> | +			log_crit(_("Error: Cannot proceed without a valid journal"
> | +				   " segment size value.\n"));
> | +			return -1;
> | +		}
> | +		log_err(_("Resetting journal segment size to %u\n"), sbd1->sb_seg_size);
> 
> Is the superblock being rewritten with the new value anywhere?
> If the value is only used for fsck purposes, then no problem. If
> there are places in gfs2_convert that may reference it (possibly
> through libgfs2) then we should fix it on disk as well.

Same thing here. I'll take a look.

> 
> | +static int correct_journal_seg_size(struct gfs2_sbd *sdp)
> | +{
> | +	int count;
> | +	struct gfs_jindex ji_0, ji_1;
> | +	char buf[sizeof(struct gfs_jindex)];
> | +	unsigned int jsize = GFS2_DEFAULT_JSIZE * 1024 * 1024;
> | +
> | +	count = gfs2_readi(sdp->md.jiinode, buf, 0, sizeof(struct gfs_jindex));
> | +	if (count != sizeof(struct gfs_jindex))
> | +		return -1;
> 
> We should probably log an error message here.
> 
> | +	return reset_journal_seg_size(jsize, ji_0.ji_nsegment, sbd1->sb_bsize);
> 
> ALso, make sure to test with a clean file system and "-n" to make sure it
> doesn't try to make changes. I think it's okay, but I always check it
> (I've been burned too many times in the past!).
> 

Yeah. I'll do that.

Cheers!
--Abhi
> Regards,
> 
> Bob Peterson
> Red Hat File Systems
> 

[Cluster-devel] [PATCHv2 2/2] gfs2-utils: check and fix bad dinode pointers in gfs1 sb

[Abhijith Das]

This patch makes gfs2_convert check for bad values of sb_seg_size,
sb_quota_di and sb_license_di.

In fsck.gfs2, attempts are made to correct these erroneous values.
This v2 patch checks with the user before making changes to the
superblock in fsck.gfs2 and also adds some error messaging as per
Bob's comments.

Resolves: rhbz#1053668
Signed-off-by: Abhi Das <adas@redhat.com>

diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index 7a7d4df..9f7fa17 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -1502,6 +1502,24 @@ static int read_gfs1_jiindex(struct gfs2_sbd *sdp)
 	return -1;
 }
 
+static int sanity_check(struct gfs2_sbd *sdp)
+{
+	int error = 0;
+	if (!raw_gfs1_ondisk_sb.sb_quota_di.no_addr) {
+		log_crit(_("Error: Superblock Quota inode address is NULL\n"));
+		error = 1;
+	}
+	if (!raw_gfs1_ondisk_sb.sb_license_di.no_addr) {
+		log_crit(_("Error: Superblock Statfs inode address is NULL\n"));
+		error = 1;
+	}
+	if (!raw_gfs1_ondisk_sb.sb_seg_size) {
+		log_crit(_("Error: Superblock segment size is zero\n"));
+		error = 1;
+	}
+	return error;
+}
+
 /* ------------------------------------------------------------------------- */
 /* init - initialization code                                                */
 /* Returns: 0 on success, -1 on failure                                      */
@@ -2152,6 +2170,17 @@ int main(int argc, char **argv)
 	process_parameters(argc, argv, &opts);
 	error = init(&sb2);
 
+	/*
+	 * Check for some common fs errors
+	 */
+	if (!error) {
+		if (sanity_check(&sb2)) {
+			log_crit(_("%s is not a clean gfs filesytem. Please use the"
+				   " fsck.gfs2 utility to correct these errors and"
+				   " try again.\n"), device);
+			exit(0);
+		}
+	}
 	/* ---------------------------------------------- */
 	/* Make them seal their fate.                     */
 	/* ---------------------------------------------- */
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 0f33aa6..33cf85b 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -727,6 +727,19 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	}
 
 	if (sdp->gfs1) {
+		/* In gfs1, the license_di is always 3 blocks after the jindex_di */
+		if ((sbd1->sb_license_di.no_addr != sbd1->sb_jindex_di.no_addr + 3) ||
+		    (sbd1->sb_license_di.no_formal_ino != sbd1->sb_jindex_di.no_addr + 3)) {
+			if (!query( _("The gfs system statfs inode pointer is incorrect. "
+				      "Okay to correct? (y/n) "))) {
+				log_err( _("fsck.gfs2 cannot continue without a valid "
+					   "statfs file; aborting.\n"));
+				goto fail;
+			}
+			sbd1->sb_license_di.no_addr = sbd1->sb_license_di.no_formal_ino
+				= sbd1->sb_jindex_di.no_addr + 3;
+		}
+
 		sdp->md.statfs = lgfs2_inode_read(sdp, sbd1->sb_license_di.no_addr);
 		if (sdp->md.statfs == NULL) {
 			log_crit(_("Error reading statfs inode: %s\n"), strerror(errno));
@@ -773,6 +786,19 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	}
 
 	if (sdp->gfs1) {
+		/* In gfs1, the quota_di is always 2 blocks after the jindex_di */
+		if ((sbd1->sb_quota_di.no_addr != sbd1->sb_jindex_di.no_addr + 2) ||
+		    (sbd1->sb_quota_di.no_formal_ino != sbd1->sb_jindex_di.no_addr + 2)) {
+			if (!query( _("The gfs system quota inode pointer is incorrect. "
+				      " Okay to correct? (y/n) "))) {
+				log_err( _("fsck.gfs2 cannot continue without a valid "
+					   "quota file; aborting.\n"));
+				goto fail;
+			}
+			sbd1->sb_quota_di.no_addr = sbd1->sb_quota_di.no_formal_ino
+				= sbd1->sb_jindex_di.no_addr + 2;
+		}
+
 		sdp->md.qinode = lgfs2_inode_read(sdp, sbd1->sb_quota_di.no_addr);
 		if (sdp->md.qinode == NULL) {
 			log_crit(_("Error reading quota inode: %s\n"), strerror(errno));
@@ -1354,6 +1380,63 @@ static int reconstruct_single_journal(struct gfs2_sbd *sdp, int jnum,
 	return 0;
 }
 
+static int reset_journal_seg_size(unsigned int jsize, unsigned int nsegs,
+					     unsigned int bsize)
+{
+	unsigned int seg_size = jsize / (nsegs * bsize);
+	if (!seg_size)
+		seg_size = 16; /* The default with 128MB journal and 4K bsize */
+	if (seg_size != sbd1->sb_seg_size) {
+		sbd1->sb_seg_size = seg_size;
+		if (!query(_("Computed correct journal segment size to %u."
+			     " Reset it? (y/n) "), seg_size)) {
+			log_crit(_("Error: Cannot proceed without a valid journal"
+				   " segment size value.\n"));
+			return -1;
+		}
+		log_err(_("Resetting journal segment size to %u\n"), sbd1->sb_seg_size);
+	}
+	return 0;
+}
+
+static int correct_journal_seg_size(struct gfs2_sbd *sdp)
+{
+	int count;
+	struct gfs_jindex ji_0, ji_1;
+	char buf[sizeof(struct gfs_jindex)];
+	unsigned int jsize = GFS2_DEFAULT_JSIZE * 1024 * 1024;
+
+	count = gfs2_readi(sdp->md.jiinode, buf, 0, sizeof(struct gfs_jindex));
+	if (count != sizeof(struct gfs_jindex)) {
+		log_crit(_("Error %d reading system journal index inode. "
+			   "Aborting\n"), count);
+		return -1;
+	}
+	gfs_jindex_in(&ji_0, buf);
+
+	if (sdp->md.journals == 1 && sbd1->sb_seg_size == 0) {
+		if (!query(_("The gfs2 journal segment size is 0 and a correct value\n"
+			     "cannot be determined in a single-journal filesystem.\n"
+			     "Continue with default? (y/n) "))) {
+			log_crit(_("Error: Cannot proceed without a valid sb_seg_size value.\n"));
+			return -1;
+		}
+		goto out;
+	}
+
+	count = gfs2_readi(sdp->md.jiinode, buf, sizeof(struct gfs_jindex),
+			   sizeof(struct gfs_jindex));
+	if (count != sizeof(struct gfs_jindex)) {
+		log_crit(_("Error %d reading system journal index inode. "
+			   "Aborting\n"), count);
+		return -1;
+	}
+	gfs_jindex_in(&ji_1, buf);
+
+	jsize = (ji_1.ji_addr - ji_0.ji_addr) * sbd1->sb_bsize;
+out:
+	return reset_journal_seg_size(jsize, ji_0.ji_nsegment, sbd1->sb_bsize);
+}
 
 /*
  * reconstruct_journals - write fresh journals for GFS1 only
@@ -1367,6 +1450,12 @@ static int reconstruct_journals(struct gfs2_sbd *sdp)
 	struct gfs_jindex ji;
 	char buf[sizeof(struct gfs_jindex)];
 
+	/* Ensure that sb_seg_size is valid */
+	if (correct_journal_seg_size(sdp)) {
+		log_crit(_("Failed to set correct journal segment size. Cannot continue\n"));
+		return -1;
+	}
+
 	log_err(_("Clearing GFS journals (this may take a while)\n"));
 	for (i = 0; i < sdp->md.journals; i++) {
 		count = gfs2_readi(sdp->md.jiinode, buf,