On Thu, Mar 06, 2014 at 11:01:55AM -0500, Brian Foster wrote:
> On Tue, Mar 04, 2014 at 07:51:57PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Phase 1 of xfs_repair verifies and corrects the primary
> > superblock of the filesystem. It does not verify that the CRC of the
> > superblock that is found is correct, nor does it recalculate the CRC
> > of the superblock it rewrites.
> >
> > This happens because phase1 does not use the libxfs buffer cache -
> > it just uses pread/pwrite on a memory buffer, and works directly
> > from that buffer. Hence we need to add CRC verification to
> > verify_sb(), and CRC recalculation to write_primary_sb() so that it
> > works correctly.
> >
> > This also enables us to use get_sb() as the method of fetching the
> > superblock from disk after phase 1 without needing to use the libxfs
> > buffer cache and guessing at the sector size. This prevents a
> > verifier error because it attempts to CRC a superblock buffer that
> > is much longer than the usual sector sizes.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > ---
> > repair/agheader.c | 2 +-
> > repair/globals.h | 3 ++-
> > repair/phase1.c | 5 ++--
> > repair/protos.h | 3 ++-
> > repair/sb.c | 71 +++++++++++++++++++++++++++++------------------------
> > repair/xfs_repair.c | 26 +++++++++++---------
> > 6 files changed, 62 insertions(+), 48 deletions(-)
> >
> > diff --git a/repair/agheader.c b/repair/agheader.c
> > index 53e47b6..fc5dac9 100644
> > --- a/repair/agheader.c
> > +++ b/repair/agheader.c
> > @@ -472,7 +472,7 @@ verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
> > int status = XR_OK;
> > int status_sb = XR_OK;
> >
> > - status = verify_sb(sb, (i == 0));
> > + status = verify_sb(sbuf->b_addr, sb, (i == 0));
> >
> > if (status != XR_OK) {
> > do_warn(_("bad on-disk superblock %d - %s\n"),
> > diff --git a/repair/globals.h b/repair/globals.h
> > index cbb2ce7..f6e0a22 100644
> > --- a/repair/globals.h
> > +++ b/repair/globals.h
> > @@ -49,7 +49,8 @@
> > #define XR_BAD_SB_UNIT 17 /* bad stripe unit */
> > #define XR_BAD_SB_WIDTH 18 /* bad stripe width */
> > #define XR_BAD_SVN 19 /* bad shared version number */
> > -#define XR_BAD_ERR_CODE 20 /* Bad error code */
> > +#define XR_BAD_CRC 20 /* Bad CRC */
> > +#define XR_BAD_ERR_CODE 21 /* Bad error code */
> >
> > /* XFS filesystem (il)legal values */
> >
> > diff --git a/repair/phase1.c b/repair/phase1.c
> > index 62de211..ec75ada 100644
> > --- a/repair/phase1.c
> > +++ b/repair/phase1.c
> > @@ -70,13 +70,14 @@ phase1(xfs_mount_t *mp)
> > ag_bp = alloc_ag_buf(MAX_SECTSIZE);
> > sb = (xfs_sb_t *) ag_bp;
> >
> > - if (get_sb(sb, 0LL, MAX_SECTSIZE, 0) == XR_EOF)
> > + rval = get_sb(sb, 0LL, MAX_SECTSIZE, 0);
> > + if (rval == XR_EOF)
> > do_error(_("error reading primary superblock\n"));
> >
> > /*
> > * is this really an sb, verify internal consistency
> > */
>
> This comment can probably go away now. Otherwise, looks good...
>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
Ok, thanks. I'll kill the comment in a followup patch that touches
this code again...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
