Creating a 4TB file on a filesystem with the 64bit flag set results in e2fsck consistently complaining about i_blocks being wrong, with confusing messages like this:
Inode 29818882, i_blocks is 8388608816, should be 8388608816. Fix? no
That appears to be caused by ext2fs_inode_i_blocks() checking for the EXT4_FEATURE_RO_COMPAT_HUGE_FILE in the wrong place. Fix it.
Signed-off-by: Justin Maggard <[email protected]>
---
lib/ext2fs/blknum.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
index a48b696..560edc7 100644
--- a/lib/ext2fs/blknum.c
+++ b/lib/ext2fs/blknum.c
@@ -49,7 +49,7 @@ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
struct ext2_inode *inode)
{
return (inode->i_blocks |
- ((fs->super->s_feature_incompat &
+ ((fs->super->s_feature_ro_compat &
EXT4_FEATURE_RO_COMPAT_HUGE_FILE) ?
(__u64) inode->osd2.linux2.l_i_blocks_hi << 32 : 0)) -
(inode->i_file_acl ? fs->blocksize >> 9 : 0);
@@ -62,7 +62,7 @@ blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
struct ext2_inode *inode)
{
return (inode->i_blocks |
- ((fs->super->s_feature_incompat &
+ ((fs->super->s_feature_ro_compat &
EXT4_FEATURE_RO_COMPAT_HUGE_FILE) ?
(__u64)inode->osd2.linux2.l_i_blocks_hi << 32 : 0));
}
--
1.5.6.5
On 2010-08-23, at 19:34, Justin Maggard wrote:
> Creating a 4TB file on a filesystem with the 64bit flag set results in e2fsck consistently complaining about i_blocks being wrong, with confusing messages like this:
>
> That appears to be caused by ext2fs_inode_i_blocks() checking for the EXT4_FEATURE_RO_COMPAT_HUGE_FILE in the wrong place. Fix it.
>
> - ((fs->super->s_feature_incompat &
> + ((fs->super->s_feature_ro_compat &
> EXT4_FEATURE_RO_COMPAT_HUGE_FILE) ?
This isn't the first time that we've had problems like this, and it won't be the last time unless there is a better API for checking these flags.
I've long thought about having something like:
#define EXT4_CHECK_COMPAT(sb, flag) ((super)->s_feature_compat & \
EXT4_FEATURE_COMPAT_ ## flag)
#define EXT4_CHECK_RO_COMPAT(sb, flag) ((super)->s_feature_ro_compat & \
EXT4_FEATURE_RO_COMPAT_ ## flag)
#define EXT4_CHECK_INCOMPAT(sb, flag) ((super)->s_feature_incompat & \
EXT4_FEATURE_INCOMPAT_ ## flag)
#define EXT4_SET_COMPAT(sb, flag) do { (super)->s_feature_compat |= \
EXT4_FEATURE_COMPAT_ ## flag; } while(0)
#define EXT4_SET_RO_COMPAT(sb, flag) do { (super)->s_feature_ro_compat |= \
EXT4_FEATURE_RO_COMPAT_ ##flag;} while(0)
#define EXT4_SET_INCOMPAT(sb, flag) do { (super)->s_feature_incompat |= \
EXT4_FEATURE_INCOMPAT_ ## flag;} while(0)
This will produce a compile error if the flag name and the macro name don't match. Unfortunately, for e2fsprogs we will need 3 sets of such macros because the feature flags are named EXT2_*, EXT3_*, and EXT4_*, depending on when they were added. For the kernel code that wouldn't be needed.
Cheers, Andreas
Andreas Dilger wrote:
> This isn't the first time that we've had problems like this, and it
> won't be the last time unless there is a better API for checking
> these flags.
*nod*
> I've long thought about having something like:
Same thing crossed my mind; seems like a very good idea.
> #define EXT4_CHECK_COMPAT(sb, flag) ((super)->s_feature_compat & \
> EXT4_FEATURE_COMPAT_ ## flag)
> #define EXT4_CHECK_RO_COMPAT(sb, flag) ((super)->s_feature_ro_compat & \
> EXT4_FEATURE_RO_COMPAT_ ## flag)
> #define EXT4_CHECK_INCOMPAT(sb, flag) ((super)->s_feature_incompat & \
> EXT4_FEATURE_INCOMPAT_ ## flag)
> #define EXT4_SET_COMPAT(sb, flag) do { (super)->s_feature_compat |= \
> EXT4_FEATURE_COMPAT_ ## flag; } while(0)
> #define EXT4_SET_RO_COMPAT(sb, flag) do { (super)->s_feature_ro_compat |= \
> EXT4_FEATURE_RO_COMPAT_ ##flag;} while(0)
> #define EXT4_SET_INCOMPAT(sb, flag) do { (super)->s_feature_incompat |= \
> EXT4_FEATURE_INCOMPAT_ ## flag;} while(0)
>
> This will produce a compile error if the flag name and the macro name
> don't match. Unfortunately, for e2fsprogs we will need 3 sets of such
> macros because the feature flags are named EXT2_*, EXT3_*, and
> EXT4_*, depending on when they were added. For the kernel code that
> wouldn't be needed.
Time to name them consistently in e2fsprogs, then, I think?
-Eric
> Cheers, Andreas
On Tue, Aug 24, 2010 at 9:41 AM, Eric Sandeen <[email protected]> wrote:
> Andreas Dilger wrote:
>
>> This isn't the first time that we've had problems like this, and it
>> won't be the last time unless there is a better API for checking
>> these flags.
>
> *nod*
>
On a related note (to my original problem)... is the bad i_blocks
count fixup code in the check_blocks() function of pass1.c broken for
large files, or am I misunderstanding the code? pb.num_blocks and
ext2fs_inode_i_blocks() are both >32 bits, but the fixup code appears
to always try to shove pb.num_blocks into the 32-bit inode->i_blocks
and set inode->osd2.linux2.l_i_blocks_hi = 0.
-Justin
On Tue, Aug 24, 2010 at 01:34:16AM +0000, Justin Maggard wrote:
> Creating a 4TB file on a filesystem with the 64bit flag set results
> in e2fsck consistently complaining about i_blocks being wrong, with
> confusing messages like this:
>
> Inode 29818882, i_blocks is 8388608816, should be 8388608816. Fix? no
>
> That appears to be caused by ext2fs_inode_i_blocks() checking for
> the EXT4_FEATURE_RO_COMPAT_HUGE_FILE in the wrong place. Fix it.
>
> Signed-off-by: Justin Maggard <[email protected]>
Applied to the e2fsprogs master branch. Sorry for the delay in
attending to this patch. This Fall has been incredibly busy for me;
now that I have the maint branch tidied up for the 1.41.12 release,
I'm not catching up with the backlog on the master branch.
- Ted