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

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

[Srivathsa Dara]

In ext4, number of blocks can be greater than 2^32. Therefore, if
btrfs-convert is used on filesystems greater than 16TiB (Staring from
16TiB, number of blocks overflow 32 bits), it fails to convert.

Example:

Here, /dev/sdc1 is 16TiB partition intitialized with an ext4 filesystem.

[root@rasivara-arm2 opc]# btrfs-convert -d -p /dev/sdc1
btrfs-convert from btrfs-progs v5.15.1

convert/main.c:1164: do_convert: Assertion `cctx.total_bytes != 0` failed, value 0
btrfs-convert(+0xfd04)[0xaaaaba44fd04]
btrfs-convert(main+0x258)[0xaaaaba44d278]
/lib64/libc.so.6(__libc_start_main+0xdc)[0xffffb962777c]
btrfs-convert(+0xd4fc)[0xaaaaba44d4fc]
Aborted (core dumped)

Fix it by considering 64 bit block numbers.

Signed-off-by: Srivathsa Dara <srivathsa.d.dara@oracle.com>
---
 convert/source-ext2.c | 6 +++---
 convert/source-ext2.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/convert/source-ext2.c b/convert/source-ext2.c
index 2186b252..afa48606 100644
--- a/convert/source-ext2.c
+++ b/convert/source-ext2.c
@@ -288,8 +288,8 @@ error:
 	return -1;
 }
 
-static int ext2_block_iterate_proc(ext2_filsys fs, blk_t *blocknr,
-			        e2_blkcnt_t blockcnt, blk_t ref_block,
+static int ext2_block_iterate_proc(ext2_filsys fs, blk64_t *blocknr,
+			        e2_blkcnt_t blockcnt, blk64_t ref_block,
 			        int ref_offset, void *priv_data)
 {
 	int ret;
@@ -323,7 +323,7 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
 	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
 			convert_flags & CONVERT_FLAG_DATACSUM);
 
-	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
+	err = ext2fs_block_iterate3(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
 				    NULL, ext2_block_iterate_proc, &data);
 	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)
 #define EXT2FS_CLUSTER_RATIO(fs)	(1)
 #define EXT2_CLUSTERS_PER_GROUP(s)	(EXT2_BLOCKS_PER_GROUP(s))
 #define EXT2FS_B2C(fs, blk)		(blk)
-- 
2.39.3

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

[Qu Wenruo]

