Re: [PATCH 2/2] xfs: fix log recovery when unknown rocompat bits are set

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Aug 30, 2023 at 10:31:59AM +1000, Dave Chinner wrote:
> On Tue, Aug 29, 2023 at 04:09:35PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Fix log recovery to proceed on a readonly mount if the primary sb
> > advertises unknown rocompat bits.  We used to allow this, but due to a
> > misunderstanding between Eric and Darrick back in 2018, we accidentally
> > changed the superblock write verifier to shutdown the fs over that exact
> > scenario.  As a result, the log cleaning that occurs at the end of the
> > mounting process fails.
> > 
> > We now allow writing of the superblock if there are unknown rocompat
> > bits set, but only if the filesystem is read only.  Hence we still have
> > to remove all ro state toggling that occurs around the log mount calls.
> 
> APart from the wording of the commit message, this all looks good.
> It took me a little while to parse what it meant paragraph meant,
> so can I suggest something like:
> 
> 	Log recovery has always run on read only mounts, even where the
> 	primary superblock advertises unknown rocompat bits. Due to a
> 	misunderstanding between Eric and Darrick back in 2018, we accidentally
> 	changed the superblock write verifier to shutdown the fs over that exact
> 	scenario.  As a result, the log cleaning that occurs at the end of the
> 	mounting process fails if there are unknown rocompat bits set.
> 
> 	As we now allow writing of the superblock if there are unknown
> 	rocompat bits set on a RO mount, we no longer want to turn off RO
> 	state to allow log recovery to succeed on a RO mount.  Hence we also
> 	remove all the (now unnecessary) RO state toggling from the log
> 	recovery path.

Ok, I've copied that in verbatim.  Thanks for reviewing these!

--D

> Other than that,
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> 
> -- 
> Dave Chinner
> david@fromorbit.com