On Thu, Sep 01, 2016 at 04:31:45PM +0100, Martin Dev wrote:
> I made the changes, but I don't know the process for submitting a patch
>
> diff --git a/repair/phase2.c b/repair/phase2.c
> index e21ffa6..f58f1cd 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -85,8 +85,8 @@ zero_log(
> "attempt a repair.\n"));
> } else {
> if (verbose) {
> - do_warn(
> - _("zero_log: head block %" PRId64 " tail block %" PRId64 "\n"),
> + do_log(
> + _(" - zero_log: head block %" PRId64 " tail block %"
> PRId64 "\n"),
> head_blk, tail_blk);
> }
> if (!no_modify && head_blk != tail_blk) {
>
> Thanks for confirmation, this bug has been in xfs_repair for 15 years :)
The patch looks more or less ok... without the line wrapping. I'm curious, why
the change in the message text? The string between the _() gets pulled into a
message catalog for translation, which means that any change generally requires
the string to be re-translated.
The patch also needs a changelog describing why the change is being made (e.g.
"do_warn sets fs_is_dirty, but we're not dirtying anything, so use do_log
instead") and a Signed-off-by[1] tag from you.
Then send it to the maintainer (Dave Chinner) and cc the lists
(linux-xfs@xxxxxxxxxxxxxxx and xfs@xxxxxxxxxxx; we're in the middle of moving
lists) with '[PATCH]' at the beginning of the subject line.
--D
[1] http://developercertificate.org/
>
> --Martin
>
> On Thu, Sep 1, 2016 at 4:03 PM, Darrick J. Wong <darrick.wong@xxxxxxxxxx> wrote:
> > On Thu, Sep 01, 2016 at 03:08:57PM +0100, Martin Dev wrote:
> >> Hey guys,
> >> Using xfs_repair to check the integrity of an xfs filesystem I noticed
> >> it was exiting non-zero with no obvious reason, I started digging
> >> around and think I found the issue
> >>
> >> Cloned / built latest source from
> >> git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
> >>
> >> in repair/phase2.c:76
> >>
> >> error = xlog_find_tail(log, &head_blk, &tail_blk);
> >> if (error) {
> >> do_warn(
> >> _("zero_log: cannot find log head/tail (xlog_find_tail=%d)\n"),
> >> error);
> >> if (!no_modify && !zap_log)
> >> do_error(_(
> >> "ERROR: The log head and/or tail cannot be discovered. Attempt to mount the\n"
> >> "filesystem to replay the log or use the -L option to destroy the log and\n"
> >> "attempt a repair.\n"));
> >> } else {
> >> if (verbose) {
> >> do_warn(
> >> _("zero_log: head block %" PRId64 " tail block %" PRId64 "\n"),
> >> head_blk, tail_blk);
> >> }
> >>
> >> the problem here is the do_warn function sets fs_is_dirty = 1, which
> >> will cause main() to return 1. The if statement checks if there was an
> >> error, if not (and verbose is enabled) it still uses do_warn()
> >>
> >> I inserted a printf in the do_warn function to track the offending
> >> message setting the fs_is_dirty, and I also inserted a printf directly
> >> after the error = xlog_find_tail() call
> >>
> >> DEBUG: ERROR IS 0
> >> zero_log: head block 25408 tail block 25408
> >> DEBUG: WARNING CAUSING FS_IS_DIRTY: zero_log: head block %ld tail block %ld
> >>
> >> It exits correctly (0 exit status) when the -v flag is not passed to
> >> the program.
> >>
> >> I don't really know C, or git well enough to provide a patch but if
> >> someone can check I'm right then it should be a nice easy fix :)
> >
> > Yes, the do_warn should be a do_log since we're not modifying the fs there.
> >
> > --D
> >
> >>
> >> --Martin
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
