public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfs_repair: allow filesystems with a single allocation group
Date: Sun, 18 Jan 2009 13:35:39 -0600	[thread overview]
Message-ID: <4973848B.1020201@sandeen.net> (raw)
In-Reply-To: <20090118173417.GA32597@infradead.org>

Christoph Hellwig wrote:
> Lachlan, Eric,
> 
> does this version look okay now?

just minor edits on the manpage change, below

> On Mon, Jan 05, 2009 at 03:19:42PM -0500, 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.
>>
>> 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.
>>
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Reported-By: Arkadiusz Miskiewicz <arekm@maven.pl>
>>
>> Index: xfsprogs-dev/repair/sb.c
>> ===================================================================
>> --- xfsprogs-dev.orig/repair/sb.c	2009-01-03 21:57:42.415546683 +0100
>> +++ xfsprogs-dev/repair/sb.c	2009-01-05 21:19:01.706752325 +0100
>> @@ -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
>> ===================================================================
>> --- xfsprogs-dev.orig/repair/xfs_repair.c	2009-01-03 21:57:42.416547229 +0100
>> +++ xfsprogs-dev/repair/xfs_repair.c	2009-01-05 20:53:24.810738262 +0100
>> @@ -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
>> +++ xfsprogs-dev/man/man8/xfs_repair.8	2009-01-05 21:14:49.228027165 +0100
>> @@ -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."

>> +Geometry information can not be validated if there is only a single

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

>> +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"

>> +in no modify mode first.

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

>>  .RE
>>  .TP
>>  .B \-t " interval"
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
> ---end quoted text---
> 
> _______________________________________________
> 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

  reply	other threads:[~2009-01-18 19:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-05 20:19 [PATCH] xfs_repair: allow filesystems with a single allocation group Christoph Hellwig
2009-01-18 17:34 ` Christoph Hellwig
2009-01-18 19:35   ` Eric Sandeen [this message]
2009-01-20  2:12     ` Lachlan McIlroy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4973848B.1020201@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox