Re: [PATCH] resize2fs: Fix support for the uninit_bg feature

[Andreas Dilger <adilger@sun.com>]

On Jun 17, 2008  12:30 -0400, Theodore Ts'o wrote:
> On Tue, Jun 17, 2008 at 09:16:47AM -0600, Andreas Dilger wrote:
> > I'm a bit confused by this.  It doesn't seem that you use old_desc_blocks
> > in the META_BG case?  I'm trying to figure out what "old_desc_blocks"
> > means in the META_BG case.
> 
> In the META_BG case, s_first_meta_bg is the number of the "meta
> blockgroup" where the META_BG layout starts getting used.  The idea
> here is that you can have a traditional filesystem layout, and then
> instead of using ext2_prepare, you simply grow the block groups until
> the traditional block group descriptor space is filled out, and then
> after that, you switch over to the meta_bg layout.  

Sure, I'm aware of how META_BG works.

> To convert conversion between the block group number and a
> meta_block_group number, the formula is:
> 
>        block_grp_num = meta_bg_num * (block_size / descriptor_size)
> 
> Because of this, very conveniently fs->super->s_first_meta also is the
> size of the block group descriptors for block groups below the meta_bg
> boundary.  
> Hence, old_desc_blocks is the number of blocks used by block group
> descriptors for block groups that have the traditional block group
> descriptor layout.

Ah, this is what I wasn't seeing.  For the above code, can you please
add a comment to this effect.

> > A group descriptor would almost be the right struct, but it doesn't
> > (yet?) include the GDT block count, itable block count, or a flag if
> > there is a superblock.  Adding these fields into the on-disk group_desc
> > struct wouldn't be a bad idea, then all the need to recalculate the
> > group layout in 10 places would disappear.
> 
> It's not worth managing the disk format change, nor the space on disk
> for all this information, IMHO.

Given that the kernel increasingly needs to be able to generate correct
bitmaps itself (uninit_bg, online resize) it wouldn't be a bad idea IMHO.
The 64-bit descriptor still has some reserved space in it...

On a related note, I'd like to "reserve" the 2 __u32 fields in the 32-bit
group descriptor for inode and block bitmap checksums (at least some
comments to that effect).

> Probably what we should do is have a new 64-bit version of
> ext2fs_super_and_bgd_loc() fill in a data structure which contains all
> of the calculated information, instead of in a series of pointers
> passed into the function, and then create a new function which given a
> block number and the said data structure, returns a boolean true/false
> value if the block is part of the metadata.

Yes, that seems reasonable.

> > A second function would take the above block numbers (possibly in the
> > form of a modified group descriptor) and return a populated block bitmap.
> 
> Yes, possibly; this will only be used by e2fsck and resize2fs, but
> it's probably worth factoring out the code.

Ideally mke2fs would be restructured to also use this, for consistency,
instead of the current implementation where it "allocates" the blocks
from the available blocks in the bitmap.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.