Re: [PATCH] metadump: handle corruption errors without aborting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dave,

It counted up to inode 13555712 and then crashed with the error:

malloc_consolidate(): invalid chunk size

Immediately before that, it printed:

xfs_metadump: invalid block number 4358190/50414336 (1169892770398976)
in bmap extent 0 in symlink ino 98799839421

Best,

Sean

On Wed, Feb 2, 2022 at 2:43 PM Sean Caron <scaron@xxxxxxxxx> wrote:
>
> OK, I got it applied with:
>
> git apply --ignore-space-change --ignore-whitespace
>
> I've got it running and so far it looks good. It's gotten past the
> inode number where exited before and now just prints as an
> informational message:
>
> inode xx has unexpected extents
>
> I'll let this run and follow up on this thread if I have any more
> issues with xfs_metadump in particular.
>
> Thanks again,
>
> Sean
>
> On Wed, Feb 2, 2022 at 1:49 PM Sean Caron <scaron@xxxxxxxxx> wrote:
> >
> > Hi Dave,
> >
> > Thank you! I tried copying and pasting this into a file and applying it with:
> >
> > patch < thispatchfile
> >
> > against both the 5.14.2 release and xfsprogs-dev Git pull and I'm
> > getting errors from patch.
> >
> > I also tried using "git patch" and it told me the patch does not apply.
> >
> > I tried applying the patch by hand to the 5.14.2 metadump.c but I get
> > a compilation error in process_dev_inode but I'm not sure if that's
> > because I made a mistake or because the patch is expecting other
> > content to be there, that isn't.
> >
> > I'm sorry for the ignorance but how would I make use of this?
> >
> > Thanks,
> >
> > Sean
> >
> > On Wed, Feb 2, 2022 at 2:42 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> > >
> > >
> > > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > >
> > > Sean Caron reported that a metadump terminated after givin gthis
> > > warning:
> > >
> > > xfs_metadump: inode 2216156864 has unexpected extents
> > >
> > > Metadump is supposed to ignore corruptions and continue dumping the
> > > filesystem as best it can. Whilst it warns about many situations
> > > where it can't fully dump structures, it should stop processing that
> > > structure and continue with the next one until the entire filesystem
> > > has been processed.
> > >
> > > Unfortunately, some warning conditions also return an "abort" error
> > > status, causing metadump to abort if that condition is hit. Most of
> > > these abort conditions should really be "continue on next object"
> > > conditions so that the we attempt to dump the rest of the
> > > filesystem.
> > >
> > > Fix the returns for warnings that incorrectly cause aborts
> > > such that the only abort conditions are read errors when
> > > "stop-on-read-error" semantics are specified. Also make the return
> > > values consistently mean abort/continue rather than returning -errno
> > > to mean "stop because read error" and then trying to infer what
> > > the error means in callers without the context it occurred in.
> > >
> > > Reported-by: Sean Caron <scaron@xxxxxxxxx>
> > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > > ---
> > >
> > > Sean,
> > >
> > > Can you please apply this patch to your xfsprogs source tree and
> > > rebuild it? This should let metadump continue past the corrupt
> > > inodes it aborted on and run through to completion.
> > >
> > > -Dave.
> > >
> > >  db/metadump.c | 94 +++++++++++++++++++++++++++++------------------------------
> > >  1 file changed, 47 insertions(+), 47 deletions(-)
> > >
> > > diff --git a/db/metadump.c b/db/metadump.c
> > > index 96b098b0eaca..9b32b88a3c50 100644
> > > --- a/db/metadump.c
> > > +++ b/db/metadump.c
> > > @@ -1645,7 +1645,7 @@ process_symlink_block(
> > >  {
> > >         struct bbmap    map;
> > >         char            *link;
> > > -       int             ret = 0;
> > > +       int             rval = 1;
> > >
> > >         push_cur();
> > >         map.nmaps = 1;
> > > @@ -1658,8 +1658,7 @@ process_symlink_block(
> > >
> > >                 print_warning("cannot read %s block %u/%u (%llu)",
> > >                                 typtab[btype].name, agno, agbno, s);
> > > -               if (stop_on_read_error)
> > > -                       ret = -1;
> > > +               rval = !stop_on_read_error;
> > >                 goto out_pop;
> > >         }
> > >         link = iocur_top->data;
> > > @@ -1682,10 +1681,11 @@ process_symlink_block(
> > >         }
> > >
> > >         iocur_top->need_crc = 1;
> > > -       ret = write_buf(iocur_top);
> > > +       if (write_buf(iocur_top))
> > > +               rval = 0;
> > >  out_pop:
> > >         pop_cur();
> > > -       return ret;
> > > +       return rval;
> > >  }
> > >
> > >  #define MAX_REMOTE_VALS                4095
> > > @@ -1843,8 +1843,8 @@ process_single_fsb_objects(
> > >         typnm_t         btype,
> > >         xfs_fileoff_t   last)
> > >  {
> > > +       int             rval = 1;
> > >         char            *dp;
> > > -       int             ret = 0;
> > >         int             i;
> > >
> > >         for (i = 0; i < c; i++) {
> > > @@ -1858,8 +1858,7 @@ process_single_fsb_objects(
> > >
> > >                         print_warning("cannot read %s block %u/%u (%llu)",
> > >                                         typtab[btype].name, agno, agbno, s);
> > > -                       if (stop_on_read_error)
> > > -                               ret = -EIO;
> > > +                       rval = !stop_on_read_error;
> > >                         goto out_pop;
> > >
> > >                 }
> > > @@ -1925,16 +1924,17 @@ process_single_fsb_objects(
> > >                 }
> > >
> > >  write:
> > > -               ret = write_buf(iocur_top);
> > > +               if (write_buf(iocur_top))
> > > +                       rval = 0;
> > >  out_pop:
> > >                 pop_cur();
> > > -               if (ret)
> > > +               if (!rval)
> > >                         break;
> > >                 o++;
> > >                 s++;
> > >         }
> > >
> > > -       return ret;
> > > +       return rval;
> > >  }
> > >
> > >  /*
> > > @@ -1952,7 +1952,7 @@ process_multi_fsb_dir(
> > >         xfs_fileoff_t   last)
> > >  {
> > >         char            *dp;
> > > -       int             ret = 0;
> > > +       int             rval = 1;
> > >
> > >         while (c > 0) {
> > >                 unsigned int    bm_len;
> > > @@ -1978,8 +1978,7 @@ process_multi_fsb_dir(
> > >
> > >                                 print_warning("cannot read %s block %u/%u (%llu)",
> > >                                                 typtab[btype].name, agno, agbno, s);
> > > -                               if (stop_on_read_error)
> > > -                                       ret = -1;
> > > +                               rval = !stop_on_read_error;
> > >                                 goto out_pop;
> > >
> > >                         }
> > > @@ -1998,18 +1997,19 @@ process_multi_fsb_dir(
> > >                         }
> > >                         iocur_top->need_crc = 1;
> > >  write:
> > > -                       ret = write_buf(iocur_top);
> > > +                       if (write_buf(iocur_top))
> > > +                               rval = 0;
> > >  out_pop:
> > >                         pop_cur();
> > >                         mfsb_map.nmaps = 0;
> > > -                       if (ret)
> > > +                       if (!rval)
> > >                                 break;
> > >                 }
> > >                 c -= bm_len;
> > >                 s += bm_len;
> > >         }
> > >
> > > -       return ret;
> > > +       return rval;
> > >  }
> > >
> > >  static bool
> > > @@ -2039,15 +2039,15 @@ process_multi_fsb_objects(
> > >                 return process_symlink_block(o, s, c, btype, last);
> > >         default:
> > >                 print_warning("bad type for multi-fsb object %d", btype);
> > > -               return -EINVAL;
> > > +               return 1;
> > >         }
> > >  }
> > >
> > >  /* inode copy routines */
> > >  static int
> > >  process_bmbt_reclist(
> > > -       xfs_bmbt_rec_t          *rp,
> > > -       int                     numrecs,
> > > +       xfs_bmbt_rec_t          *rp,
> > > +       int                     numrecs,
> > >         typnm_t                 btype)
> > >  {
> > >         int                     i;
> > > @@ -2059,7 +2059,7 @@ process_bmbt_reclist(
> > >         xfs_agnumber_t          agno;
> > >         xfs_agblock_t           agbno;
> > >         bool                    is_multi_fsb = is_multi_fsb_object(mp, btype);
> > > -       int                     error;
> > > +       int                     rval = 1;
> > >
> > >         if (btype == TYP_DATA)
> > >                 return 1;
> > > @@ -2123,16 +2123,16 @@ process_bmbt_reclist(
> > >
> > >                 /* multi-extent blocks require special handling */
> > >                 if (is_multi_fsb)
> > > -                       error = process_multi_fsb_objects(o, s, c, btype,
> > > +                       rval = process_multi_fsb_objects(o, s, c, btype,
> > >                                         last);
> > >                 else
> > > -                       error = process_single_fsb_objects(o, s, c, btype,
> > > +                       rval = process_single_fsb_objects(o, s, c, btype,
> > >                                         last);
> > > -               if (error)
> > > -                       return 0;
> > > +               if (!rval)
> > > +                       break;
> > >         }
> > >
> > > -       return 1;
> > > +       return rval;
> > >  }
> > >
> > >  static int
> > > @@ -2331,7 +2331,7 @@ process_inode_data(
> > >         return 1;
> > >  }
> > >
> > > -static int
> > > +static void
> > >  process_dev_inode(
> > >         xfs_dinode_t            *dip)
> > >  {
> > > @@ -2339,15 +2339,13 @@ process_dev_inode(
> > >                 if (show_warnings)
> > >                         print_warning("inode %llu has unexpected extents",
> > >                                       (unsigned long long)cur_ino);
> > > -               return 0;
> > > -       } else {
> > > -               if (zero_stale_data) {
> > > -                       unsigned int    size = sizeof(xfs_dev_t);
> > > +               return;
> > > +       }
> > > +       if (zero_stale_data) {
> > > +               unsigned int    size = sizeof(xfs_dev_t);
> > >
> > > -                       memset(XFS_DFORK_DPTR(dip) + size, 0,
> > > -                                       XFS_DFORK_DSIZE(dip, mp) - size);
> > > -               }
> > > -               return 1;
> > > +               memset(XFS_DFORK_DPTR(dip) + size, 0,
> > > +                               XFS_DFORK_DSIZE(dip, mp) - size);
> > >         }
> > >  }
> > >
> > > @@ -2365,11 +2363,10 @@ process_inode(
> > >         xfs_dinode_t            *dip,
> > >         bool                    free_inode)
> > >  {
> > > -       int                     success;
> > > +       int                     rval = 1;
> > >         bool                    crc_was_ok = false; /* no recalc by default */
> > >         bool                    need_new_crc = false;
> > >
> > > -       success = 1;
> > >         cur_ino = XFS_AGINO_TO_INO(mp, agno, agino);
> > >
> > >         /* we only care about crc recalculation if we will modify the inode. */
> > > @@ -2390,32 +2387,34 @@ process_inode(
> > >         /* copy appropriate data fork metadata */
> > >         switch (be16_to_cpu(dip->di_mode) & S_IFMT) {
> > >                 case S_IFDIR:
> > > -                       success = process_inode_data(dip, TYP_DIR2);
> > > +                       rval = process_inode_data(dip, TYP_DIR2);
> > >                         if (dip->di_format == XFS_DINODE_FMT_LOCAL)
> > >                                 need_new_crc = 1;
> > >                         break;
> > >                 case S_IFLNK:
> > > -                       success = process_inode_data(dip, TYP_SYMLINK);
> > > +                       rval = process_inode_data(dip, TYP_SYMLINK);
> > >                         if (dip->di_format == XFS_DINODE_FMT_LOCAL)
> > >                                 need_new_crc = 1;
> > >                         break;
> > >                 case S_IFREG:
> > > -                       success = process_inode_data(dip, TYP_DATA);
> > > +                       rval = process_inode_data(dip, TYP_DATA);
> > >                         break;
> > >                 case S_IFIFO:
> > >                 case S_IFCHR:
> > >                 case S_IFBLK:
> > >                 case S_IFSOCK:
> > > -                       success = process_dev_inode(dip);
> > > +                       process_dev_inode(dip);
> > >                         need_new_crc = 1;
> > >                         break;
> > >                 default:
> > >                         break;
> > >         }
> > >         nametable_clear();
> > > +       if (!rval)
> > > +               goto done;
> > >
> > >         /* copy extended attributes if they exist and forkoff is valid */
> > > -       if (success && XFS_DFORK_DSIZE(dip, mp) < XFS_LITINO(mp)) {
> > > +       if (XFS_DFORK_DSIZE(dip, mp) < XFS_LITINO(mp)) {
> > >                 attr_data.remote_val_count = 0;
> > >                 switch (dip->di_aformat) {
> > >                         case XFS_DINODE_FMT_LOCAL:
> > > @@ -2425,11 +2424,11 @@ process_inode(
> > >                                 break;
> > >
> > >                         case XFS_DINODE_FMT_EXTENTS:
> > > -                               success = process_exinode(dip, TYP_ATTR);
> > > +                               rval = process_exinode(dip, TYP_ATTR);
> > >                                 break;
> > >
> > >                         case XFS_DINODE_FMT_BTREE:
> > > -                               success = process_btinode(dip, TYP_ATTR);
> > > +                               rval = process_btinode(dip, TYP_ATTR);
> > >                                 break;
> > >                 }
> > >                 nametable_clear();
> > > @@ -2442,7 +2441,8 @@ done:
> > >
> > >         if (crc_was_ok && need_new_crc)
> > >                 libxfs_dinode_calc_crc(mp, dip);
> > > -       return success;
> > > +
> > > +       return rval;
> > >  }
> > >
> > >  static uint32_t        inodes_copied;
> > > @@ -2541,7 +2541,7 @@ copy_inode_chunk(
> > >
> > >                         /* process_inode handles free inodes, too */
> > >                         if (!process_inode(agno, agino + ioff + i, dip,
> > > -                           XFS_INOBT_IS_FREE_DISK(rp, ioff + i)))
> > > +                                       XFS_INOBT_IS_FREE_DISK(rp, ioff + i)))
> > >                                 goto pop_out;
> > >
> > >                         inodes_copied++;
> > > @@ -2800,7 +2800,7 @@ copy_ino(
> > >         xfs_agblock_t           agbno;
> > >         xfs_agino_t             agino;
> > >         int                     offset;
> > > -       int                     rval = 0;
> > > +       int                     rval = 1;
> > >
> > >         if (ino == 0 || ino == NULLFSINO)
> > >                 return 1;



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux