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

[David Sterba <dsterba@suse.cz>]

On Thu, Jun 13, 2024 at 09:40:57AM +0930, Qu Wenruo wrote:
> 在 2024/6/13 09:32, David Sterba 写道:
> > 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.
> 
> The error is from mke2fs, and considering how old the version is in CI,
> it may be e2fsprogs itself unable to properly detect the 16T file.
> 
> If we begin to start add checks on e2fsprogs, it's going to end up ugly...
> 
> Any good idea to go forward?
> Update CI (which seems impossible) or make the mke2fs part as mayfail
> and skip it?

I think using mayfail for mke2fs is the best option, I don't want to
check versions. The release of 1.46.5 is 2021-12-30, this is not that
old and likely widely used. Skipping the particular case is not a big
deal as it'll be covered on other testing instances.