xfs_repair: improve checking of existing rmap and refcount btrees

There are a few deficiencies in the xfs_repair functions that check the
existing reverse mapping and reference count btrees.  First of all, we
don't report corruption or IO errors if we can't read the ondisk
metadata.  Second, we don't consistently warn if we cannot allocate
memory to perform the check.

Add the missing warnings, and simplify the calling convention since we
don't need the extra layer of do_error logging in phase4.c.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>

diff --git a/repair/phase4.c b/repair/phase4.c
index 2260f6a310ebe471fb078893f08a4357a059e507..746e0ccd211d1c54f1614d154ee66e4b6dc0d3c1 100644 (file)
--- a/repair/phase4.c
+++ b/repair/phase4.c
@@ -173,11 +173,7 @@ _("unable to add AG %u metadata reverse-mapping data.\n"), agno);
                do_error(
 _("unable to merge AG %u metadata reverse-mapping data.\n"), agno);
 
-       error = rmaps_verify_btree(wq->wq_ctx, agno);
-       if (error)
-               do_error(
-_("%s while checking reverse-mappings"),
-                        strerror(-error));
+       rmaps_verify_btree(wq->wq_ctx, agno);
 }
 
 static void
@@ -212,17 +208,11 @@ _("%s while fixing inode reflink flags.\n"),
 
 static void
 check_refcount_btrees(
-       struct workqueue*wq,
-       xfs_agnumber_t  agno,
-       void            *arg)
+       struct workqueue        *wq,
+       xfs_agnumber_t          agno,
+       void                    *arg)
 {
-       int             error;
-
-       error = check_refcounts(wq->wq_ctx, agno);
-       if (error)
-               do_error(
-_("%s while checking reference counts"),
-                        strerror(-error));
+       check_refcounts(wq->wq_ctx, agno);
 }
 
 static void

