e2fsck: fix error in computing blocks of the ending group
PATCH 1/1 e2fsck: fix error in computing blocks of the ending group
If the blocks of a filesystem is a multiple of blocks_per_group, blocks of the ending group is computed wrongly. This patch computes it by substracting blocks processed from blocks of the filesystem.
--- e2fsck/pass5.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
e2fsck/pass5.c +1 -1
@@ -227,7 +227,7 @@ redo_counts:if (group == (int )fs->group_desc_count -1 ) cmp_block =EXT2FS_NUM_B2C (fs, -ext2fs_blocks_count (fs->super) % fs->super->s_blocks_per_group); +ext2fs_blocks_count (fs->super) - i); } bitmap =0 ;
Signature · 1 line
1.7.5.1
Quoted 14 lines · Yongqiang Yang
If the blocks of a filesystem is a multiple of
blocks_per_group, blocks of the ending group is computed wrongly. This patch computes it by substracting blocks processed from blocks of the filesystem.Signed-off-by Yongqiang Yang <[email protected]>--- e2fsck/pass5.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)e2fsck/pass5.c +0
-0
e2fsck/pass5.c redo_counts: · line 233
Quoted 8 lines
e2fsck/pass5.c +1
-1@@ -227,7 +227,7 @@ redo_counts:if (group == (int )fs->group_desc_count -1 ) cmp_block =EXT2FS_NUM_B2C (fs, -ext2fs_blocks_count (fs->super) % fs->super->s_blocks_per_group); +ext2fs_blocks_count (fs->super) - i); } bitmap =0 ;
I think it would be clearer to just get the last group block count from superblock information, rather than from using the loop counter.
For example resize does:
if (i == fs->group_desc_count-1) {
numblocks = (ext2fs_blocks_count(fs->super) -
fs->super->s_first_data_block) %
fs->super->s_blocks_per_group;
if (!numblocks)
numblocks = fs->super->s_blocks_per_group;same as this function in alloc_sb.c:
if (group == fs->group_desc_count-1) {
num_blocks = (ext2fs_blocks_count(fs->super) -
fs->super->s_first_data_block) %
fs->super->s_blocks_per_group;
if (!num_blocks)
num_blocks = fs->super->s_blocks_per_group;Hm, seems maybe an ext2fs_blocks_in_group() helper function might be nice, since this is a little verbose, and is used fairly often.
-Eric
PATCH 1/1 e2fsprogs: add ext2fs_group_blocks_count helper
Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch).
Making this a helper function should improve matters.
---
(I have not fixed up the spot Yongqiang Yang discovered, but this helper should now make that fix easy to do.)
e2fsck/pass5.c | 6 +----- lib/ext2fs/alloc_sb.c | 10 +--------- lib/ext2fs/blknum.c | 19 +++++++++++++++++++ lib/ext2fs/closefs.c | 9 +-------- lib/ext2fs/ext2fs.h | 1 + resize/online.c | 7 +------ resize/resize2fs.c | 14 ++------------ 7 files changed, 26 insertions(+), 40 deletions(-)
e2fsck/pass5.c +1 -5
@@ -218,11 +218,7 @@ redo_counts: fs->super->s_reserved_gdt_blocks; count =0 ; - cmp_block = fs->super->s_blocks_per_group; -if (group == (int )fs->group_desc_count -1 ) - cmp_block = -ext2fs_blocks_count (fs->super) % - fs->super->s_blocks_per_group; + cmp_block =ext2fs_group_blocks_count (fs, group); } bitmap =0 ;
lib/ext2fs/alloc_sb.c +1 -9
@@ -71,15 +71,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs,if (new_desc_blk)ext2fs_mark_block_bitmap2 (bmap, new_desc_blk); -if (group == fs->group_desc_count-1 ) { - num_blocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!num_blocks) - num_blocks = fs->super->s_blocks_per_group; - }else - num_blocks = fs->super->s_blocks_per_group; - + num_blocks =ext2fs_group_blocks_count (fs, group); num_blocks -=2 + fs->inode_blocks_per_group + used_blks;return num_blocks ;
lib/ext2fs/blknum.c +19 -0
@@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) }/* +* Return the number of blocks in a group +*/ +int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group) +{ +int num_blocks; + +if (group == fs->group_desc_count-1 ) { + num_blocks = (ext2fs_blocks_count (fs->super) - + fs->super->s_first_data_block) % + fs->super->s_blocks_per_group; +if (!num_blocks) + num_blocks = fs->super->s_blocks_per_group; + }else + num_blocks = fs->super->s_blocks_per_group; + +return num_blocks; +} + +/* * Return the inode data block count */ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
lib/ext2fs/closefs.c +1 -8
@@ -150,14 +150,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, &ret_new_desc_blk2, &ret_used_blks); -if (group == fs->group_desc_count-1 ) { - numblocks = (ext2fs_blocks_count (fs->super) - - (blk64_t ) fs->super->s_first_data_block) % - (blk64_t ) fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else - numblocks = fs->super->s_blocks_per_group; + numblocks =ext2fs_group_blocks_count (fs, group);if (ret_super_blk) *ret_super_blk = (blk_t )ret_super_blk2;
lib/ext2fs/ext2fs.h +1 -0
@@ -731,6 +731,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap,extern dgrp_t ext2fs_group_of_blk2 (ext2_filsys fs,blk64_t );extern blk64_t ext2fs_group_first_block2 (ext2_filsys fs,dgrp_t group);extern blk64_t ext2fs_group_last_block2 (ext2_filsys fs,dgrp_t group); +extern int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group);extern blk64_t ext2fs_inode_data_blocks2 (ext2_filsys fs,struct ext2_inode *inode); extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
resize/online.c +1 -6
@@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, input.block_bitmap =ext2fs_block_bitmap_loc (new_fs, i); input.inode_bitmap =ext2fs_inode_bitmap_loc (new_fs, i); input.inode_table =ext2fs_inode_table_loc (new_fs, i); - input.blocks_count = sb->s_blocks_per_group; -if (i == new_fs->group_desc_count-1 ) { - input.blocks_count =ext2fs_blocks_count (new_fs->super) - - sb->s_first_data_block - - (i * sb->s_blocks_per_group); - } + input.blocks_count =ext2fs_group_blocks_count (new_fs, i); input.reserved_blocks = (blk_t) (percent * input.blocks_count /100.0 );
resize/resize2fs.c +2 -12
@@ -499,18 +499,8 @@ retry:ext2fs_bg_flags_zap (fs, i);if (csum_flag)ext2fs_bg_flags_set (fs, i,EXT2_BG_INODE_UNINIT |EXT2_BG_INODE_ZEROED ); -if (i == fs->group_desc_count-1 ) { - numblocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else { - numblocks = fs->super->s_blocks_per_group; -if (csum_flag) -ext2fs_bg_flags_set (fs, i, -EXT2_BG_BLOCK_UNINIT ); - } + + numblocks =ext2fs_group_blocks_count (fs, i); has_super =ext2fs_bg_has_super (fs, i);if (has_super) {
PATCH v2 1/1 e2fsprogs: add ext2fs_group_blocks_count helper
Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch).
Making this a helper function should improve matters.
---
(I have not fixed up the spot Yongqiang Yang discovered, but this helper should now make that fix easy to do.)
V2: Sorry, first patch was against an old tree.
lib/ext2fs/alloc_sb.c | 10 +--------- lib/ext2fs/blknum.c | 19 +++++++++++++++++++ lib/ext2fs/closefs.c | 9 +-------- lib/ext2fs/ext2fs.h | 1 + resize/online.c | 7 +------ resize/resize2fs.c | 14 ++------------ 6 files changed, 25 insertions(+), 35 deletions(-)
lib/ext2fs/alloc_sb.c +1 -9
@@ -74,15 +74,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs,if (new_desc_blk)ext2fs_mark_block_bitmap2 (bmap, new_desc_blk); -if (group == fs->group_desc_count-1 ) { - num_blocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!num_blocks) - num_blocks = fs->super->s_blocks_per_group; - }else - num_blocks = fs->super->s_blocks_per_group; - + num_blocks =ext2fs_group_blocks_count (fs, group); num_blocks -=2 + fs->inode_blocks_per_group + used_blks;return num_blocks ;
lib/ext2fs/blknum.c +19 -0
@@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) }/* +* Return the number of blocks in a group +*/ +int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group) +{ +int num_blocks; + +if (group == fs->group_desc_count-1 ) { + num_blocks = (ext2fs_blocks_count (fs->super) - + fs->super->s_first_data_block) % + fs->super->s_blocks_per_group; +if (!num_blocks) + num_blocks = fs->super->s_blocks_per_group; + }else + num_blocks = fs->super->s_blocks_per_group; + +return num_blocks; +} + +/* * Return the inode data block count */ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
lib/ext2fs/closefs.c +1 -8
@@ -152,14 +152,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, &ret_new_desc_blk2, &ret_used_blks); -if (group == fs->group_desc_count-1 ) { - numblocks = (ext2fs_blocks_count (fs->super) - - (blk64_t ) fs->super->s_first_data_block) % - (blk64_t ) fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else - numblocks = fs->super->s_blocks_per_group; + numblocks =ext2fs_group_blocks_count (fs, group);if (ret_super_blk) *ret_super_blk = (blk_t )ret_super_blk2;
lib/ext2fs/ext2fs.h +1 -0
@@ -757,6 +757,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap,extern dgrp_t ext2fs_group_of_blk2 (ext2_filsys fs,blk64_t );extern blk64_t ext2fs_group_first_block2 (ext2_filsys fs,dgrp_t group);extern blk64_t ext2fs_group_last_block2 (ext2_filsys fs,dgrp_t group); +extern int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group);extern blk64_t ext2fs_inode_data_blocks2 (ext2_filsys fs,struct ext2_inode *inode); extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
resize/online.c +1 -6
@@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, input.block_bitmap =ext2fs_block_bitmap_loc (new_fs, i); input.inode_bitmap =ext2fs_inode_bitmap_loc (new_fs, i); input.inode_table =ext2fs_inode_table_loc (new_fs, i); - input.blocks_count = sb->s_blocks_per_group; -if (i == new_fs->group_desc_count-1 ) { - input.blocks_count =ext2fs_blocks_count (new_fs->super) - - sb->s_first_data_block - - (i * sb->s_blocks_per_group); - } + input.blocks_count =ext2fs_group_blocks_count (new_fs, i); input.reserved_blocks = (blk_t) (percent * input.blocks_count /100.0 );
resize/resize2fs.c +2 -12
@@ -499,18 +499,8 @@ retry:ext2fs_bg_flags_zap (fs, i);if (csum_flag)ext2fs_bg_flags_set (fs, i,EXT2_BG_INODE_UNINIT |EXT2_BG_INODE_ZEROED ); -if (i == fs->group_desc_count-1 ) { - numblocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else { - numblocks = fs->super->s_blocks_per_group; -if (csum_flag) -ext2fs_bg_flags_set (fs, i, -EXT2_BG_BLOCK_UNINIT ); - } + + numblocks =ext2fs_group_blocks_count (fs, i); has_super =ext2fs_bg_has_super (fs, i);if (has_super) {
Re: [PATCH V2] e2fsprogs: add ext2fs_group_blocks_count helper
Quoted 9 lines · Eric Sandeen
Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch).
Making this a helper function should improve matters.
Signed-off-by Eric Sandeen <[email protected]>---
Argh V3 coming today, I got a little too fast and loose sending this one at the end of the day yesterday didn't I:
Quoted 5 lines
resize/resize2fs.c +0
-1@@ -499,18 +499,8 @@ retry:ext2fs_bg_flags_zap (fs, i);if (csum_flag)ext2fs_bg_flags_set (fs, i,EXT2_BG_INODE_UNINIT |EXT2_BG_INODE_ZEROED ); -if (i == fs->group_desc_count-1 ) {
resize/resize2fs.c retry: · line 512
Quoted 10 lines
resize/resize2fs.c +0
-10- numblocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else { - numblocks = fs->super->s_blocks_per_group; -if (csum_flag) -ext2fs_bg_flags_set (fs, i, -EXT2_BG_BLOCK_UNINIT );
still need to do that flag set with the new helper.
- Eric
Quoted 11 lines
resize/resize2fs.c +2
-2- } + + numblocks =ext2fs_group_blocks_count (fs, i); has_super =ext2fs_bg_has_super (fs, i);if (has_super) { --To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] e2fsprogs: add ext2fs_group_blocks_count helper
Quoted 6 lines · Eric Sandeen
Quoted 5 lines · Eric Sandeen
Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch).
Quoted 7 lines
Quoted 4 lines
Making this a helper function should improve matters.
Signed-off-by Eric Sandeen <[email protected]>---
Argh V3 coming today, I got a little too fast and loose sending this one at the end of the day yesterday didn't I:
Hi Eric,
It seemed you did not send out the v3 patch. You just pointed out
what to be added in the V3 patch.
Yongqiang.
Quoted 34 lines
Quoted 6 lines
resize/resize2fs.c +0
-1@@ -499,18 +499,8 @@ retry: ext2fs_bg_flags_zap(fs, i); if (csum_flag) ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); - if (i == fs->group_desc_count-1) {resize/resize2fs.c retry: · line 512
Quoted 10 lines
resize/resize2fs.c +0
-10- numblocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else { - numblocks = fs->super->s_blocks_per_group; - if (csum_flag) - ext2fs_bg_flags_set(fs, i, - EXT2_BG_BLOCK_UNINIT);still need to do that flag set with the new helper.
- Eric
Quoted 11 lines
resize/resize2fs.c +2
-2- } + + numblocks = ext2fs_group_blocks_count(fs, i); has_super = ext2fs_bg_has_super(fs, i); if (has_super) { --To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 6 lines
Best Wishes Yongqiang Yang -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] e2fsprogs: add ext2fs_group_blocks_count helper
Quoted 12 lines · Yongqiang Yang
Quoted 11 lines · Eric Sandeen
Quoted 9 lines · Eric Sandeen
Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch).
Making this a helper function should improve matters.
Signed-off-by Eric Sandeen <[email protected]>---
Quoted 6 lines
Quoted 2 lines
Argh V3 coming today, I got a little too fast and loose sending this one at the end of the day yesterday didn't I:
Hi Eric,
It seemed you did not send out the v3 patch. You just pointed out
what to be added in the V3 patch.
That is correct. :)
I'll send it out a bit later after I carefully review this time :(
-Eric
Quoted 38 lines
Yongqiang.
Quoted 34 lines
Quoted 6 lines
resize/resize2fs.c +0
-1@@ -499,18 +499,8 @@ retry:ext2fs_bg_flags_zap (fs, i);if (csum_flag)ext2fs_bg_flags_set (fs, i,EXT2_BG_INODE_UNINIT |EXT2_BG_INODE_ZEROED ); -if (i == fs->group_desc_count-1 ) {resize/resize2fs.c retry: · line 512
Quoted 10 lines
resize/resize2fs.c +0
-10- numblocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else { - numblocks = fs->super->s_blocks_per_group; -if (csum_flag) -ext2fs_bg_flags_set (fs, i, -EXT2_BG_BLOCK_UNINIT );still need to do that flag set with the new helper.
- Eric
Quoted 11 lines
resize/resize2fs.c +2
-2- } + + numblocks =ext2fs_group_blocks_count (fs, i); has_super =ext2fs_bg_has_super (fs, i);if (has_super) { --To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v3 1/1 e2fsprogs: add ext2fs_group_blocks_count helper
In reply to Eric SandeenCode to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase a few times.
Making this a helper function should improve matters.
---
V2: Sorry, first patch was against an old tree.
V3: Restore lost EXT2_BG_BLOCK_UNINIT in resize2fs.c
lib/ext2fs/alloc_sb.c +1 -9
@@ -74,15 +74,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs,if (new_desc_blk)ext2fs_mark_block_bitmap2 (bmap, new_desc_blk); -if (group == fs->group_desc_count-1 ) { - num_blocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!num_blocks) - num_blocks = fs->super->s_blocks_per_group; - }else - num_blocks = fs->super->s_blocks_per_group; - + num_blocks =ext2fs_group_blocks_count (fs, group); num_blocks -=2 + fs->inode_blocks_per_group + used_blks;return num_blocks ;
lib/ext2fs/blknum.c +19 -0
@@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) }/* +* Return the number of blocks in a group +*/ +int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group) +{ +int num_blocks; + +if (group == fs->group_desc_count -1 ) { + num_blocks = (ext2fs_blocks_count (fs->super) - + fs->super->s_first_data_block) % + fs->super->s_blocks_per_group; +if (!num_blocks) + num_blocks = fs->super->s_blocks_per_group; + }else + num_blocks = fs->super->s_blocks_per_group; + +return num_blocks; +} + +/* * Return the inode data block count */ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
lib/ext2fs/closefs.c +1 -8
@@ -152,14 +152,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, &ret_new_desc_blk2, &ret_used_blks); -if (group == fs->group_desc_count-1 ) { - numblocks = (ext2fs_blocks_count (fs->super) - - (blk64_t ) fs->super->s_first_data_block) % - (blk64_t ) fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else - numblocks = fs->super->s_blocks_per_group; + numblocks =ext2fs_group_blocks_count (fs, group);if (ret_super_blk) *ret_super_blk = (blk_t )ret_super_blk2;
lib/ext2fs/ext2fs.h +1 -0
@@ -757,6 +757,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap,extern dgrp_t ext2fs_group_of_blk2 (ext2_filsys fs,blk64_t );extern blk64_t ext2fs_group_first_block2 (ext2_filsys fs,dgrp_t group);extern blk64_t ext2fs_group_last_block2 (ext2_filsys fs,dgrp_t group); +extern int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group);extern blk64_t ext2fs_inode_data_blocks2 (ext2_filsys fs,struct ext2_inode *inode); extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
resize/online.c +1 -6
@@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, input.block_bitmap =ext2fs_block_bitmap_loc (new_fs, i); input.inode_bitmap =ext2fs_inode_bitmap_loc (new_fs, i); input.inode_table =ext2fs_inode_table_loc (new_fs, i); - input.blocks_count = sb->s_blocks_per_group; -if (i == new_fs->group_desc_count-1 ) { - input.blocks_count =ext2fs_blocks_count (new_fs->super) - - sb->s_first_data_block - - (i * sb->s_blocks_per_group); - } + input.blocks_count =ext2fs_group_blocks_count (new_fs, i); input.reserved_blocks = (blk_t) (percent * input.blocks_count /100.0 );
resize/resize2fs.c +4 -12
@@ -499,18 +499,10 @@ retry:ext2fs_bg_flags_zap (fs, i);if (csum_flag)ext2fs_bg_flags_set (fs, i,EXT2_BG_INODE_UNINIT |EXT2_BG_INODE_ZEROED ); -if (i == fs->group_desc_count-1 ) { - numblocks = (ext2fs_blocks_count (fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; -if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - }else { - numblocks = fs->super->s_blocks_per_group; -if (csum_flag) -ext2fs_bg_flags_set (fs, i, -EXT2_BG_BLOCK_UNINIT ); - } + + numblocks =ext2fs_group_blocks_count (fs, i); +if ((i < fs->group_desc_count -1 ) && csum_flag) +ext2fs_bg_flags_set (fs, i,EXT2_BG_BLOCK_UNINIT ); has_super =ext2fs_bg_has_super (fs, i);if (has_super) {
Ted Ts'o
AppliedRe: [V3] e2fsprogs: add ext2fs_group_blocks_count helper
Quoted 7 lines · Eric Sandeen
Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase a few times.
Making this a helper function should improve matters.
Signed-off-by Eric Sandeen <[email protected]>
Thanks, applied. Apologies for the delay.
- Ted
Re: [PATCH] e2fsprogs: add ext2fs_group_blocks_count helper
In reply to Eric SandeenQuoted 28 lines · Eric Sandeen
Code to count the number of blocks in the last partial group is cut and pasted around the e2fsprogs codebase, and is wrong in at least one instancem as pointed out by Yongqiang Yang (but not fixed in this patch).
Making this a helper function should improve matters.
Signed-off-by Eric Sandeen <[email protected]>---
(I have not fixed up the spot Yongqiang Yang discovered, but this helper should now make that fix easy to do.)
e2fsck/pass5.c | 6 +-----
lib/ext2fs/alloc_sb.c | 10 +---------lib/ext2fs/blknum.c | 19 +++++++++++++++++++ lib/ext2fs/closefs.c | 9 +-------- lib/ext2fs/ext2fs.h | 1 + resize/online.c | 7 +------ resize/resize2fs.c | 14 ++------------ 7 files changed, 26 insertions(+), 40 deletions(-)e2fsck/pass5.c +0
-0
e2fsck/pass5.c redo_counts: · line 222
Quoted 10 lines
e2fsck/pass5.c +1
-5@@ -218,11 +218,7 @@ redo_counts: fs->super->s_reserved_gdt_blocks; count = 0; - cmp_block = fs->super->s_blocks_per_group; - if (group == (int)fs->group_desc_count - 1) - cmp_block = - ext2fs_blocks_count(fs->super) % - fs->super->s_blocks_per_group; + cmp_block = ext2fs_group_blocks_count(fs, group); }
Hi Eric,
You fixed the bug I pointed out here. but your commit log said it is not fixed.
Yongqiang.
Quoted 130 lines
e2fsck/pass5.c +0
-0bitmap = 0;lib/ext2fs/alloc_sb.c +1
-9@@ -71,15 +71,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs, if (new_desc_blk) ext2fs_mark_block_bitmap2(bmap, new_desc_blk); - if (group == fs->group_desc_count-1) { - num_blocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!num_blocks) - num_blocks = fs->super->s_blocks_per_group; - } else - num_blocks = fs->super->s_blocks_per_group; - + num_blocks = ext2fs_group_blocks_count(fs, group); num_blocks -= 2 + fs->inode_blocks_per_group + used_blks; return num_blocks ;lib/ext2fs/blknum.c +19
-0@@ -43,6 +43,25 @@ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group) } /* +* Return the number of blocks in a group +*/ +int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group) +{ + int num_blocks; + + if (group == fs->group_desc_count-1) { + num_blocks = (ext2fs_blocks_count(fs->super) - + fs->super->s_first_data_block) % + fs->super->s_blocks_per_group; + if (!num_blocks) + num_blocks = fs->super->s_blocks_per_group; + } else + num_blocks = fs->super->s_blocks_per_group; + + return num_blocks; +} + +/* * Return the inode data block count */ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,lib/ext2fs/closefs.c +1
-8@@ -150,14 +150,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs, &ret_new_desc_blk2, &ret_used_blks); - if (group == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - (blk64_t) fs->super->s_first_data_block) % - (blk64_t) fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else - numblocks = fs->super->s_blocks_per_group; + numblocks = ext2fs_group_blocks_count(fs, group); if (ret_super_blk) *ret_super_blk = (blk_t)ret_super_blk2;lib/ext2fs/ext2fs.h +1
-0@@ -731,6 +731,7 @@ extern errcode_t ext2fs_get_block_bitmap_range2(ext2fs_block_bitmap bmap, extern dgrp_t ext2fs_group_of_blk2(ext2_filsys fs, blk64_t); extern blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group); extern blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group); +extern int ext2fs_group_blocks_count (ext2_filsys fs,dgrp_t group); extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs, struct ext2_inode *inode); extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,resize/online.c +1
-6@@ -135,12 +135,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt, input.block_bitmap = ext2fs_block_bitmap_loc(new_fs, i); input.inode_bitmap = ext2fs_inode_bitmap_loc(new_fs, i); input.inode_table = ext2fs_inode_table_loc(new_fs, i); - input.blocks_count = sb->s_blocks_per_group; - if (i == new_fs->group_desc_count-1) { - input.blocks_count = ext2fs_blocks_count(new_fs->super) - - sb->s_first_data_block - - (i * sb->s_blocks_per_group); - } + input.blocks_count = ext2fs_group_blocks_count(new_fs, i);input.reserved_blocks = (
blk_t) (percent * input.blocks_count / 100.0);resize/resize2fs.c +2
-12@@ -499,18 +499,8 @@ retry: ext2fs_bg_flags_zap(fs, i); if (csum_flag) ext2fs_bg_flags_set(fs, i, EXT2_BG_INODE_UNINIT | EXT2_BG_INODE_ZEROED); - if (i == fs->group_desc_count-1) { - numblocks = (ext2fs_blocks_count(fs->super) - - fs->super->s_first_data_block) % - fs->super->s_blocks_per_group; - if (!numblocks) - numblocks = fs->super->s_blocks_per_group; - } else { - numblocks = fs->super->s_blocks_per_group; - if (csum_flag) - ext2fs_bg_flags_set(fs, i, - EXT2_BG_BLOCK_UNINIT); - } + + numblocks = ext2fs_group_blocks_count(fs, i); has_super = ext2fs_bg_has_super(fs, i); if (has_super) {
Signature · 6 lines
Best Wishes Yongqiang Yang -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 22 lines · Eric Sandeen
Quoted 21 lines · Yongqiang Yang
If the blocks of a filesystem is a multiple of
blocks_per_group, blocks of the ending group is computed wrongly. This patch computes it by substracting blocks processed from blocks of the filesystem.Signed-off-by Yongqiang Yang <[email protected]>--- e2fsck/pass5.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)e2fsck/pass5.c +1
-1@@ -227,7 +227,7 @@ redo_counts:if (group == (int )fs->group_desc_count -1 ) cmp_block =EXT2FS_NUM_B2C (fs, -ext2fs_blocks_count (fs->super) % fs->super->s_blocks_per_group); +ext2fs_blocks_count (fs->super) - i); }
Quoted 4 lines
e2fsck/pass5.c redo_counts: · line 233
Quoted 1 line
e2fsck/pass5.c +0
-0bitmap =0 ;I think it would be clearer to just get the last group block count from superblock information, rather than from using the loop counter.
Actually I see that I misread this... here we are dealing with clusters per group and my proposed blocks per group helper won't help. Sorry for the noise, I think your patch should go in as proposed.
I'll still send my helper, but I don't think it will help in this location, and your patch is correct.
-Eric
PATCH v2 1/1 e2fsck: fix error in computing blocks of the ending group
In reply to Yongqiang YangFrom: Yongqiang Yang <[email protected]>
If the blocks of a filesystem is a multiple of blocks_per_group, blocks of the ending group is computed wrongly. Use the new ext2fs_group_blocks_count() helper instead.
Eric Sandeen: Converted to use new blocks per group helper
---
e2fsck/pass5.c +2 -3
@@ -225,9 +225,8 @@ redo_counts: count =0 ; cmp_block = fs->super->s_clusters_per_group;if (group == (int )fs->group_desc_count -1 ) - cmp_block = -EXT2FS_NUM_B2C (fs, -ext2fs_blocks_count (fs->super) % fs->super->s_blocks_per_group); + cmp_block =EXT2FS_NUM_B2C (fs, +ext2fs_group_blocks_count (fs, group)); } bitmap =0 ;
Ted Ts'o
AppliedQuoted 9 lines · Eric Sandeen
From: Yongqiang Yang <[email protected]>
If the blocks of a filesystem is a multiple of
blocks_per_group, blocks of the ending group is computed wrongly. Use the newext2fs_group_blocks_count()helper instead.Eric Sandeen: Converted to use new blocks per group helper
Signed-off-by Yongqiang Yang <[email protected]>
Thanks applied to the next branch. Apologies for the delay.
- Ted