On Jun 17, 2008  12:30 -0400, Theodore Ts'o wrote:

On Tue, Jun 17, 2008 at 09:16:47AM -0600, Andreas Dilger wrote:

quoted

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.

quoted

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.

quoted

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.