The option '-G' is used to pass number of groups in a flex_bg, the
previous help text - 'meta-group-size' - could confuse users with
meta_bg.
Signed-off-by: Yongqiang Yang <[email protected]>
---
misc/mke2fs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 0ef2531..f01d05c 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -116,7 +116,7 @@ static void usage(void)
fprintf(stderr, _("Usage: %s [-c|-l filename] [-b block-size] "
"[-C cluster-size]\n\t[-i bytes-per-inode] [-I inode-size] "
"[-J journal-options]\n"
- "\t[-G meta group size] [-N number-of-inodes]\n"
+ "\t[-G flex-group-size] [-N number-of-inodes]\n"
"\t[-m reserved-blocks-percentage] [-o creator-os]\n"
"\t[-g blocks-per-group] [-L volume-label] "
"[-M last-mounted-directory]\n\t[-O feature[,...]] "
--
1.7.5.1
Code computing old desc blocks is put into a function.
Signed-off-by: Yongqiang Yang <[email protected]>
---
e2fsck/pass5.c | 8 +-------
lib/ext2fs/alloc.c | 6 +-----
lib/ext2fs/alloc_sb.c | 6 +-----
lib/ext2fs/blknum.c | 14 ++++++++++++++
lib/ext2fs/closefs.c | 7 +------
lib/ext2fs/ext2fs.h | 1 +
resize/resize2fs.c | 41 ++++++++---------------------------------
7 files changed, 27 insertions(+), 56 deletions(-)
diff --git a/e2fsck/pass5.c b/e2fsck/pass5.c
index a60e84a..d9a4488 100644
--- a/e2fsck/pass5.c
+++ b/e2fsck/pass5.c
@@ -216,13 +216,7 @@ redo_counts:
ext2fs_super_and_bgd_loc2(fs, group, &super_blk,
&old_desc_blk, &new_desc_blk, 0);
- if (fs->super->s_feature_incompat &
- EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks =
- fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks +
- fs->super->s_reserved_gdt_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
count = 0;
cmp_block = fs->super->s_clusters_per_group;
diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
index 948a0ec..960a6e2 100644
--- a/lib/ext2fs/alloc.c
+++ b/lib/ext2fs/alloc.c
@@ -47,11 +47,7 @@ static void check_block_uninit(ext2_filsys fs, ext2fs_block_bitmap map,
ext2fs_super_and_bgd_loc2(fs, group, &super_blk,
&old_desc_blk, &new_desc_blk, 0);
- if (fs->super->s_feature_incompat &
- EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
for (i=0; i < fs->super->s_blocks_per_group; i++, blk++)
ext2fs_fast_unmark_block_bitmap2(map, blk);
diff --git a/lib/ext2fs/alloc_sb.c b/lib/ext2fs/alloc_sb.c
index 7517fbc..feb39f7 100644
--- a/lib/ext2fs/alloc_sb.c
+++ b/lib/ext2fs/alloc_sb.c
@@ -52,11 +52,7 @@ int ext2fs_reserve_super_and_bgd(ext2_filsys fs,
ext2fs_super_and_bgd_loc2(fs, group, &super_blk,
&old_desc_blk, &new_desc_blk, &used_blks);
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks =
- fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
if (super_blk || (group == 0))
ext2fs_mark_block_bitmap2(bmap, super_blk);
diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
index 33da7d6..f1d5156 100644
--- a/lib/ext2fs/blknum.c
+++ b/lib/ext2fs/blknum.c
@@ -98,6 +98,20 @@ blk64_t ext2fs_blocks_count(struct ext2_super_block *super)
}
/*
+ * Return the old desc blocks count
+ */
+blk64_t ext2fs_old_desc_blocks_count(ext2_filsys fs)
+{
+ blk64_t old_desc_blocks;
+
+ old_desc_blocks = fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
+ if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG &&
+ old_desc_blocks > fs->super->s_first_meta_bg)
+ old_desc_blocks = fs->super->s_first_meta_bg;
+ return old_desc_blocks;
+}
+
+/*
* Set the fs block count
*/
void ext2fs_blocks_count_set(struct ext2_super_block *super, blk64_t blk)
diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
index 54a24f8..0a97b69 100644
--- a/lib/ext2fs/closefs.c
+++ b/lib/ext2fs/closefs.c
@@ -77,12 +77,7 @@ errcode_t ext2fs_super_and_bgd_loc2(ext2_filsys fs,
if (group_block == 0 && fs->blocksize == 1024)
group_block = 1; /* Deal with 1024 blocksize && bigalloc */
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks =
- fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
-
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
has_super = ext2fs_bg_has_super(fs, group);
if (has_super) {
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 227ee58..78f0f94 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -805,6 +805,7 @@ extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
struct ext2_inode *inode);
extern blk64_t ext2fs_blocks_count(struct ext2_super_block *super);
+extern blk64_t ext2fs_old_desc_blocks_count(ext2_filsys fs);
extern void ext2fs_blocks_count_set(struct ext2_super_block *super,
blk64_t blk);
extern void ext2fs_blocks_count_add(struct ext2_super_block *super,
diff --git a/resize/resize2fs.c b/resize/resize2fs.c
index 06ce73e..0e5cb9e 100644
--- a/resize/resize2fs.c
+++ b/resize/resize2fs.c
@@ -195,6 +195,8 @@ static void fix_uninit_block_bitmaps(ext2_filsys fs)
EXT4_FEATURE_RO_COMPAT_GDT_CSUM)))
return;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
+
for (g=0; g < fs->group_desc_count; g++) {
if (!(ext2fs_bg_flags_test(fs, g, EXT2_BG_BLOCK_UNINIT)))
continue;
@@ -205,11 +207,6 @@ static void fix_uninit_block_bitmaps(ext2_filsys fs)
ext2fs_super_and_bgd_loc2(fs, g, &super_blk,
&old_desc_blk, &new_desc_blk, 0);
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks +
- fs->super->s_reserved_gdt_blocks;
for (i=0; i < fs->super->s_blocks_per_group; i++, blk++) {
if (blk >= ext2fs_blocks_count(fs->super))
@@ -486,11 +483,7 @@ retry:
EXT4_FEATURE_RO_COMPAT_GDT_CSUM);
adj = old_fs->group_desc_count;
max_group = fs->group_desc_count - adj;
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks +
- fs->super->s_reserved_gdt_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
for (i = old_fs->group_desc_count;
i < fs->group_desc_count; i++) {
memset(ext2fs_group_desc(fs, fs->group_desc, i), 0,
@@ -684,11 +677,7 @@ static errcode_t mark_table_blocks(ext2_filsys fs,
unsigned int old_desc_blocks;
meta_bg_size = EXT2_DESC_PER_BLOCK(fs->super);
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks +
- fs->super->s_reserved_gdt_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
for (i = 0; i < fs->group_desc_count; i++) {
ext2fs_reserve_super_and_bgd(fs, i, bmap);
@@ -823,13 +812,8 @@ static errcode_t blocks_to_move(ext2_resize_t rfs)
ext2fs_mark_block_bitmap2(rfs->reserve_blocks, blk);
}
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) {
- old_blocks = old_fs->super->s_first_meta_bg;
- new_blocks = fs->super->s_first_meta_bg;
- } else {
- old_blocks = old_fs->desc_blocks + old_fs->super->s_reserved_gdt_blocks;
- new_blocks = fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
- }
+ old_blocks = ext2fs_old_desc_blocks_count(old_fs);
+ new_blocks = ext2fs_old_desc_blocks_count(fs);
if (old_blocks == new_blocks) {
retval = 0;
@@ -1773,11 +1757,7 @@ static errcode_t ext2fs_calculate_summary_stats(ext2_filsys fs)
uninit = ext2fs_bg_flags_test(fs, group, EXT2_BG_BLOCK_UNINIT);
ext2fs_super_and_bgd_loc2(fs, group, &super_blk, &old_desc_blk,
&new_desc_blk, 0);
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks +
- fs->super->s_reserved_gdt_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
for (blk = fs->super->s_first_data_block;
blk < ext2fs_blocks_count(fs->super); blk++) {
if ((uninit &&
@@ -1809,12 +1789,7 @@ static errcode_t ext2fs_calculate_summary_stats(ext2_filsys fs)
ext2fs_super_and_bgd_loc2(fs, group, &super_blk,
&old_desc_blk,
&new_desc_blk, 0);
- if (fs->super->s_feature_incompat &
- EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks +
- fs->super->s_reserved_gdt_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count(fs);
}
}
ext2fs_free_blocks_count_set(fs->super, total_free);
--
1.7.5.1
If first_meta_bg > desc_blocks, ext2fs_open reads more decs_blocks,
however desc buffer in memory is allocated based on desc_blocks.
Maybe there are similar problems in other places, so this patch adds a
function which computes right old_desc_blocks.
The problem can be reproduced by setting first_meta_bg.
Signed-off-by: Yongqiang Yang <[email protected]>
---
lib/ext2fs/blknum.c | 15 +++++++++++++++
lib/ext2fs/closefs.c | 5 +----
lib/ext2fs/ext2fs.h | 1 +
lib/ext2fs/openfs.c | 18 ++++++++----------
misc/dumpe2fs.c | 10 ++--------
5 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
index f1d5156..6d057ed 100644
--- a/lib/ext2fs/blknum.c
+++ b/lib/ext2fs/blknum.c
@@ -112,6 +112,21 @@ blk64_t ext2fs_old_desc_blocks_count(ext2_filsys fs)
}
/*
+ * Return the old desc blocks count without reserved ones.
+ */
+blk64_t ext2fs_old_desc_blocks_count_without_rsved(ext2_filsys fs)
+{
+ blk64_t old_desc_blocks;
+
+ old_desc_blocks = fs->desc_blocks;
+ if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG &&
+ old_desc_blocks > fs->super->s_first_meta_bg)
+ old_desc_blocks = fs->super->s_first_meta_bg;
+
+ return old_desc_blocks;
+}
+
+/*
* Set the fs block count
*/
void ext2fs_blocks_count_set(struct ext2_super_block *super, blk64_t blk)
diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
index 0a97b69..75210c2 100644
--- a/lib/ext2fs/closefs.c
+++ b/lib/ext2fs/closefs.c
@@ -328,10 +328,7 @@ errcode_t ext2fs_flush2(ext2_filsys fs, int flags)
* superblocks and group descriptors.
*/
group_ptr = (char *) group_shadow;
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count_without_rsved(fs);
ext2fs_numeric_progress_init(fs, &progress, NULL,
fs->group_desc_count);
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 78f0f94..aae57e3 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -806,6 +806,7 @@ extern blk64_t ext2fs_inode_i_blocks(ext2_filsys fs,
struct ext2_inode *inode);
extern blk64_t ext2fs_blocks_count(struct ext2_super_block *super);
extern blk64_t ext2fs_old_desc_blocks_count(ext2_filsys fs);
+extern blk64_t ext2fs_old_desc_blocks_count_without_rsved(ext2_filsys fs);
extern void ext2fs_blocks_count_set(struct ext2_super_block *super,
blk64_t blk);
extern void ext2fs_blocks_count_add(struct ext2_super_block *super,
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index 40a52c5..1077321 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -95,7 +95,7 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
{
ext2_filsys fs;
errcode_t retval;
- unsigned long i, first_meta_bg;
+ unsigned long i, old_desc_blocks;
__u32 features;
unsigned int groups_per_block, blocks_per_group, io_flags;
blk64_t group_block, blk;
@@ -331,25 +331,23 @@ errcode_t ext2fs_open2(const char *name, const char *io_options,
group_block = 1; /* Deal with 1024 blocksize && bigalloc */
dest = (char *) fs->group_desc;
groups_per_block = EXT2_DESC_PER_BLOCK(fs->super);
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- first_meta_bg = fs->super->s_first_meta_bg;
- else
- first_meta_bg = fs->desc_blocks;
- if (first_meta_bg) {
+
+ old_desc_blocks = ext2fs_old_desc_blocks_count_without_rsved(fs);
+ if (old_desc_blocks) {
retval = io_channel_read_blk(fs->io, group_block+1,
- first_meta_bg, dest);
+ old_desc_blocks, dest);
if (retval)
goto cleanup;
#ifdef WORDS_BIGENDIAN
gdp = (struct ext2_group_desc *) dest;
- for (j=0; j < groups_per_block*first_meta_bg; j++) {
+ for (j=0; j < groups_per_block * old_desc_blocks; j++) {
gdp = ext2fs_group_desc(fs, fs->group_desc, j);
ext2fs_swap_group_desc2(fs, gdp);
}
#endif
- dest += fs->blocksize*first_meta_bg;
+ dest += fs->blocksize * old_desc_blocks;
}
- for (i=first_meta_bg ; i < fs->desc_blocks; i++) {
+ for (i = old_desc_blocks; i < fs->desc_blocks; i++) {
blk = ext2fs_descriptor_block_loc2(fs, group_block, i);
retval = io_channel_read_blk64(fs->io, blk, 1, dest);
if (retval)
diff --git a/misc/dumpe2fs.c b/misc/dumpe2fs.c
index 2f9e87b..61c32f2 100644
--- a/misc/dumpe2fs.c
+++ b/misc/dumpe2fs.c
@@ -175,10 +175,7 @@ static void list_desc (ext2_filsys fs)
reserved_gdt = fs->super->s_reserved_gdt_blocks;
fputc('\n', stdout);
first_block = fs->super->s_first_data_block;
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count_without_rsved(fs);
for (i = 0; i < fs->group_desc_count; i++) {
first_block = ext2fs_group_first_block2(fs, i);
last_block = ext2fs_group_last_block2(fs, i);
@@ -495,10 +492,7 @@ void print_super_gdt_blocks_count (ext2_filsys fs) {
blk64_t old_desc_blocks;
unsigned int grp;
- if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
- old_desc_blocks = fs->super->s_first_meta_bg;
- else
- old_desc_blocks = fs->desc_blocks;
+ old_desc_blocks = ext2fs_old_desc_blocks_count_without_rsved(fs);
for (grp = 0; grp < fs->group_desc_count; grp++) {
ext2fs_super_and_bgd_loc2(fs, grp, &super_blk,
--
1.7.5.1
On Wed, Jan 04, 2012 at 07:02:12PM +0800, Yongqiang Yang wrote:
> If first_meta_bg > desc_blocks, ext2fs_open reads more decs_blocks,
> however desc buffer in memory is allocated based on desc_blocks.
> Maybe there are similar problems in other places, so this patch adds a
> function which computes right old_desc_blocks.
>
> The problem can be reproduced by setting first_meta_bg.
s_first_meta_bg should never be greater than desc_blocks. If it is,
the file system is corrupt. This is something that we should check in
ext2fs_open() and in e2fsck as well.
A much better thing to do would be to have ext2fs_open simply fail the
open with an EXT2_ET_CORRUPT_SUPERBLOCK error. Then e2fsck will
automatically try using the backup superblock, which will hopefully
allow the user to recover from the corrupted superblock.
- Ted
On Wed, Jan 04, 2012 at 07:02:10PM +0800, Yongqiang Yang wrote:
> The option '-G' is used to pass number of groups in a flex_bg, the
> previous help text - 'meta-group-size' - could confuse users with
> meta_bg.
>
> Signed-off-by: Yongqiang Yang <[email protected]>
Thanks, applied.
- Ted
On Wed, Jan 04, 2012 at 07:02:11PM +0800, Yongqiang Yang wrote:
> diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
> index 33da7d6..f1d5156 100644
> --- a/lib/ext2fs/blknum.c
> +++ b/lib/ext2fs/blknum.c
> @@ -98,6 +98,20 @@ blk64_t ext2fs_blocks_count(struct ext2_super_block *super)
> }
>
> /*
> + * Return the old desc blocks count
> + */
> +blk64_t ext2fs_old_desc_blocks_count(ext2_filsys fs)
> +{
> + blk64_t old_desc_blocks;
> +
> + old_desc_blocks = fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
> + if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG &&
> + old_desc_blocks > fs->super->s_first_meta_bg)
> + old_desc_blocks = fs->super->s_first_meta_bg;
> + return old_desc_blocks;
I'd prefer if you don't refactor code and change what it does at the
same time. That makes it harder later to figure out whether a bug is
introduced by the refactorization, or by the change in the
calculation. It's especially bad when there is no mention of the
change of the calculation in the commit description.
As I mentioned in my comments for your 3/3 patch, s_first_meta_bg
should *always* be less than old_desc_blocks, since it describes the
meta-blockgroup number where we start using the meta-blockgroup
layout. Since there the size of the meta-blockgroup is defined to be
the number of block groups descriptors that fit in a block, by
definition the meta_bg must be less than or equal to old_desc_blocks.
If it isn't the superblock must be corrupt.
I think it's much better to fix this by adding a check to open and
initialization functions.
- Ted