On Mon, May 20, 2019 at 12:12:00PM -0400, Brian Foster wrote: > On Mon, May 20, 2019 at 04:02:06PM +0000, bugzilla-daemon@xxxxxxxxxxxxxxxxxxx wrote: > > https://bugzilla.kernel.org/show_bug.cgi?id=203655 > > > > Eric Sandeen (sandeen@xxxxxxxxxxx) changed: > > > > What |Removed |Added > > ---------------------------------------------------------------------------- > > CC| |sandeen@xxxxxxxxxxx > > > > --- Comment #2 from Eric Sandeen (sandeen@xxxxxxxxxxx) --- > > I think the question here is whether the ASSERT() is valid - we don't ever want > > to assert on disk corruption, it should only be for "this should never happen > > in the code" scenarios. > > > > Makes sense. It's not clear to me whether that's the intent of the bug, > but regardless I think it would be reasonable to kill off that > particular assert. We already warn and return an error. IMO, the assert is most definitely valid for a debug build. If I'm writing new code and I corrupt the log, I want it to stop immediately so I can look at what I did wrong the moment it is detected and (hopefully) preserving the underlying filesystem state that is associated with the corrupt journal. Production systems will not have the assert built in and so will return -EIO and fail log recovery gracefully. i.e. The ASSERT is there for the benefit of the XFS developers and has no impact on user systems, so I'd close this NOTABUG. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
