Message ID | 20110930194126.GU12086@tux1.beaverton.ibm.com |
---|---|
State | Accepted, archived |
Headers | show |
On 2011-09-30, at 1:41 PM, Darrick J. Wong wrote: > The MMP code in libext2fs tries to gate MMP block swab'ing with this test: > > #ifdef EXT2FS_ENABLE_SWAPFS > if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC)) We've never run Lustre on big-endian servers (clients generally work), so it's possible this has been in our patch for years. Thanks for the catch. > However, EXT2FS_ENABLE_SWAPFS never seems to be defined anywhere (all possible > #define's were cleaned out of e2fsprogs ages ago), and even if any still > existed, the field fs->super->s_magic is always in host byteorder, so the test > always fails. So, we can change the #ifdef to WORDS_BIGENDIAN (which is > conditionally defined on BE platforms) and get rid of the broken if test. > > (This fix came up while testing the metadata checksumming patchset) > > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> > --- > > lib/ext2fs/mmp.c | 15 ++++++--------- > 1 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/lib/ext2fs/mmp.c b/lib/ext2fs/mmp.c > index ed6ee42..91f4fb2 100644 > --- a/lib/ext2fs/mmp.c > +++ b/lib/ext2fs/mmp.c > @@ -91,9 +91,8 @@ errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf) > } > > mmp_cmp = fs->mmp_cmp; > -#ifdef EXT2FS_ENABLE_SWAPFS > - if (fs->flags & EXT2_FLAG_SWAP_BYTES) > - ext2fs_swap_mmp(mmp_cmp); > +#ifdef WORDS_BIGENDIAN > + ext2fs_swap_mmp(mmp_cmp); > #endif > > if (buf != NULL && buf != fs->mmp_cmp) > @@ -122,18 +121,16 @@ errcode_t ext2fs_mmp_write(ext2_filsys fs, blk64_t mmp_blk, void *buf) > fs->super->s_mmp_block > ext2fs_blocks_count(fs->super)) > return EXT2_ET_MMP_BAD_BLOCK; > > -#ifdef EXT2FS_ENABLE_SWAPFS > - if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC)) > - ext2fs_swap_mmp(mmp_s); > +#ifdef WORDS_BIGENDIAN > + ext2fs_swap_mmp(mmp_s); > #endif > > /* I was tempted to make this use O_DIRECT and the mmp_fd, but > * this caused no end of grief, while leaving it as-is works. */ > retval = io_channel_write_blk64(fs->io, mmp_blk, -fs->blocksize, buf); > > -#ifdef EXT2FS_ENABLE_SWAPFS > - if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC)) > - ext2fs_swap_mmp(mmp_s); > +#ifdef WORDS_BIGENDIAN > + ext2fs_swap_mmp(mmp_s); > #endif > > /* Make sure the block gets to disk quickly */ > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Cheers, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 30, 2011 at 12:41:26PM -0700, Darrick J. Wong wrote: > The MMP code in libext2fs tries to gate MMP block swab'ing with this test: > > #ifdef EXT2FS_ENABLE_SWAPFS > if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC)) > > However, EXT2FS_ENABLE_SWAPFS never seems to be defined anywhere (all possible > #define's were cleaned out of e2fsprogs ages ago), and even if any still > existed, the field fs->super->s_magic is always in host byteorder, so the test > always fails. So, we can change the #ifdef to WORDS_BIGENDIAN (which is > conditionally defined on BE platforms) and get rid of the broken if test. > > (This fix came up while testing the metadata checksumming patchset) > > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> Applied, thanks. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/ext2fs/mmp.c b/lib/ext2fs/mmp.c index ed6ee42..91f4fb2 100644 --- a/lib/ext2fs/mmp.c +++ b/lib/ext2fs/mmp.c @@ -91,9 +91,8 @@ errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf) } mmp_cmp = fs->mmp_cmp; -#ifdef EXT2FS_ENABLE_SWAPFS - if (fs->flags & EXT2_FLAG_SWAP_BYTES) - ext2fs_swap_mmp(mmp_cmp); +#ifdef WORDS_BIGENDIAN + ext2fs_swap_mmp(mmp_cmp); #endif if (buf != NULL && buf != fs->mmp_cmp) @@ -122,18 +121,16 @@ errcode_t ext2fs_mmp_write(ext2_filsys fs, blk64_t mmp_blk, void *buf) fs->super->s_mmp_block > ext2fs_blocks_count(fs->super)) return EXT2_ET_MMP_BAD_BLOCK; -#ifdef EXT2FS_ENABLE_SWAPFS - if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC)) - ext2fs_swap_mmp(mmp_s); +#ifdef WORDS_BIGENDIAN + ext2fs_swap_mmp(mmp_s); #endif /* I was tempted to make this use O_DIRECT and the mmp_fd, but * this caused no end of grief, while leaving it as-is works. */ retval = io_channel_write_blk64(fs->io, mmp_blk, -fs->blocksize, buf); -#ifdef EXT2FS_ENABLE_SWAPFS - if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC)) - ext2fs_swap_mmp(mmp_s); +#ifdef WORDS_BIGENDIAN + ext2fs_swap_mmp(mmp_s); #endif /* Make sure the block gets to disk quickly */
The MMP code in libext2fs tries to gate MMP block swab'ing with this test: #ifdef EXT2FS_ENABLE_SWAPFS if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC)) However, EXT2FS_ENABLE_SWAPFS never seems to be defined anywhere (all possible #define's were cleaned out of e2fsprogs ages ago), and even if any still existed, the field fs->super->s_magic is always in host byteorder, so the test always fails. So, we can change the #ifdef to WORDS_BIGENDIAN (which is conditionally defined on BE platforms) and get rid of the broken if test. (This fix came up while testing the metadata checksumming patchset) Signed-off-by: Darrick J. Wong <djwong@us.ibm.com> --- lib/ext2fs/mmp.c | 15 ++++++--------- 1 files changed, 6 insertions(+), 9 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html