How about this: Refactor all the places in the code where we try to render an inode number as a prefix for some sort of status message. This will help make message prefixes more consistent, which should help users to locate broken metadata. Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> [sandeen: swizzle stuff] Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> --- this uses a single function, scrub_render_ino_descr() with a new comment, and automatic space-adding for any extra format. diff --git a/scrub/common.c b/scrub/common.c index 7db47044..7f971de8 100644 --- a/scrub/common.c +++ b/scrub/common.c @@ -354,3 +354,38 @@ within_range( return true; } + +/* + * Render an inode number into a buffer in a format suitable for use as a + * prefix for log messages. The buffer will be filled with: + * "inode <inode number> (<ag number>/<ag inode number>)" + * If the @format argument is non-NULL, it will be rendered into the buffer + * after the inode representation and a single space. + */ +int +scrub_render_ino_descr( + const struct scrub_ctx *ctx, + char *buf, + size_t buflen, + uint64_t ino, + uint32_t gen, + const char *format, + ...) +{ + va_list args; + uint32_t agno; + uint32_t agino; + int ret; + + agno = cvt_ino_to_agno(&ctx->mnt, ino); + agino = cvt_ino_to_agino(&ctx->mnt, ino); + ret = snprintf(buf, buflen, _("inode %"PRIu64" (%"PRIu32"/%"PRIu32")%s"), + ino, agno, agino, format ? " " : ""); + if (ret < 0 || ret >= buflen || format == NULL) + return ret; + + va_start(args, format); + ret += vsnprintf(buf + ret, buflen - ret, format, args); + va_end(args); + return ret; +} diff --git a/scrub/common.h b/scrub/common.h index 33555891..9a37e9ed 100644 --- a/scrub/common.h +++ b/scrub/common.h @@ -86,4 +86,8 @@ bool within_range(struct scrub_ctx *ctx, unsigned long long value, unsigned long long desired, unsigned long long abs_threshold, unsigned int n, unsigned int d, const char *descr); +int scrub_render_ino_descr(const struct scrub_ctx *ctx, char *buf, + size_t buflen, uint64_t ino, uint32_t gen, + const char *format, ...); + #endif /* XFS_SCRUB_COMMON_H_ */ diff --git a/scrub/inodes.c b/scrub/inodes.c index 91632b55..7aa61ebe 100644 --- a/scrub/inodes.c +++ b/scrub/inodes.c @@ -159,8 +159,8 @@ xfs_iterate_inodes_ag( ireq->hdr.ino = inumbers->xi_startino; goto igrp_retry; } - snprintf(idescr, DESCR_BUFSZ, "inode %"PRIu64, - (uint64_t)bs->bs_ino); + scrub_render_ino_descr(ctx, idescr, DESCR_BUFSZ, + bs->bs_ino, bs->bs_gen, NULL); str_info(ctx, idescr, _("Changed too many times during scan; giving up.")); break; diff --git a/scrub/phase3.c b/scrub/phase3.c index 1e908c2c..0d2c9019 100644 --- a/scrub/phase3.c +++ b/scrub/phase3.c @@ -48,14 +48,10 @@ xfs_scrub_inode_vfs_error( struct xfs_bulkstat *bstat) { char descr[DESCR_BUFSZ]; - xfs_agnumber_t agno; - xfs_agino_t agino; int old_errno = errno; - agno = cvt_ino_to_agno(&ctx->mnt, bstat->bs_ino); - agino = cvt_ino_to_agino(&ctx->mnt, bstat->bs_ino); - snprintf(descr, DESCR_BUFSZ, _("inode %"PRIu64" (%u/%u)"), - (uint64_t)bstat->bs_ino, agno, agino); + scrub_render_ino_descr(ctx, descr, DESCR_BUFSZ, bstat->bs_ino, + bstat->bs_gen, NULL); errno = old_errno; str_errno(ctx, descr); } diff --git a/scrub/phase5.c b/scrub/phase5.c index 99cd51b2..27941907 100644 --- a/scrub/phase5.c +++ b/scrub/phase5.c @@ -234,15 +234,11 @@ xfs_scrub_connections( bool *pmoveon = arg; char descr[DESCR_BUFSZ]; bool moveon = true; - xfs_agnumber_t agno; - xfs_agino_t agino; int fd = -1; int error; - agno = cvt_ino_to_agno(&ctx->mnt, bstat->bs_ino); - agino = cvt_ino_to_agino(&ctx->mnt, bstat->bs_ino); - snprintf(descr, DESCR_BUFSZ, _("inode %"PRIu64" (%u/%u)"), - (uint64_t)bstat->bs_ino, agno, agino); + scrub_render_ino_descr(ctx, descr, DESCR_BUFSZ, bstat->bs_ino, + bstat->bs_gen, NULL); background_sleep(); /* Warn about naming problems in xattrs. */ diff --git a/scrub/phase6.c b/scrub/phase6.c index 8063d6ce..2ce2a19e 100644 --- a/scrub/phase6.c +++ b/scrub/phase6.c @@ -180,15 +180,15 @@ xfs_report_verify_inode( int fd; int error; - snprintf(descr, DESCR_BUFSZ, _("inode %"PRIu64" (unlinked)"), - (uint64_t)bstat->bs_ino); - /* Ignore linked files and things we can't open. */ if (bstat->bs_nlink != 0) return 0; if (!S_ISREG(bstat->bs_mode) && !S_ISDIR(bstat->bs_mode)) return 0; + scrub_render_ino_descr(ctx, descr, DESCR_BUFSZ, + bstat->bs_ino, bstat->bs_gen, _("(unlinked)")); + /* Try to open the inode. */ fd = xfs_open_handle(handle); if (fd < 0) { diff --git a/scrub/scrub.c b/scrub/scrub.c index d7a6b49b..0293ce30 100644 --- a/scrub/scrub.c +++ b/scrub/scrub.c @@ -26,11 +26,13 @@ /* Format a scrub description. */ static void format_scrub_descr( + struct scrub_ctx *ctx, char *buf, size_t buflen, - struct xfs_scrub_metadata *meta, - const struct xfrog_scrub_descr *sc) + struct xfs_scrub_metadata *meta) { + const struct xfrog_scrub_descr *sc = &xfrog_scrubbers[meta->sm_type]; + switch (sc->type) { case XFROG_SCRUB_TYPE_AGHEADER: case XFROG_SCRUB_TYPE_PERAG: @@ -38,8 +40,9 @@ format_scrub_descr( _(sc->descr)); break; case XFROG_SCRUB_TYPE_INODE: - snprintf(buf, buflen, _("Inode %"PRIu64" %s"), - (uint64_t)meta->sm_ino, _(sc->descr)); + scrub_render_ino_descr(ctx, buf, buflen, + meta->sm_ino, meta->sm_gen, "%s", + _(sc->descr)); break; case XFROG_SCRUB_TYPE_FS: snprintf(buf, buflen, _("%s"), _(sc->descr)); @@ -123,8 +126,7 @@ xfs_check_metadata( assert(!debug_tweak_on("XFS_SCRUB_NO_KERNEL")); assert(meta->sm_type < XFS_SCRUB_TYPE_NR); - format_scrub_descr(buf, DESCR_BUFSZ, meta, - &xfrog_scrubbers[meta->sm_type]); + format_scrub_descr(ctx, buf, DESCR_BUFSZ, meta); dbg_printf("check %s flags %xh\n", buf, meta->sm_flags); retry: @@ -681,8 +683,7 @@ xfs_repair_metadata( return CHECK_RETRY; memcpy(&oldm, &meta, sizeof(oldm)); - format_scrub_descr(buf, DESCR_BUFSZ, &meta, - &xfrog_scrubbers[meta.sm_type]); + format_scrub_descr(ctx, buf, DESCR_BUFSZ, &meta); if (needs_repair(&meta)) str_info(ctx, buf, _("Attempting repair."));