Re: [PATCH 8/9] xfs_mdrestore: EXTERNALLOG is a compat value, not incompat

Darrick J. Wong
2023-11-27T18:27:39Z

On Wed, Nov 22, 2023 at 10:42:40PM -0800, Christoph Hellwig wrote:
> On Wed, Nov 22, 2023 at 03:07:33PM -0800, Darrick J. Wong wrote:
> > @@ -280,10 +278,8 @@ read_header_v2(
> > if (h->v2.xmh_reserved != 0)
> > fatal("Metadump header's reserved field has a non-zero value\n");
> >
> > - want_external_log = !!(be32_to_cpu(h->v2.xmh_incompat_flags) &
> > - XFS_MD2_COMPAT_EXTERNALLOG);
> > -
> > - if (want_external_log && !mdrestore.external_log)
> > + if ((h->v2.xmh_compat_flags & cpu_to_be32(XFS_MD2_COMPAT_EXTERNALLOG)) &&
> > + !mdrestore.external_log)
>
> Nit: overly long line. Trivially fixable by just inverting the
> conditions :)

Hmm. Or I could decode the ondisk field into a stack variable so that
future flags don't have to deal with that:

compat = be32_to_cpu(h->v2.xmh_compat_flags);

if (!mdrestore.external_log && (compat & XFS_MD2_COMPAT_EXTERNALLOG))
fatal("External Log device is required\n");

--D

>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>