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;
