Re: [PATCH v2] btrfs-progs: convert: Add 64 bit block numbers support

[David Sterba <dsterba@suse.cz>]

On Wed, Jun 12, 2024 at 10:37:43PM +0200, David Sterba wrote:
> On Tue, Jun 11, 2024 at 06:03:30AM +0000, Srivathsa Dara wrote:
> > > 在 2024/6/6 19:52, Srivathsa Dara 写道:
> > > >   	if (err)
> > > >   		goto error;
> > > > diff --git a/convert/source-ext2.h b/convert/source-ext2.h index 
> > > > d204aac5..62c9b1fa 100644
> > > > --- a/convert/source-ext2.h
> > > > +++ b/convert/source-ext2.h
> > > > @@ -46,7 +46,7 @@ struct btrfs_trans_handle;
> > > >   #define ext2fs_get_block_bitmap_range2 ext2fs_get_block_bitmap_range
> > > >   #define ext2fs_inode_data_blocks2 ext2fs_inode_data_blocks
> > > >   #define ext2fs_read_ext_attr2 ext2fs_read_ext_attr
> > > > -#define ext2fs_blocks_count(s)		((s)->s_blocks_count)
> > > > +#define ext2fs_blocks_count(s)		(((s)->s_blocks_count_hi << 32) | (s)->s_blocks_count)
> > > 
> > > This is definitely needed, or it would trigger the ASSERT().
> > > 
> > > But again, the newer btrfs-progs no longer go with internally defined ext2fs_blocks_count(), but using the one from e2fsprogs headers, and the library version is already returning blk64_t.
> > 
> > Okay, got it.
> > 
> > Tested the code base with the commit c23e068aaf91, it does handle 64 bit block numbers.
> 
> So, is this patch still needed? I'm not sure after Qu fixed the
> iteration, the tests pass without the patch too.

Locally the test succeeds (e2fsprogs 1.47.0), however in the github CI
it fails with:

mke2fs -t ext4 -b 4096 -F /home/runner/work/btrfs-progs/btrfs-progs/tests/test.img
mke2fs 1.46.5 (30-Dec-2021)
mke2fs: Device size reported to be zero.  Invalid partition specified, or
	partition table wasn't reread after running fdisk, due to
	a modified partition being busy and in use.  You may need to reboot
	to re-read your partition table.

This is the updated test case 018-fs-size-overflow and in the code the change
is the updated definition of ext2fs_blocks_count.

I'll put the test case to a topic branch and remove it from devel so the
CI can be used. You can open a pull request to verify if things work.