diff --git a/repair/rmap.c b/repair/rmap.c
index e48f6c1e39b91f0ea0f5085c4019742b250f27a7..b76a149d7465e84778bc8ae99fce7adbbb4d2ad4 100644 (file)
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -974,7 +974,7 @@ rmap_is_good(
 /*
  * Compare the observed reverse mappings against what's in the ag btree.
  */
-int
+void
 rmaps_verify_btree(
        struct xfs_mount        *mp,
        xfs_agnumber_t          agno)
@@ -989,21 +989,26 @@ rmaps_verify_btree(
        int                     error;
 
        if (!xfs_has_rmapbt(mp))
-               return 0;
+               return;
        if (rmapbt_suspect) {
                if (no_modify && agno == 0)
                        do_warn(_("would rebuild corrupt rmap btrees.\n"));
-               return 0;
+               return;
        }
 
        /* Create cursors to refcount structures */
        error = rmap_init_cursor(agno, &rm_cur);
-       if (error)
-               return error;
+       if (error) {
+               do_warn(_("Not enough memory to check reverse mappings.\n"));
+               return;
+       }
 
        error = -libxfs_alloc_read_agf(mp, NULL, agno, 0, &agbp);
-       if (error)
+       if (error) {
+               do_warn(_("Could not read AGF %u to check rmap btree.\n"),
+                               agno);
                goto err;
+       }
 
        /* Leave the per-ag data "uninitialized" since we rewrite it later */
        pag = libxfs_perag_get(mp, agno);
@@ -1011,15 +1016,20 @@ rmaps_verify_btree(
 
        bt_cur = libxfs_rmapbt_init_cursor(mp, NULL, agbp, pag);
        if (!bt_cur) {
-               error = -ENOMEM;
+               do_warn(_("Not enough memory to check reverse mappings.\n"));
                goto err;
        }
 
        rm_rec = pop_slab_cursor(rm_cur);
        while (rm_rec) {
                error = rmap_lookup(bt_cur, rm_rec, &tmp, &have);
-               if (error)
+               if (error) {
+                       do_warn(
+_("Could not read reverse-mapping record for (%u/%u).\n"),
+                                       agno, rm_rec->rm_startblock);
                        goto err;
+               }
+
                /*
                 * Using the range query is expensive, so only do it if
                 * the regular lookup doesn't find anything or if it doesn't
@@ -1029,8 +1039,12 @@ rmaps_verify_btree(
                                (!have || !rmap_is_good(rm_rec, &tmp))) {
                        error = rmap_lookup_overlapped(bt_cur, rm_rec,
                                        &tmp, &have);
-                       if (error)
+                       if (error) {
+                               do_warn(
+_("Could not read reverse-mapping record for (%u/%u).\n"),
+                                               agno, rm_rec->rm_startblock);
                                goto err;
+                       }
                }
                if (!have) {
                        do_warn(
@@ -1088,7 +1102,6 @@ err:
        if (agbp)
                libxfs_buf_relse(agbp);
        free_slab_cursor(&rm_cur);
-       return 0;
 }
 
 /*
@@ -1335,7 +1348,7 @@ refcount_avoid_check(void)
 /*
  * Compare the observed reference counts against what's in the ag btree.
  */
-int
+void
 check_refcounts(
        struct xfs_mount                *mp,
        xfs_agnumber_t                  agno)
@@ -1351,21 +1364,26 @@ check_refcounts(
        int                             error;
 
        if (!xfs_has_reflink(mp))
-               return 0;
+               return;
        if (refcbt_suspect) {
                if (no_modify && agno == 0)
                        do_warn(_("would rebuild corrupt refcount btrees.\n"));
-               return 0;
+               return;
        }
 
        /* Create cursors to refcount structures */
        error = init_refcount_cursor(agno, &rl_cur);
-       if (error)
-               return error;
+       if (error) {
+               do_warn(_("Not enough memory to check refcount data.\n"));
+               return;
+       }
 
        error = -libxfs_alloc_read_agf(mp, NULL, agno, 0, &agbp);
-       if (error)
+       if (error) {
+               do_warn(_("Could not read AGF %u to check refcount btree.\n"),
+                               agno);
                goto err;
+       }
 
        /* Leave the per-ag data "uninitialized" since we rewrite it later */
        pag = libxfs_perag_get(mp, agno);
@@ -1373,7 +1391,7 @@ check_refcounts(
 
        bt_cur = libxfs_refcountbt_init_cursor(mp, NULL, agbp, pag);
        if (!bt_cur) {
-               error = -ENOMEM;
+               do_warn(_("Not enough memory to check refcount data.\n"));
                goto err;
        }
 
@@ -1382,8 +1400,12 @@ check_refcounts(
                /* Look for a refcount record in the btree */
                error = -libxfs_refcount_lookup_le(bt_cur,
                                rl_rec->rc_startblock, &have);
-               if (error)
+               if (error) {
+                       do_warn(
+_("Could not read reference count record for (%u/%u).\n"),
+                                       agno, rl_rec->rc_startblock);
                        goto err;
+               }
                if (!have) {
                        do_warn(
 _("Missing reference count record for (%u/%u) len %u count %u\n"),
@@ -1393,8 +1415,12 @@ _("Missing reference count record for (%u/%u) len %u count %u\n"),
                }
 
                error = -libxfs_refcount_get_rec(bt_cur, &tmp, &i);
-               if (error)
+               if (error) {
+                       do_warn(
+_("Could not read reference count record for (%u/%u).\n"),
+                                       agno, rl_rec->rc_startblock);
                        goto err;
+               }
                if (!i) {
                        do_warn(
 _("Missing reference count record for (%u/%u) len %u count %u\n"),
@@ -1425,7 +1451,6 @@ err:
        if (agbp)
                libxfs_buf_relse(agbp);
        free_slab_cursor(&rl_cur);
-       return 0;
 }
 
 /*

diff --git a/repair/rmap.h b/repair/rmap.h
index e5a6a3b46d9f10d42d24c77a8c0d65771d8f0adf..8d176cb3e99cc4cd593c525f914053389c0815df 100644 (file)
--- a/repair/rmap.h
+++ b/repair/rmap.h
@@ -28,7 +28,7 @@ extern int rmap_store_ag_btree_rec(struct xfs_mount *, xfs_agnumber_t);
 extern size_t rmap_record_count(struct xfs_mount *, xfs_agnumber_t);
 extern int rmap_init_cursor(xfs_agnumber_t, struct xfs_slab_cursor **);
 extern void rmap_avoid_check(void);
-extern int rmaps_verify_btree(struct xfs_mount *, xfs_agnumber_t);
+void rmaps_verify_btree(struct xfs_mount *mp, xfs_agnumber_t agno);
 
 extern int64_t rmap_diffkeys(struct xfs_rmap_irec *kp1,
                struct xfs_rmap_irec *kp2);
@@ -39,7 +39,7 @@ extern int compute_refcounts(struct xfs_mount *, xfs_agnumber_t);
 extern size_t refcount_record_count(struct xfs_mount *, xfs_agnumber_t);
 extern int init_refcount_cursor(xfs_agnumber_t, struct xfs_slab_cursor **);
 extern void refcount_avoid_check(void);
-extern int check_refcounts(struct xfs_mount *, xfs_agnumber_t);
+void check_refcounts(struct xfs_mount *mp, xfs_agnumber_t agno);
 
 extern void record_inode_reflink_flag(struct xfs_mount *, struct xfs_dinode *,
        xfs_agnumber_t, xfs_agino_t, xfs_ino_t);