On Tue, Oct 15, 2019 at 06:25:10PM -0500, Eric Sandeen wrote:
> 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>
Looks good to me. :)
--D
> ---
>
> 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."));
>
