On Mon, May 07, 2018 at 07:56:21AM -0500, hal@xxxxxxxxxxxx wrote:
> Darrick's suggested patch to sb_logcheck() is the way to accomplish
> what I want with the minimum amount of code changes. But as I look at
> the code, I'm not sure doing it the way Darrick suggests actually
> expresses what we're trying to accomplish.
>
> Let's state the problem as "Recognize that the log is dirty but allow
> blockget to proceed if '-r' is used". If you accept that problem
> statement, then sb_logcheck() should just tell the caller whether the
> log is dirty or not. It's up to the caller to decide what to do with
> that information-- in this case the init() function in db/check.c.
>
> However, right now sb_logcheck() returns 0 for the case where
> there's a fatal error (like not being able to find the log) and for
> the log being dirty. init() would need to distinguish between those
> two cases and act appropriately. That means changing the return
> semantics of sb_logcheck() to something like -1 on error, 0 on dirty,
> and 1 on clean.
>
> Note that sb_logcheck() is also called by sb_logzero() and
> that code would have to be tweaked to handle the new return values.
Ultimately I defer to Eric on this one, but I don't see how sb_logzero
would run to completion in readonly mode, since the only caller of it is
uuid_f, which bails out if we're in readonly mode.
(Not to mention the buffer writes should fail in ro mode).
--D
>
> So the end result is more code changes to do it this way, but I
> feel like it's a more "correct" fix. I have patches created for
> both scenarios-- does anybody have a strong opinion about which way
> to proceed?
>
> --Hal
>
>
> On Mon May 07 06:35, hal@xxxxxxxxxxxx wrote:
> > > Why skip the warning? I think xfs_db should always warn on a dirty log,
> > > but perhaps we could relax the 'return 0' at the end of this hunk if the
> > > fs was opened readonly?
> > >
> > > i.e.
> > >
> > > dbprintf(_("ERROR: The filesystem has..."));
> > > return (x.isreadonly & LIBXFS_ISREADONLY) ? 1 : 0;
> > >
> > > which would also make adding the -R option unnecessary.
> >
> > I like this solution very much. I'll send in this patch in a
> > separate email.
> >
> > Hal Pomeranz
>
> --
> 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
