From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay2.corp.sgi.com [137.38.102.29]) by oss.sgi.com (8.12.11.20060308/8.12.11/SuSE Linux 0.7) with ESMTP id n0K29NrL006899 for ; Mon, 19 Jan 2009 20:09:24 -0600 Message-ID: <497532FD.9030302@sgi.com> Date: Tue, 20 Jan 2009 13:12:13 +1100 From: Lachlan McIlroy MIME-Version: 1.0 Subject: Re: [PATCH] xfs_repair: allow filesystems with a single allocation group References: <20090105201942.GA6960@infradead.org> <20090118173417.GA32597@infradead.org> <4973848B.1020201@sandeen.net> In-Reply-To: <4973848B.1020201@sandeen.net> Reply-To: lachlan@sgi.com List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: xfs-bounces@oss.sgi.com Errors-To: xfs-bounces@oss.sgi.com To: Eric Sandeen Cc: Christoph Hellwig , xfs@oss.sgi.com Looks okay to me (with Eric's suggestions). Eric Sandeen wrote: > 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 >>> Reported-By: Arkadiusz Miskiewicz >>> >>> 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 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs