Skip to contentSkip to conversation

xfs_repair: copy, don't clear, stripe geometry in backup SB

PATCH In review Eric Sandeen 3 participants 3 messages Jul 11, 2014 – Jul 16, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs_repair: copy, don't clear, stripe geometry in backup SB

Today, if we have a filesystem with stripe geometry and a damaged primary superblock, we will zero out stripe geometry if we have copied the backup.

I'm guessing this might be because changing geometry with mount options only updates the primary, so backups aren't guaranteed to be current or correct.

Unfortunately, that leaves us with sb 0 w/ no geom, and backups *with* geom, so the next repair finds the mismatch, and complains. (In other words, the 2nd repair does not come up clean.)_ And ... the second repair copies the backup stripe geometry back into the primary!

Rather than clearing stripe geometry in this case, just leave it at what was found in the backup super, and inform the user that this was done. This leaves a consistent filesystem, and gives the user a heads-up to double-check the result.

This can all be demonstrated and tested by running xfs/030 with geometry set in MKFS_OPTIONS. (To really make the test pass, we need to filter the warning out of repair output.)

---

repair/globals.h +1 -1
index f6e0a22..6207ca1 100644
@@ -124,7 +124,7 @@ EXTERN int	lazy_count;		/* What to set if to if converting */
 
 EXTERN int		primary_sb_modified;
 EXTERN int		bad_ino_btree;
-EXTERN int		clear_sunit;
+EXTERN int		copied_sunit;
 EXTERN int		fs_is_dirty;
 
 /* for hunting down the root inode */
repair/sb.c +1 -1
index bc421cc..ad27756 100644
@@ -151,7 +151,7 @@ find_secondary_sb(xfs_sb_t *rsb)
 			 */
 			memmove(rsb, &bufsb, sizeof(xfs_sb_t));
 			rsb->sb_inprogress = 0;
-			clear_sunit = 1;
+			copied_sunit = 1;
 
 			if (verify_set_primary_sb(rsb, 0, &dirty) == XR_OK)  {
 				do_warn(
repair/xfs_repair.c +4 -6
index 9eb2fa4..834697a 100644
@@ -193,7 +193,7 @@ process_args(int argc, char **argv)
 	delete_attr_ok = 1;
 	force_geo = 0;
 	assume_xfs = 0;
-	clear_sunit = 0;
+	copied_sunit = 0;
 	sb_inoalignmt = 0;
 	sb_unit = 0;
 	sb_width = 0;
@@ -898,13 +898,11 @@ _("Warning:  project quota information would be cleared.\n"
 		dsb->sb_qflags &= cpu_to_be16(~XFS_ALL_QUOTA_CHKD);
 	}
 
-	if (clear_sunit) {
+	if (copied_sunit) {
 		do_warn(
-_("Note - stripe unit (%d) and width (%d) fields have been reset.\n"
-  "Please set with mount -o sunit=<value>,swidth=<value>\n"),
+_("Note - stripe unit (%d) and width (%d) were copied from a backup superblock.\n"
+  "Please reset with mount -o sunit=<value>,swidth=<value> if necessary\n"),
 			be32_to_cpu(dsb->sb_unit), be32_to_cpu(dsb->sb_width));
-		dsb->sb_unit = 0;
-		dsb->sb_width = 0;
 	}
 
 	libxfs_writebuf(sbp, 0);

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

Reviewed-by on v1 1/1
#1 lore raw
Quoted 19 lines · Eric Sandeen

Today, if we have a filesystem with stripe geometry and a damaged primary superblock, we will zero out stripe geometry if we have copied the backup.

I'm guessing this might be because changing geometry with mount options only updates the primary, so backups aren't guaranteed to be current or correct.

Unfortunately, that leaves us with sb 0 w/ no geom, and backups *with* geom, so the next repair finds the mismatch, and complains. (In other words, the 2nd repair does not come up clean.)_ And ... the second repair copies the backup stripe geometry back into the primary!

Rather than clearing stripe geometry in this case, just leave it at what was found in the backup super, and inform the user that this was done. This leaves a consistent filesystem, and gives the user a heads-up to double-check the result.

Quoted 6 lines

This can all be demonstrated and tested by running xfs/030 with geometry set in MKFS_OPTIONS. (To really make the test pass, we need to filter the warning out of repair output.)

---

Seems reasonable to me.

Quoted 58 lines
repair/globals.h +1 -1
index f6e0a22..6207ca1 100644
@@ -124,7 +124,7 @@ EXTERN int	lazy_count;		/* What to set if to if converting */
 
 EXTERN int		primary_sb_modified;
 EXTERN int		bad_ino_btree;
-EXTERN int		clear_sunit;
+EXTERN int		copied_sunit;
 EXTERN int		fs_is_dirty;
 
 /* for hunting down the root inode */
repair/sb.c +1 -1
index bc421cc..ad27756 100644
@@ -151,7 +151,7 @@ find_secondary_sb(xfs_sb_t *rsb)
 			 */
 			memmove(rsb, &bufsb, sizeof(xfs_sb_t));
 			rsb->sb_inprogress = 0;
-			clear_sunit = 1;
+			copied_sunit = 1;
 
 			if (verify_set_primary_sb(rsb, 0, &dirty) == XR_OK)  {
 				do_warn(
repair/xfs_repair.c +4 -6
index 9eb2fa4..834697a 100644
@@ -193,7 +193,7 @@ process_args(int argc, char **argv)
 	delete_attr_ok = 1;
 	force_geo = 0;
 	assume_xfs = 0;
-	clear_sunit = 0;
+	copied_sunit = 0;
 	sb_inoalignmt = 0;
 	sb_unit = 0;
 	sb_width = 0;
@@ -898,13 +898,11 @@ _("Warning:  project quota information would be cleared.\n"
 		dsb->sb_qflags &= cpu_to_be16(~XFS_ALL_QUOTA_CHKD);
 	}
 
-	if (clear_sunit) {
+	if (copied_sunit) {
 		do_warn(
-_("Note - stripe unit (%d) and width (%d) fields have been reset.\n"
-  "Please set with mount -o sunit=<value>,swidth=<value>\n"),
+_("Note - stripe unit (%d) and width (%d) were copied from a backup superblock.\n"
+  "Please reset with mount -o sunit=<value>,swidth=<value> if necessary\n"),
 			be32_to_cpu(dsb->sb_unit), be32_to_cpu(dsb->sb_width));
-		dsb->sb_unit = 0;
-		dsb->sb_width = 0;
 	}
 
 	libxfs_writebuf(sbp, 0);
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

#2 lore raw
In reply to Eric Sandeen
Quoted 14 lines · Eric Sandeen

Today, if we have a filesystem with stripe geometry and a damaged primary superblock, we will zero out stripe geometry if we have copied the backup.

I'm guessing this might be because changing geometry with mount options only updates the primary, so backups aren't guaranteed to be current or correct.

Unfortunately, that leaves us with sb 0 w/ no geom, and backups *with* geom, so the next repair finds the mismatch, and complains. (In other words, the 2nd repair does not come up clean.)_ And ... the second repair copies the backup stripe geometry back into the primary!

Quoted 8 lines

Rather than clearing stripe geometry in this case, just leave it at what was found in the backup super, and inform the user that this was done. This leaves a consistent filesystem, and gives the user a heads-up to double-check the result.

This can all be demonstrated and tested by running xfs/030 with geometry set in MKFS_OPTIONS. (To really make the test pass, we need to filter the warning out of repair output.)

There are a few other tests that also fail due to this. Can you please send an updated filter patch to [email protected]?

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

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