[PATCH, RFC] xfs_repair: allow filesystems with a single AG

[PATCH, RFC] xfs_repair: allow filesystems with a single AG

[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.

Skip the geomery validation because we simply can't do it if we don't
have a secondary SB, and make sure to take the internal log into account
when guestimating the first inode cluster.

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


Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-By: Arkadiusz Miskiewicz <arekm@maven.pl>

Index: xfsprogs/repair/sb.c
===================================================================
--- xfsprogs.orig/repair/sb.c	2008-12-24 23:36:29.946033933 +0100
+++ xfsprogs/repair/sb.c	2008-12-24 23:42:08.101044710 +0100
@@ -773,14 +773,17 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 		break;
 	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.  Not much we can do here, as we don't want
+		 * to prevent the user from checking the filesystem.
+		 *
+		 * XXX(hch): We should allow putting a secondary superblock
+		 *	     into the last sector of a filesystem to so that
+		 *	     we can still have a backup for single allocation
+		 *	     group filesystems.
 		 */
-		if (!force_geo)  {
-			do_warn(_("Only one AG detected - cannot proceed.\n"));
-			exit(1);
-		}
+		break;
 	default:
 		/*
 		 * at least half of the probed superblocks have
Index: xfsprogs/repair/xfs_repair.c
===================================================================
--- xfsprogs.orig/repair/xfs_repair.c	2008-12-25 00:00:09.116033372 +0100
+++ xfsprogs/repair/xfs_repair.c	2008-12-25 00:07:50.295036179 +0100
@@ -409,6 +409,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)  {

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, RFC] xfs_repair: allow filesystems with a single AG

[Eric Sandeen]

Christoph Hellwig wrote:
> 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.
> 
> Skip the geomery validation because we simply can't do it if we don't
> have a secondary SB, and make sure to take the internal log into account
> when guestimating the first inode cluster.
> 
> I'll also cook up a testcase for repair on single AG filesystems.

While I think we should certainly allow this, what's the worst-case
scenario for a corrupted superblock when we can't validate it and
continue with repair?

I wonder if something like

# xfs_repair --allow-single-sb

should be required, with some man page docs suggesting a run with -n
first etc to be sure that garbled geometry doesn't trash the whole thing...?

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, RFC] xfs_repair: allow filesystems with a single AG

[Christoph Hellwig]

On Thu, Dec 25, 2008 at 11:54:00AM -0600, Eric Sandeen wrote:
> Christoph Hellwig wrote:
> > 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.
> > 
> > Skip the geomery validation because we simply can't do it if we don't
> > have a secondary SB, and make sure to take the internal log into account
> > when guestimating the first inode cluster.
> > 
> > I'll also cook up a testcase for repair on single AG filesystems.
> 
> While I think we should certainly allow this, what's the worst-case
> scenario for a corrupted superblock when we can't validate it and
> continue with repair?
> 
> I wonder if something like
> 
> # xfs_repair --allow-single-sb
> 
> should be required, with some man page docs suggesting a run with -n
> first etc to be sure that garbled geometry doesn't trash the whole thing...?

Maybe.  Given that we don't auto fsck anyway it's at least doable.  But
given that there is no other way to repair a single AG filesystem I'm
not sure it helps.  But I can cook up a variant that requires an option.
In fact existing repair code would allow it (and fail utterly) when
a flag is set - there's just no way to set that flag on the command
line..

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, RFC] xfs_repair: allow filesystems with a single AG

[Lachlan McIlroy]

Looks fine to me but I feel that repair is compromised without the second
SB so we should fix mkfs too to prevent people from creating single AG
filesystems.

Christoph Hellwig wrote:
> 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.
> 
> Skip the geomery validation because we simply can't do it if we don't
> have a secondary SB, and make sure to take the internal log into account
> when guestimating the first inode cluster.
> 
> I'll also cook up a testcase for repair on single AG filesystems.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-By: Arkadiusz Miskiewicz <arekm@maven.pl>
> 
> Index: xfsprogs/repair/sb.c
> ===================================================================
> --- xfsprogs.orig/repair/sb.c	2008-12-24 23:36:29.946033933 +0100
> +++ xfsprogs/repair/sb.c	2008-12-24 23:42:08.101044710 +0100
> @@ -773,14 +773,17 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
>  		break;
>  	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.  Not much we can do here, as we don't want
> +		 * to prevent the user from checking the filesystem.
> +		 *
> +		 * XXX(hch): We should allow putting a secondary superblock
> +		 *	     into the last sector of a filesystem to so that
> +		 *	     we can still have a backup for single allocation
> +		 *	     group filesystems.
>  		 */
> -		if (!force_geo)  {
> -			do_warn(_("Only one AG detected - cannot proceed.\n"));
> -			exit(1);
> -		}
> +		break;
>  	default:
>  		/*
>  		 * at least half of the probed superblocks have
> Index: xfsprogs/repair/xfs_repair.c
> ===================================================================
> --- xfsprogs.orig/repair/xfs_repair.c	2008-12-25 00:00:09.116033372 +0100
> +++ xfsprogs/repair/xfs_repair.c	2008-12-25 00:07:50.295036179 +0100
> @@ -409,6 +409,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)  {
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs