| Message ID | 20240606102215.3695032-1-srivathsa.d.dara@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] btrfs-progs: convert: Add 64 bit block numbers support | expand |
在 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)
> 在 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)
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.
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.
在 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. >
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.
在 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
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.
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.
> -----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. ```
> -----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.
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)
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(-)