Re: [PATCH 2/3] xfs: restrict the h_size fixup in xlog_do_recovery_pass

[Brian Foster <bfoster@redhat.com>]

On Mon, Apr 29, 2024 at 09:01:59AM +0200, Christoph Hellwig wrote:
> The reflink and rmap features require a fixed xfsprogs, so don't allow
> this fixup for them.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Seems reasonable..

>  fs/xfs/xfs_log_recover.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index bb8957927c3c2e..d73bec65f93b46 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3040,10 +3040,14 @@ xlog_do_recovery_pass(
>  		 * Detect this condition here. Use lsunit for the buffer size as
>  		 * long as this looks like the mkfs case. Otherwise, return an
>  		 * error to avoid a buffer overrun.
> +		 *
> +		 * Reject the invalid size if the file system has new enough
> +		 * features that require a fixed mkfs.
>  		 */
>  		h_size = be32_to_cpu(rhead->h_size);
>  		h_len = be32_to_cpu(rhead->h_len);
> -		if (h_len > h_size && h_len <= log->l_mp->m_logbsize &&
> +		if (!xfs_has_reflink(log->l_mp) && xfs_has_reflink(log->l_mp) &&
> +		    h_len > h_size && h_len <= log->l_mp->m_logbsize &&

... but I'm going to assume this hasn't been tested. ;) Do you mean to
also check !rmapbt here?

Can you please also just double check that we still handle the original
mkfs problem correctly after these changes? I think that just means mkfs
from a sufficiently old xfsprogs using a larger log stripe unit, and
confirm the fs mounts (with a warning).

Brian

>  		    rhead->h_num_logops == cpu_to_be32(1)) {
>  			xfs_warn(log->l_mp,
>  		"invalid iclog size (%d bytes), using lsunit (%d bytes)",
> -- 
> 2.39.2
> 
>