| Message ID | 170069445376.1865809.6391643475229742760.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfsprogs: minor fixes for 6.6 | expand |
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 :) Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Nov 22, 2023 at 10:42:40PM -0800, Christoph Hellwig wrote: > Nit: overly long line. Trivially fixable by just inverting the > conditions :) s/invert/reorder/
On Wed, Nov 22, 2023 at 03:07:33 PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Fix this check to look at the correct header field. Looks good to me. Reviewed-by: Chandan Babu R <chandanbabu@kernel.org> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > mdrestore/xfs_mdrestore.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > > diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c > index 3190e07e478..3f761e8fe8d 100644 > --- a/mdrestore/xfs_mdrestore.c > +++ b/mdrestore/xfs_mdrestore.c > @@ -268,8 +268,6 @@ read_header_v2( > union mdrestore_headers *h, > FILE *md_fp) > { > - bool want_external_log; > - > if (fread((uint8_t *)&(h->v2) + sizeof(h->v2.xmh_magic), > sizeof(h->v2) - sizeof(h->v2.xmh_magic), 1, md_fp) != 1) > fatal("error reading from metadump file\n"); > @@ -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) > fatal("External Log device is required\n"); > } >
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> >
On Mon, Nov 27, 2023 at 10:27:38AM -0800, Darrick J. Wong wrote: > 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"); That looks even better.
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c index 3190e07e478..3f761e8fe8d 100644 --- a/mdrestore/xfs_mdrestore.c +++ b/mdrestore/xfs_mdrestore.c @@ -268,8 +268,6 @@ read_header_v2( union mdrestore_headers *h, FILE *md_fp) { - bool want_external_log; - if (fread((uint8_t *)&(h->v2) + sizeof(h->v2.xmh_magic), sizeof(h->v2) - sizeof(h->v2.xmh_magic), 1, md_fp) != 1) fatal("error reading from metadump file\n"); @@ -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) fatal("External Log device is required\n"); }