xfs
[Top] [All Lists]

Re: [PATCH] xfs_repair: copy, don't clear, stripe geometry in backup SB

To: Eric Sandeen <sandeen@xxxxxxxxxx>
Subject: Re: [PATCH] xfs_repair: copy, don't clear, stripe geometry in backup SB
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 14 Jul 2014 08:34:07 -0400
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <53C06CE9.9000209@xxxxxxxxxx>
References: <53C06CE9.9000209@xxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Fri, Jul 11, 2014 at 06:02:01PM -0500, Eric Sandeen wrote:
> 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.)
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---

Seems reasonable to me.

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

> 
> diff --git a/repair/globals.h b/repair/globals.h
> index f6e0a22..6207ca1 100644
> --- a/repair/globals.h
> +++ b/repair/globals.h
> @@ -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 */
> diff --git a/repair/sb.c b/repair/sb.c
> index bc421cc..ad27756 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -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(
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index 9eb2fa4..834697a 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -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
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>