在 2024/6/6 19:52, Srivathsa Dara 写道:
> In ext4, number of blocks can be greater than 2^32. Therefore, if
> btrfs-convert is used on filesystems greater than 16TiB (Staring from
> 16TiB, number of blocks overflow 32 bits), it fails to convert.
>
> Example:
>
> Here, /dev/sdc1 is 16TiB partition intitialized with an ext4 filesystem.
>
> [root@rasivara-arm2 opc]# btrfs-convert -d -p /dev/sdc1
> btrfs-convert from btrfs-progs v5.15.1
>
> convert/main.c:1164: do_convert: Assertion `cctx.total_bytes != 0` failed, value 0
> btrfs-convert(+0xfd04)[0xaaaaba44fd04]
> btrfs-convert(main+0x258)[0xaaaaba44d278]
> /lib64/libc.so.6(__libc_start_main+0xdc)[0xffffb962777c]
> btrfs-convert(+0xd4fc)[0xaaaaba44d4fc]
> Aborted (core dumped)
>
> Fix it by considering 64 bit block numbers.
>
> Signed-off-by: Srivathsa Dara <srivathsa.d.dara@oracle.com>
> ---
>   convert/source-ext2.c | 6 +++---
>   convert/source-ext2.h | 2 +-
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/convert/source-ext2.c b/convert/source-ext2.c
> index 2186b252..afa48606 100644
> --- a/convert/source-ext2.c
> +++ b/convert/source-ext2.c
> @@ -288,8 +288,8 @@ error:
>   	return -1;
>   }
>
> -static int ext2_block_iterate_proc(ext2_filsys fs, blk_t *blocknr,
> -			        e2_blkcnt_t blockcnt, blk_t ref_block,
> +static int ext2_block_iterate_proc(ext2_filsys fs, blk64_t *blocknr,
> +			        e2_blkcnt_t blockcnt, blk64_t ref_block,
>   			        int ref_offset, void *priv_data)
>   {
>   	int ret;
> @@ -323,7 +323,7 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
>   	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
>   			convert_flags & CONVERT_FLAG_DATACSUM);
>
> -	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
> +	err = ext2fs_block_iterate3(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
>   				    NULL, ext2_block_iterate_proc, &data);

I'm wondering does ext2 really supports 64bit block number.

For ext* fs with extent support (3 and 4), we're no longer utilizing
ext2fs_block_iterate2(), instead we go with iterate_file_extents()
instead, and that function is already using blk64_t for both file offset
and the block number.

I'm guessing the code base doesn't have the latest c23e068aaf91
("btrfs-progs: convert: rework file extent iteration to handle unwritten
extents") commit yet?


>   	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.

So I'm afraid you're testing an older version of btrfs-progs.

Thanks,
Qu
>   #define EXT2FS_CLUSTER_RATIO(fs)	(1)
>   #define EXT2_CLUSTERS_PER_GROUP(s)	(EXT2_BLOCKS_PER_GROUP(s))
>   #define EXT2FS_B2C(fs, blk)		(blk)

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

[Srivathsa Dara]

> 在 2024/6/6 19:52, Srivathsa Dara 写道:
> > In ext4, number of blocks can be greater than 2^32. Therefore, if 
> > btrfs-convert is used on filesystems greater than 16TiB (Staring from 
> > 16TiB, number of blocks overflow 32 bits), it fails to convert.
> >
> > Example:
> >
> > Here, /dev/sdc1 is 16TiB partition intitialized with an ext4 filesystem.
> >
> > [root@rasivara-arm2 opc]# btrfs-convert -d -p /dev/sdc1 btrfs-convert 
> > from btrfs-progs v5.15.1
> >
> > convert/main.c:1164: do_convert: Assertion `cctx.total_bytes != 0` 
> > failed, value 0 btrfs-convert(+0xfd04)[0xaaaaba44fd04]
> > btrfs-convert(main+0x258)[0xaaaaba44d278]
> > /lib64/libc.so.6(__libc_start_main+0xdc)[0xffffb962777c]
> > btrfs-convert(+0xd4fc)[0xaaaaba44d4fc]
> > Aborted (core dumped)
> >
> > Fix it by considering 64 bit block numbers.
> >
> > Signed-off-by: Srivathsa Dara <srivathsa.d.dara@oracle.com>
> > ---
> >   convert/source-ext2.c | 6 +++---
> >   convert/source-ext2.h | 2 +-
> >   2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/convert/source-ext2.c b/convert/source-ext2.c index 
> > 2186b252..afa48606 100644
> > --- a/convert/source-ext2.c
> > +++ b/convert/source-ext2.c
> > @@ -288,8 +288,8 @@ error:
> >   	return -1;
> >   }
> >
> > -static int ext2_block_iterate_proc(ext2_filsys fs, blk_t *blocknr,
> > -			        e2_blkcnt_t blockcnt, blk_t ref_block,
> > +static int ext2_block_iterate_proc(ext2_filsys fs, blk64_t *blocknr,
> > +			        e2_blkcnt_t blockcnt, blk64_t ref_block,
> >   			        int ref_offset, void *priv_data)
> >   {
> >   	int ret;
> > @@ -323,7 +323,7 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
> >   	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
> >   			convert_flags & CONVERT_FLAG_DATACSUM);
> >
> > -	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
> > +	err = ext2fs_block_iterate3(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
> >   				    NULL, ext2_block_iterate_proc, &data);
> 
> I'm wondering does ext2 really supports 64bit block number.

No, it doesn't.

> 
> For ext* fs with extent support (3 and 4), we're no longer utilizing ext2fs_block_iterate2(), instead we go with iterate_file_extents() instead, and that function is already using blk64_t for both file offset and the block number.
> 
> I'm guessing the code base doesn't have the latest c23e068aaf91
> ("btrfs-progs: convert: rework file extent iteration to handle unwritten
> extents") commit yet?

I've used btrfs-progs-6.8.1, it doesn't have this commit.

> 
> 
> >   	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.

Thanks for informing,
Srivathsa

> 
> So I'm afraid you're testing an older version of btrfs-progs.
> 
> Thanks,
> Qu
> >   #define EXT2FS_CLUSTER_RATIO(fs)	(1)
> >   #define EXT2_CLUSTERS_PER_GROUP(s)	(EXT2_BLOCKS_PER_GROUP(s))
> >   #define EXT2FS_B2C(fs, blk)		(blk)

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

[David Sterba]

On Tue, Jun 11, 2024 at 06:03:30AM +0000, Srivathsa Dara wrote:
> > 在 2024/6/6 19:52, Srivathsa Dara 写道:
> > > In ext4, number of blocks can be greater than 2^32. Therefore, if 
> > > btrfs-convert is used on filesystems greater than 16TiB (Staring from 
> > > 16TiB, number of blocks overflow 32 bits), it fails to convert.
> > >
> > > Example:
> > >
> > > Here, /dev/sdc1 is 16TiB partition intitialized with an ext4 filesystem.
> > >
> > > [root@rasivara-arm2 opc]# btrfs-convert -d -p /dev/sdc1 btrfs-convert 
> > > from btrfs-progs v5.15.1
> > >
> > > convert/main.c:1164: do_convert: Assertion `cctx.total_bytes != 0` 
> > > failed, value 0 btrfs-convert(+0xfd04)[0xaaaaba44fd04]
> > > btrfs-convert(main+0x258)[0xaaaaba44d278]
> > > /lib64/libc.so.6(__libc_start_main+0xdc)[0xffffb962777c]
> > > btrfs-convert(+0xd4fc)[0xaaaaba44d4fc]
> > > Aborted (core dumped)
> > >
> > > Fix it by considering 64 bit block numbers.
> > >
> > > Signed-off-by: Srivathsa Dara <srivathsa.d.dara@oracle.com>
> > > ---
> > >   convert/source-ext2.c | 6 +++---
> > >   convert/source-ext2.h | 2 +-
> > >   2 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/convert/source-ext2.c b/convert/source-ext2.c index 
> > > 2186b252..afa48606 100644
> > > --- a/convert/source-ext2.c
> > > +++ b/convert/source-ext2.c
> > > @@ -288,8 +288,8 @@ error:
> > >   	return -1;
> > >   }
> > >
> > > -static int ext2_block_iterate_proc(ext2_filsys fs, blk_t *blocknr,
> > > -			        e2_blkcnt_t blockcnt, blk_t ref_block,
> > > +static int ext2_block_iterate_proc(ext2_filsys fs, blk64_t *blocknr,
> > > +			        e2_blkcnt_t blockcnt, blk64_t ref_block,
> > >   			        int ref_offset, void *priv_data)
> > >   {
> > >   	int ret;
> > > @@ -323,7 +323,7 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
> > >   	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
> > >   			convert_flags & CONVERT_FLAG_DATACSUM);
> > >
> > > -	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
> > > +	err = ext2fs_block_iterate3(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
> > >   				    NULL, ext2_block_iterate_proc, &data);
> > 
> > I'm wondering does ext2 really supports 64bit block number.
> 
> No, it doesn't.
> 
> > 
> > For ext* fs with extent support (3 and 4), we're no longer utilizing ext2fs_block_iterate2(), instead we go with iterate_file_extents() instead, and that function is already using blk64_t for both file offset and the block number.
> > 
> > I'm guessing the code base doesn't have the latest c23e068aaf91
> > ("btrfs-progs: convert: rework file extent iteration to handle unwritten
> > extents") commit yet?
> 
> I've used btrfs-progs-6.8.1, it doesn't have this commit.
> 
> > 
> > 
> > >   	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.

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

[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.

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.

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

[Qu Wenruo]

在 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?

Thanks,
Qu
>
> 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.
>

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

[David Sterba]

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.

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

[Qu Wenruo]

在 2024/6/13 09:53, David Sterba 写道:
[...]
>>
>> 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.

Sounds good to me.

Just add an extra comment on the situation.

Although I really hope github CI can someday updates its infrastructures.

Thanks,
Qu

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

[David Sterba]

On Thu, Jun 13, 2024 at 09:55:56AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2024/6/13 09:53, David Sterba 写道:
> [...]
> >>
> >> 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.
> 
> Sounds good to me.
> 
> Just add an extra comment on the situation.
> 
> Although I really hope github CI can someday updates its infrastructures.

The updates happen from time to time, following Ubuntu LTS releases. The
upstream repository is https://github.com/actions/runner-images.

Now that I'm looking there it seems that 24.04 has been made available a
month ago but has to be selected explicitly, as ubuntu-latest is still
pointing to 22.

Kernel version is 6.8 and e2fsprogs is 1.47 so the update can fix that
and we don't have to do the workarounds. I'll do a test and update the
CI files if everythings works.

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

[David Sterba]

On Thu, Jun 13, 2024 at 02:52:55AM +0200, David Sterba wrote:
> On Thu, Jun 13, 2024 at 09:55:56AM +0930, Qu Wenruo wrote:
> > 
> > 
> > 在 2024/6/13 09:53, David Sterba 写道:
> > [...]
> > >>
> > >> 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.
> > 
> > Sounds good to me.
> > 
> > Just add an extra comment on the situation.
> > 
> > Although I really hope github CI can someday updates its infrastructures.
> 
> The updates happen from time to time, following Ubuntu LTS releases. The
> upstream repository is https://github.com/actions/runner-images.
> 
> Now that I'm looking there it seems that 24.04 has been made available a
> month ago but has to be selected explicitly, as ubuntu-latest is still
> pointing to 22.
> 
> Kernel version is 6.8 and e2fsprogs is 1.47 so the update can fix that
> and we don't have to do the workarounds. I'll do a test and update the
> CI files if everythings works.

It still fails with 1.47.0:

https://github.com/kdave/btrfs-progs/actions/runs/9492025265/job/26158483515

mke2fs 1.47.0 (5-Feb-2023)
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.

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

[Srivathsa Dara]

> -----Original Message-----
> From: David Sterba <dsterba@suse.cz> 
> Sent: Thursday, June 13, 2024 6:53 AM
> To: David Sterba <dsterba@suse.cz>
> Cc: Qu Wenruo <quwenruo.btrfs@gmx.com>; Srivathsa Dara <srivathsa.d.dara@oracle.com>; linux-btrfs@vger.kernel.org; Rajesh Sivaramasubramaniom <rajesh.sivaramasubramaniom@oracle.com>; Junxiao Bi <junxiao.bi@oracle.com>; clm@fb.com; josef@toxicpanda.com; dsterba@suse.com
> Subject: [External] : Re: [PATCH v2] btrfs-progs: convert: Add 64 bit block numbers support
> 
> On Thu, Jun 13, 2024 at 02:52:55AM +0200, David Sterba wrote:
> > On Thu, Jun 13, 2024 at 09:55:56AM +0930, Qu Wenruo wrote:
> > > 
> > > 
> > > 在 2024/6/13 09:53, David Sterba 写道:
> > > [...]
> > > >>
> > > >> 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.
> > > 
> > > Sounds good to me.
> > > 
> > > Just add an extra comment on the situation.
> > > 
> > > Although I really hope github CI can someday updates its infrastructures.
> > 
> > The updates happen from time to time, following Ubuntu LTS releases. 
> > The upstream repository is https://urldefense.com/v3/__https://github.com/actions/runner-images__;!!ACWV5N9M2RV99hQ!IfrPyGUqBrz7hnrHbZo-tCuxnrJgJ7llG538QEXxcUVhZ5looibcBIHmx9d5QMKcO4hRFXmi4zWNmShacnZfig$ .
> > 
> > Now that I'm looking there it seems that 24.04 has been made available 
> > a month ago but has to be selected explicitly, as ubuntu-latest is 
> > still pointing to 22.
> > 
> > Kernel version is 6.8 and e2fsprogs is 1.47 so the update can fix that 
> > and we don't have to do the workarounds. I'll do a test and update the 
> > CI files if everythings works.
> 
> It still fails with 1.47.0:
> 
> https://urldefense.com/v3/__https://github.com/kdave/btrfs-progs/actions/runs/9492025265/job/26158483515__;!!ACWV5N9M2RV99hQ!IfrPyGUqBrz7hnrHbZo-tCuxnrJgJ7llG538QEXxcUVhZ5looibcBIHmx9d5QMKcO4hRFXmi4zWNmShgMsVkbA$ 
> 
> mke2fs 1.47.0 (5-Feb-2023)
> 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.

I encountered the same error in one of my setups. I don't think the error is due to mke2fs,
truncate might have failed to resize test.img to 16T, which caused mke2fs to fail.

```
[root@sridara-s tests]# truncate -s 16T test.img
truncate: failed to truncate 'test.img' at 17592186044416 bytes: File too large
[root@sridara-s tests]# ls -lrth test.img
-rw-r--r--. 1 root www 0 Jun 10 06:19 test.img
[root@sridara-s tests]# mkfs.ext4 test.img
mke2fs 1.45.6 (20-Mar-2020)
mkfs.ext4: 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.
```

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

[Srivathsa Dara]

> -----Original Message-----
> From: David Sterba <dsterba@suse.cz> 
> Sent: Thursday, June 13, 2024 2:08 AM
> To: Srivathsa Dara <srivathsa.d.dara@oracle.com>
> Cc: Qu Wenruo <quwenruo.btrfs@gmx.com>; linux-btrfs@vger.kernel.org; Rajesh Sivaramasubramaniom <rajesh.sivaramasubramaniom@oracle.com>; Junxiao Bi <junxiao.bi@oracle.com>; clm@fb.com; josef@toxicpanda.com; dsterba@suse.com
> Subject: [External] : Re: [PATCH v2] btrfs-progs: convert: Add 64 bit block numbers support
> 
> On Tue, Jun 11, 2024 at 06:03:30AM +0000, Srivathsa Dara wrote:
> > > 在 2024/6/6 19:52, Srivathsa Dara 写道:
> > > > In ext4, number of blocks can be greater than 2^32. Therefore, if 
> > > > btrfs-convert is used on filesystems greater than 16TiB (Staring 
> > > > from 16TiB, number of blocks overflow 32 bits), it fails to convert.
> > > >
> > > > Example:
> > > >
> > > > Here, /dev/sdc1 is 16TiB partition intitialized with an ext4 filesystem.
> > > >
> > > > [root@rasivara-arm2 opc]# btrfs-convert -d -p /dev/sdc1 
> > > > btrfs-convert from btrfs-progs v5.15.1
> > > >
> > > > convert/main.c:1164: do_convert: Assertion `cctx.total_bytes != 0` 
> > > > failed, value 0 btrfs-convert(+0xfd04)[0xaaaaba44fd04]
> > > > btrfs-convert(main+0x258)[0xaaaaba44d278]
> > > > /lib64/libc.so.6(__libc_start_main+0xdc)[0xffffb962777c]
> > > > btrfs-convert(+0xd4fc)[0xaaaaba44d4fc]
> > > > Aborted (core dumped)
> > > >
> > > > Fix it by considering 64 bit block numbers.
> > > >
> > > > Signed-off-by: Srivathsa Dara <srivathsa.d.dara@oracle.com>
> > > > ---
> > > >   convert/source-ext2.c | 6 +++---
> > > >   convert/source-ext2.h | 2 +-
> > > >   2 files changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/convert/source-ext2.c b/convert/source-ext2.c index
> > > > 2186b252..afa48606 100644
> > > > --- a/convert/source-ext2.c
> > > > +++ b/convert/source-ext2.c
> > > > @@ -288,8 +288,8 @@ error:
> > > >   	return -1;
> > > >   }
> > > >
> > > > -static int ext2_block_iterate_proc(ext2_filsys fs, blk_t *blocknr,
> > > > -			        e2_blkcnt_t blockcnt, blk_t ref_block,
> > > > +static int ext2_block_iterate_proc(ext2_filsys fs, blk64_t *blocknr,
> > > > +			        e2_blkcnt_t blockcnt, blk64_t ref_block,
> > > >   			        int ref_offset, void *priv_data)
> > > >   {
> > > >   	int ret;
> > > > @@ -323,7 +323,7 @@ static int ext2_create_file_extents(struct btrfs_trans_handle *trans,
> > > >   	init_blk_iterate_data(&data, trans, root, btrfs_inode, objectid,
> > > >   			convert_flags & CONVERT_FLAG_DATACSUM);
> > > >
> > > > -	err = ext2fs_block_iterate2(ext2_fs, ext2_ino, BLOCK_FLAG_DATA_ONLY,
> > > > +	err = ext2fs_block_iterate3(ext2_fs, ext2_ino, 
> > > > +BLOCK_FLAG_DATA_ONLY,
> > > >   				    NULL, ext2_block_iterate_proc, &data);
> > > 
> > > I'm wondering does ext2 really supports 64bit block number.
> > 
> > No, it doesn't.
> > 
> > > 
> > > For ext* fs with extent support (3 and 4), we're no longer utilizing ext2fs_block_iterate2(), instead we go with iterate_file_extents() instead, and that function is already using blk64_t for both file offset and the block number.
> > > 
> > > I'm guessing the code base doesn't have the latest c23e068aaf91
> > > ("btrfs-progs: convert: rework file extent iteration to handle 
> > > unwritten
> > > extents") commit yet?
> > 
> > I've used btrfs-progs-6.8.1, it doesn't have this commit.
> > 
> > > 
> > > 
> > > >   	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.

Qu's patch fixes this issue and the unwritten extents issue as well.
Although my patch is simple, it fails to address the unwritten extents
issue. Therefore, you can ignore this patch.