Skip to contentSkip to conversation

xfs_repair: allow filesystems with a single allocation group

PATCH In review Christoph Hellwig 3 participants 4 messages Jan 5, 2009 – Jan 20, 2009

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs_repair: allow filesystems with a single allocation group

Currently xfs_repair bails out on a filesystem with just a single AG. But that's a perfectly valid configureation, so we should allow it.

Because we could cause harm only allow it when using the force_geometry suboption. But we nice enough to tell the user about it when he needs it.

Also make sure to take the internal log into account when guestimating the first inode cluster offset.

I'll also cook up a testcase for repair on single AG filesystems.

repair/sb.c +19 -8
Index: xfsprogs-dev/repair/sb.c
===================================================================
@@ -760,27 +760,37 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 	switch (num_sbs)  {
 	case 2:
 		/*
-		 * all them have to be right.  if not, report geometry
-		 * and get out unless force option is in effect (-F)
+		 * If we only have two allocation groups, and the superblock
+		 * in the second allocation group differs from the primary
+		 * superblock we can't verify the geometry information.
+		 * Warn the user about this situation and get out unless
+		 * explicitly overridden.
 		 */
 		if (current->refs != 2)  {
 			if (!force_geo)  {
 				do_warn(
-	_("Only two AGs detected and they do not match - cannot proceed.\n"));
+	_("Only two AGs detected and they do not match - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 				exit(1);
 			}
 		}
-		break;
+		goto out_free_list;
 	case 1:
 		/*
-		 * just report the geometry info and get out.
-		 * refuse to run further unless the force (-F)
-		 * option is in effect.
+		 * If we only have a single allocation group there is no
+		 * secondary superblock that we can use to verify the geometry
+		 * information.  Warn the user about this situation and get
+		 * out unless explicitly overridden.
 		 */
 		if (!force_geo)  {
-			do_warn(_("Only one AG detected - cannot proceed.\n"));
+			do_warn(
+	_("Only one AG detected - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 			exit(1);
 		}
+		goto out_free_list;
 	default:
 		/*
 		 * at least half of the probed superblocks have
@@ -820,6 +830,7 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 		sb_width = sb->sb_width;
 	}
 
+out_free_list:
 	free_geo(list);
 out:
 	free(sb);
repair/xfs_repair.c +22 -0
Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================
@@ -62,6 +62,8 @@ char *o_opts[] = {
 	"bhash",
 #define	AG_STRIDE	4
 	"ag_stride",
+#define FORCE_GEO	5
+	"force_geometry",
 	NULL
 };
 
@@ -258,6 +260,13 @@ process_args(int argc, char **argv)
 				case AG_STRIDE:
 					ag_stride = (int)strtol(val, NULL, 0);
 					break;
+				case FORCE_GEO:
+					if (val)
+						noval('o', o_opts, FORCE_GEO);
+					if (force_geo)
+						respec('o', o_opts, FORCE_GEO);
+					force_geo = 1;
+					break;
 				default:
 					unknown('o', val);
 					break;
@@ -409,6 +418,19 @@ calc_mkfs(xfs_mount_t *mp)
 	fino_bno = inobt_root + XFS_MIN_FREELIST_RAW(1, 1, mp) + 1;
 
 	/*
+	 * If we only have a single allocation group the log is also allocated
+	 * in the first allocation group and we need to add the number of
+	 * blocks used by the log to the above calculation.
+	 * All this of course doesn't apply if we have an external log.
+	 */
+	if (mp->m_sb.sb_agcount == 1 && mp->m_sb.sb_logstart) {
+		/*
+		 * XXX(hch): verify that sb_logstart makes sense?
+		 */
+		 fino_bno += mp->m_sb.sb_logblocks;
+	}
+
+	/*
 	 * ditto the location of the first inode chunks in the fs ('/')
 	 */
 	if (xfs_sb_version_hasdalign(&mp->m_sb) && do_inoalign)  {
man/man8/xfs_repair.8 +9 -0
Index: xfsprogs-dev/man/man8/xfs_repair.8
===================================================================
@@ -146,6 +146,15 @@ RAM size.
 This creates additional processing threads to parallel process
 AGs that span multiple concat units. This can significantly
 reduce repair times on concat based filesystems.
+.TP
+.BI force_geometry
+Check the filesystem even if geometry information could not be validate.
+Geometry information can not be validated if there is only a single
+allocation group exists and thus no backups superblock exists, or
+if there are two allocation groups and the two superblocks do not
+agree on the filesystem geometry.  Only use this option if you validated
+the geometry yourself and know what your doing.  In doubt do a run
+in no modify mode first.
 .RE
 .TP
 .B \-t " interval"

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw

Lachlan, Eric,

does this version look okay now?

Quoted 141 lines · Christoph Hellwig

Currently xfs_repair bails out on a filesystem with just a single AG. But that's a perfectly valid configureation, so we should allow it.

Because we could cause harm only allow it when using the force_geometry suboption. But we nice enough to tell the user about it when he needs it.

Also make sure to take the internal log into account when guestimating the first inode cluster offset.

I'll also cook up a testcase for repair on single AG filesystems.

Index: xfsprogs-dev/repair/sb.c
===================================================================

repair/sb.c +19 -8
@@ -760,27 +760,37 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 	switch (num_sbs)  {
 	case 2:
 		/*
-		 * all them have to be right.  if not, report geometry
-		 * and get out unless force option is in effect (-F)
+		 * If we only have two allocation groups, and the superblock
+		 * in the second allocation group differs from the primary
+		 * superblock we can't verify the geometry information.
+		 * Warn the user about this situation and get out unless
+		 * explicitly overridden.
 		 */
 		if (current->refs != 2)  {
 			if (!force_geo)  {
 				do_warn(
-	_("Only two AGs detected and they do not match - cannot proceed.\n"));
+	_("Only two AGs detected and they do not match - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 				exit(1);
 			}
 		}
-		break;
+		goto out_free_list;
 	case 1:
 		/*
-		 * just report the geometry info and get out.
-		 * refuse to run further unless the force (-F)
-		 * option is in effect.
+		 * If we only have a single allocation group there is no
+		 * secondary superblock that we can use to verify the geometry
+		 * information.  Warn the user about this situation and get
+		 * out unless explicitly overridden.
 		 */
 		if (!force_geo)  {
-			do_warn(_("Only one AG detected - cannot proceed.\n"));
+			do_warn(
+	_("Only one AG detected - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 			exit(1);
 		}
+		goto out_free_list;
 	default:
 		/*
 		 * at least half of the probed superblocks have
@@ -820,6 +830,7 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 		sb_width = sb->sb_width;
 	}
 
+out_free_list:
 	free_geo(list);
 out:
 	free(sb);

Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================

repair/xfs_repair.c +22 -0
@@ -62,6 +62,8 @@ char *o_opts[] = {
 	"bhash",
 #define	AG_STRIDE	4
 	"ag_stride",
+#define FORCE_GEO	5
+	"force_geometry",
 	NULL
 };
 
@@ -258,6 +260,13 @@ process_args(int argc, char **argv)
 				case AG_STRIDE:
 					ag_stride = (int)strtol(val, NULL, 0);
 					break;
+				case FORCE_GEO:
+					if (val)
+						noval('o', o_opts, FORCE_GEO);
+					if (force_geo)
+						respec('o', o_opts, FORCE_GEO);
+					force_geo = 1;
+					break;
 				default:
 					unknown('o', val);
 					break;
@@ -409,6 +418,19 @@ calc_mkfs(xfs_mount_t *mp)
 	fino_bno = inobt_root + XFS_MIN_FREELIST_RAW(1, 1, mp) + 1;
 
 	/*
+	 * If we only have a single allocation group the log is also allocated
+	 * in the first allocation group and we need to add the number of
+	 * blocks used by the log to the above calculation.
+	 * All this of course doesn't apply if we have an external log.
+	 */
+	if (mp->m_sb.sb_agcount == 1 && mp->m_sb.sb_logstart) {
+		/*
+		 * XXX(hch): verify that sb_logstart makes sense?
+		 */
+		 fino_bno += mp->m_sb.sb_logblocks;
+	}
+
+	/*
 	 * ditto the location of the first inode chunks in the fs ('/')
 	 */
 	if (xfs_sb_version_hasdalign(&mp->m_sb) && do_inoalign)  {

Index: xfsprogs-dev/man/man8/xfs_repair.8
===================================================================

man/man8/xfs_repair.8 +9 -0
@@ -146,6 +146,15 @@ RAM size.
 This creates additional processing threads to parallel process
 AGs that span multiple concat units. This can significantly
 reduce repair times on concat based filesystems.
+.TP
+.BI force_geometry
+Check the filesystem even if geometry information could not be validate.
+Geometry information can not be validated if there is only a single
+allocation group exists and thus no backups superblock exists, or
+if there are two allocation groups and the two superblocks do not
+agree on the filesystem geometry.  Only use this option if you validated
+the geometry yourself and know what your doing.  In doubt do a run
+in no modify mode first.
 .RE
 .TP
 .B \-t " interval"
Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

---end quoted text---

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw
Quoted 3 lines · Christoph Hellwig

Lachlan, Eric,

does this version look okay now?

just minor edits on the manpage change, below

Quoted 124 lines · Christoph Hellwig
Quoted 123 lines

Currently xfs_repair bails out on a filesystem with just a single AG. But that's a perfectly valid configureation, so we should allow it.

Because we could cause harm only allow it when using the force_geometry suboption. But we nice enough to tell the user about it when he needs it.

Also make sure to take the internal log into account when guestimating the first inode cluster offset.

I'll also cook up a testcase for repair on single AG filesystems.

Index: xfsprogs-dev/repair/sb.c
===================================================================

repair/sb.c +19 -8
@@ -760,27 +760,37 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 	switch (num_sbs)  {
 	case 2:
 		/*
-		 * all them have to be right.  if not, report geometry
-		 * and get out unless force option is in effect (-F)
+		 * If we only have two allocation groups, and the superblock
+		 * in the second allocation group differs from the primary
+		 * superblock we can't verify the geometry information.
+		 * Warn the user about this situation and get out unless
+		 * explicitly overridden.
 		 */
 		if (current->refs != 2)  {
 			if (!force_geo)  {
 				do_warn(
-	_("Only two AGs detected and they do not match - cannot proceed.\n"));
+	_("Only two AGs detected and they do not match - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 				exit(1);
 			}
 		}
-		break;
+		goto out_free_list;
 	case 1:
 		/*
-		 * just report the geometry info and get out.
-		 * refuse to run further unless the force (-F)
-		 * option is in effect.
+		 * If we only have a single allocation group there is no
+		 * secondary superblock that we can use to verify the geometry
+		 * information.  Warn the user about this situation and get
+		 * out unless explicitly overridden.
 		 */
 		if (!force_geo)  {
-			do_warn(_("Only one AG detected - cannot proceed.\n"));
+			do_warn(
+	_("Only one AG detected - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 			exit(1);
 		}
+		goto out_free_list;
 	default:
 		/*
 		 * at least half of the probed superblocks have
@@ -820,6 +830,7 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 		sb_width = sb->sb_width;
 	}
 
+out_free_list:
 	free_geo(list);
 out:
 	free(sb);

Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================

repair/xfs_repair.c +22 -0
@@ -62,6 +62,8 @@ char *o_opts[] = {
 	"bhash",
 #define	AG_STRIDE	4
 	"ag_stride",
+#define FORCE_GEO	5
+	"force_geometry",
 	NULL
 };
 
@@ -258,6 +260,13 @@ process_args(int argc, char **argv)
 				case AG_STRIDE:
 					ag_stride = (int)strtol(val, NULL, 0);
 					break;
+				case FORCE_GEO:
+					if (val)
+						noval('o', o_opts, FORCE_GEO);
+					if (force_geo)
+						respec('o', o_opts, FORCE_GEO);
+					force_geo = 1;
+					break;
 				default:
 					unknown('o', val);
 					break;
@@ -409,6 +418,19 @@ calc_mkfs(xfs_mount_t *mp)
 	fino_bno = inobt_root + XFS_MIN_FREELIST_RAW(1, 1, mp) + 1;
 
 	/*
+	 * If we only have a single allocation group the log is also allocated
+	 * in the first allocation group and we need to add the number of
+	 * blocks used by the log to the above calculation.
+	 * All this of course doesn't apply if we have an external log.
+	 */
+	if (mp->m_sb.sb_agcount == 1 && mp->m_sb.sb_logstart) {
+		/*
+		 * XXX(hch): verify that sb_logstart makes sense?
+		 */
+		 fino_bno += mp->m_sb.sb_logblocks;
+	}
+
+	/*
 	 * ditto the location of the first inode chunks in the fs ('/')
 	 */
 	if (xfs_sb_version_hasdalign(&mp->m_sb) && do_inoalign)  {

Index: xfsprogs-dev/man/man8/xfs_repair.8
===================================================================
--- xfsprogs-dev.orig/man/man8/xfs_repair.8 2009-01-05 21:11:50.054546919 +0100

Quoted 8 lines

repair/xfs_repair.c RAM size. · line 151

Quoted 8 lines

+++ xfsprogs-dev/man/man8/xfs_repair.8 2009-01-05 21:14:49.228027165 +0100

repair/xfs_repair.c +3 -0
@@ -146,6 +146,15 @@ RAM size.
 This creates additional processing threads to parallel process
 AGs that span multiple concat units. This can significantly
 reduce repair times on concat based filesystems.
+.TP
+.BI force_geometry
+Check the filesystem even if geometry information could not be validate.

"validated."

Quoted 1 line

repair/xfs_repair.c RAM size. · line 152

Quoted 1 line
repair/xfs_repair.c +1 -0
+Geometry information can not be validated if there is only a single

"can not be validated if only a single allocation group exists"

Quoted 4 lines

repair/xfs_repair.c RAM size. · line 156

Quoted 4 lines
repair/xfs_repair.c +4 -0
+allocation group exists and thus no backups superblock exists, or
+if there are two allocation groups and the two superblocks do not
+agree on the filesystem geometry.  Only use this option if you validated
+the geometry yourself and know what your doing.  In doubt do a run

"know what you are doing. If in doubt, do a run"

Quoted 1 line

repair/xfs_repair.c RAM size. · line 157

Quoted 1 line
repair/xfs_repair.c +1 -0
+in no modify mode first.

in "no-modify" (-n) mode first.

Quoted 8 lines
Quoted 8 lines
repair/xfs_repair.c +0 -0
 .RE
 .TP
 .B \-t " interval"

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Quoted 7 lines

---end quoted text---

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw

Looks okay to me (with Eric's suggestions).

Quoted 173 lines · Eric Sandeen
Quoted 3 lines · Christoph Hellwig

Lachlan, Eric,

does this version look okay now?

just minor edits on the manpage change, below

Quoted 124 lines · Christoph Hellwig
Quoted 123 lines

Currently xfs_repair bails out on a filesystem with just a single AG. But that's a perfectly valid configureation, so we should allow it.

Because we could cause harm only allow it when using the force_geometry suboption. But we nice enough to tell the user about it when he needs it.

Also make sure to take the internal log into account when guestimating the first inode cluster offset.

I'll also cook up a testcase for repair on single AG filesystems.

Index: xfsprogs-dev/repair/sb.c
===================================================================

repair/sb.c +19 -8
@@ -760,27 +760,37 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 	switch (num_sbs)  {
 	case 2:
 		/*
-		 * all them have to be right.  if not, report geometry
-		 * and get out unless force option is in effect (-F)
+		 * If we only have two allocation groups, and the superblock
+		 * in the second allocation group differs from the primary
+		 * superblock we can't verify the geometry information.
+		 * Warn the user about this situation and get out unless
+		 * explicitly overridden.
 		 */
 		if (current->refs != 2)  {
 			if (!force_geo)  {
 				do_warn(
-	_("Only two AGs detected and they do not match - cannot proceed.\n"));
+	_("Only two AGs detected and they do not match - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 				exit(1);
 			}
 		}
-		break;
+		goto out_free_list;
 	case 1:
 		/*
-		 * just report the geometry info and get out.
-		 * refuse to run further unless the force (-F)
-		 * option is in effect.
+		 * If we only have a single allocation group there is no
+		 * secondary superblock that we can use to verify the geometry
+		 * information.  Warn the user about this situation and get
+		 * out unless explicitly overridden.
 		 */
 		if (!force_geo)  {
-			do_warn(_("Only one AG detected - cannot proceed.\n"));
+			do_warn(
+	_("Only one AG detected - "
+	  "cannot validate filesystem geometry.\n"
+	  "Use the -o force_geometry option to proceed.\n"));
 			exit(1);
 		}
+		goto out_free_list;
 	default:
 		/*
 		 * at least half of the probed superblocks have
@@ -820,6 +830,7 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 		sb_width = sb->sb_width;
 	}
 
+out_free_list:
 	free_geo(list);
 out:
 	free(sb);

Index: xfsprogs-dev/repair/xfs_repair.c
===================================================================

repair/xfs_repair.c +22 -0
@@ -62,6 +62,8 @@ char *o_opts[] = {
 	"bhash",
 #define	AG_STRIDE	4
 	"ag_stride",
+#define FORCE_GEO	5
+	"force_geometry",
 	NULL
 };
 
@@ -258,6 +260,13 @@ process_args(int argc, char **argv)
 				case AG_STRIDE:
 					ag_stride = (int)strtol(val, NULL, 0);
 					break;
+				case FORCE_GEO:
+					if (val)
+						noval('o', o_opts, FORCE_GEO);
+					if (force_geo)
+						respec('o', o_opts, FORCE_GEO);
+					force_geo = 1;
+					break;
 				default:
 					unknown('o', val);
 					break;
@@ -409,6 +418,19 @@ calc_mkfs(xfs_mount_t *mp)
 	fino_bno = inobt_root + XFS_MIN_FREELIST_RAW(1, 1, mp) + 1;
 
 	/*
+	 * If we only have a single allocation group the log is also allocated
+	 * in the first allocation group and we need to add the number of
+	 * blocks used by the log to the above calculation.
+	 * All this of course doesn't apply if we have an external log.
+	 */
+	if (mp->m_sb.sb_agcount == 1 && mp->m_sb.sb_logstart) {
+		/*
+		 * XXX(hch): verify that sb_logstart makes sense?
+		 */
+		 fino_bno += mp->m_sb.sb_logblocks;
+	}
+
+	/*
 	 * ditto the location of the first inode chunks in the fs ('/')
 	 */
 	if (xfs_sb_version_hasdalign(&mp->m_sb) && do_inoalign)  {

Index: xfsprogs-dev/man/man8/xfs_repair.8
===================================================================
--- xfsprogs-dev.orig/man/man8/xfs_repair.8 2009-01-05 21:11:50.054546919 +0100

Quoted 8 lines

repair/xfs_repair.c RAM size. · line 151

Quoted 8 lines

+++ xfsprogs-dev/man/man8/xfs_repair.8 2009-01-05 21:14:49.228027165 +0100

repair/xfs_repair.c +3 -0
@@ -146,6 +146,15 @@ RAM size.
 This creates additional processing threads to parallel process
 AGs that span multiple concat units. This can significantly
 reduce repair times on concat based filesystems.
+.TP
+.BI force_geometry
+Check the filesystem even if geometry information could not be validate.

"validated."

Quoted 1 line

repair/xfs_repair.c RAM size. · line 152

Quoted 1 line
repair/xfs_repair.c +1 -0
+Geometry information can not be validated if there is only a single

"can not be validated if only a single allocation group exists"

Quoted 4 lines

repair/xfs_repair.c RAM size. · line 156

Quoted 4 lines
repair/xfs_repair.c +4 -0
+allocation group exists and thus no backups superblock exists, or
+if there are two allocation groups and the two superblocks do not
+agree on the filesystem geometry.  Only use this option if you validated
+the geometry yourself and know what your doing.  In doubt do a run

"know what you are doing. If in doubt, do a run"

Quoted 1 line

repair/xfs_repair.c RAM size. · line 157

Quoted 1 line
repair/xfs_repair.c +1 -0
+in no modify mode first.

in "no-modify" (-n) mode first.

Quoted 8 lines
Quoted 8 lines
repair/xfs_repair.c +0 -0
 .RE
 .TP
 .B \-t " interval"

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Quoted 7 lines

---end quoted text---

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Quoted 4 lines

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs