| Message ID | 20200923171405.17456-1-marcos@mpdesouza.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: convert: Mention which reserve_space call failed | expand |
On Wed, Sep 23, 2020 at 1:44 PM Marcos Paulo de Souza <marcos@mpdesouza.com> wrote: > > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > btrfs-convert currently can't handle more fragmented block groups when > converting ext4 because the minimum size of a data chunk is 32Mb. > > When converting an ext4 fs with more fragmented block group and the disk > almost full, we can end up hitting a ENOSPC problem [1] since smaller > block groups (10Mb for example) end up being extended to 32Mb, leaving > the free space tree smaller when converting it to btrfs. > > This patch adds error messages telling which needed bytes couldn't be > allocated from the free space tree: > > create btrfs filesystem: > blocksize: 4096 > nodesize: 16384 > features: extref, skinny-metadata (default) > checksum: crc32c > free space report: > total: 1073741824 > free: 39124992 (3.64%) > ERROR: failed to reserve 33554432 bytes from free space for metadata chunk > ERROR: unable to create initial ctree: No space left on device > > Link: https://github.com/kdave/btrfs-progs/issues/251 > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > --- > convert/common.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/convert/common.c b/convert/common.c > index 048629df..6392e7f4 100644 > --- a/convert/common.c > +++ b/convert/common.c > @@ -812,8 +812,10 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg, > */ > ret = reserve_free_space(free_space, BTRFS_STRIPE_LEN, > &cfg->super_bytenr); > - if (ret < 0) > + if (ret < 0) { > + error("failed to reserve %d bytes from free space for temporary superblock", BTRFS_STRIPE_LEN); > goto out; > + } > > /* > * Then reserve system chunk space > @@ -823,12 +825,16 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg, > */ > ret = reserve_free_space(free_space, BTRFS_MKFS_SYSTEM_GROUP_SIZE, > &sys_chunk_start); > - if (ret < 0) > + if (ret < 0) { > + error("failed to reserve %d bytes from free space for system chunk", BTRFS_MKFS_SYSTEM_GROUP_SIZE); > goto out; > + } > ret = reserve_free_space(free_space, BTRFS_CONVERT_META_GROUP_SIZE, > &meta_chunk_start); > - if (ret < 0) > + if (ret < 0) { > + error("failed to reserve %d bytes from free space for metadata chunk", BTRFS_CONVERT_META_GROUP_SIZE); > goto out; > + } > > /* > * Allocated meta/sys chunks will be mapped 1:1 with device offset. > -- > 2.28.0 > Reviewed-by: Neal Gompa <ngompa13@gmail.com> -- 真実はいつも一つ！/ Always, there's only one truth!
On 2020/9/24 上午1:14, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > btrfs-convert currently can't handle more fragmented block groups when > converting ext4 because the minimum size of a data chunk is 32Mb. > > When converting an ext4 fs with more fragmented block group and the disk > almost full, we can end up hitting a ENOSPC problem [1] since smaller > block groups (10Mb for example) end up being extended to 32Mb, leaving > the free space tree smaller when converting it to btrfs. > > This patch adds error messages telling which needed bytes couldn't be > allocated from the free space tree: > > create btrfs filesystem: > blocksize: 4096 > nodesize: 16384 > features: extref, skinny-metadata (default) > checksum: crc32c > free space report: > total: 1073741824 > free: 39124992 (3.64%) > ERROR: failed to reserve 33554432 bytes from free space for metadata chunk > ERROR: unable to create initial ctree: No space left on device > > Link: https://github.com/kdave/btrfs-progs/issues/251 > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Looks pretty good, but can be enhanced a little, inlined below. Despite that, feel free to add my tag: Reviewed-by: Qu Wenruo <wqu@suse.com> > --- > convert/common.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/convert/common.c b/convert/common.c > index 048629df..6392e7f4 100644 > --- a/convert/common.c > +++ b/convert/common.c > @@ -812,8 +812,10 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg, > */ > ret = reserve_free_space(free_space, BTRFS_STRIPE_LEN, > &cfg->super_bytenr); > - if (ret < 0) > + if (ret < 0) { > + error("failed to reserve %d bytes from free space for temporary superblock", BTRFS_STRIPE_LEN); It would be awesome if we can output the free space. Just the largest portion is enough to show that we're hitting a real ENOSPC situation. Thanks, Qu > goto out; > + } > > /* > * Then reserve system chunk space > @@ -823,12 +825,16 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg, > */ > ret = reserve_free_space(free_space, BTRFS_MKFS_SYSTEM_GROUP_SIZE, > &sys_chunk_start); > - if (ret < 0) > + if (ret < 0) { > + error("failed to reserve %d bytes from free space for system chunk", BTRFS_MKFS_SYSTEM_GROUP_SIZE); > goto out; > + } > ret = reserve_free_space(free_space, BTRFS_CONVERT_META_GROUP_SIZE, > &meta_chunk_start); > - if (ret < 0) > + if (ret < 0) { > + error("failed to reserve %d bytes from free space for metadata chunk", BTRFS_CONVERT_META_GROUP_SIZE); > goto out; > + } > > /* > * Allocated meta/sys chunks will be mapped 1:1 with device offset. >
On Thu, 2020-09-24 at 08:08 +0800, Qu Wenruo wrote: > > On 2020/9/24 上午1:14, Marcos Paulo de Souza wrote: > > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > > > btrfs-convert currently can't handle more fragmented block groups > when > > converting ext4 because the minimum size of a data chunk is 32Mb. > > > > When converting an ext4 fs with more fragmented block group and the > disk > > almost full, we can end up hitting a ENOSPC problem [1] since > smaller > > block groups (10Mb for example) end up being extended to 32Mb, > leaving > > the free space tree smaller when converting it to btrfs. > > > > This patch adds error messages telling which needed bytes couldn't > be > > allocated from the free space tree: > > > > create btrfs filesystem: > > blocksize: 4096 > > nodesize: 16384 > > features: extref, skinny-metadata (default) > > checksum: crc32c > > free space report: > > total: 1073741824 > > free: 39124992 (3.64%) > > ERROR: failed to reserve 33554432 bytes from free space for > metadata chunk > > ERROR: unable to create initial ctree: No space left on device > > > > Link: https://github.com/kdave/btrfs-progs/issues/251 > > > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > > Looks pretty good, but can be enhanced a little, inlined below. > > Despite that, feel free to add my tag: > Reviewed-by: Qu Wenruo <wqu@suse.com> > > > --- > > convert/common.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/convert/common.c b/convert/common.c > > index 048629df..6392e7f4 100644 > > --- a/convert/common.c > > +++ b/convert/common.c > > @@ -812,8 +812,10 @@ int make_convert_btrfs(int fd, struct > btrfs_mkfs_config *cfg, > > */ > > ret = reserve_free_space(free_space, BTRFS_STRIPE_LEN, > > &cfg->super_bytenr); > > - if (ret < 0) > > + if (ret < 0) { > > + error("failed to reserve %d bytes from free space for > temporary superblock", BTRFS_STRIPE_LEN); > > It would be awesome if we can output the free space. > > Just the largest portion is enough to show that we're hitting a real > ENOSPC situation. Indeed, I'll send a v2 printing the free space tree when ENOSPC happens. > > Thanks, > Qu > > goto out; > > + } > > > > /* > > * Then reserve system chunk space > > @@ -823,12 +825,16 @@ int make_convert_btrfs(int fd, struct > btrfs_mkfs_config *cfg, > > */ > > ret = reserve_free_space(free_space, > BTRFS_MKFS_SYSTEM_GROUP_SIZE, > > &sys_chunk_start); > > - if (ret < 0) > > + if (ret < 0) { > > + error("failed to reserve %d bytes from free space for > system chunk", BTRFS_MKFS_SYSTEM_GROUP_SIZE); > > goto out; > > + } > > ret = reserve_free_space(free_space, > BTRFS_CONVERT_META_GROUP_SIZE, > > &meta_chunk_start); > > - if (ret < 0) > > + if (ret < 0) { > > + error("failed to reserve %d bytes from free space for > metadata chunk", BTRFS_CONVERT_META_GROUP_SIZE); > > goto out; > > + } > > > > /* > > * Allocated meta/sys chunks will be mapped 1:1 with device > offset. > > >
On 2020/9/24 下午7:54, Marcos Paulo de Souza wrote: > On Thu, 2020-09-24 at 08:08 +0800, Qu Wenruo wrote: >> >> On 2020/9/24 上午1:14, Marcos Paulo de Souza wrote: >>> From: Marcos Paulo de Souza <mpdesouza@suse.com> >>> >>> btrfs-convert currently can't handle more fragmented block groups >> when >>> converting ext4 because the minimum size of a data chunk is 32Mb. >>> >>> When converting an ext4 fs with more fragmented block group and the >> disk >>> almost full, we can end up hitting a ENOSPC problem [1] since >> smaller >>> block groups (10Mb for example) end up being extended to 32Mb, >> leaving >>> the free space tree smaller when converting it to btrfs. >>> >>> This patch adds error messages telling which needed bytes couldn't >> be >>> allocated from the free space tree: >>> >>> create btrfs filesystem: >>> blocksize: 4096 >>> nodesize: 16384 >>> features: extref, skinny-metadata (default) >>> checksum: crc32c >>> free space report: >>> total: 1073741824 >>> free: 39124992 (3.64%) >>> ERROR: failed to reserve 33554432 bytes from free space for >> metadata chunk >>> ERROR: unable to create initial ctree: No space left on device >>> >>> Link: https://github.com/kdave/btrfs-progs/issues/251 >>> >>> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> >> >> Looks pretty good, but can be enhanced a little, inlined below. >> >> Despite that, feel free to add my tag: >> Reviewed-by: Qu Wenruo <wqu@suse.com> >> >>> --- >>> convert/common.c | 12 +++++++++--- >>> 1 file changed, 9 insertions(+), 3 deletions(-) >>> >>> diff --git a/convert/common.c b/convert/common.c >>> index 048629df..6392e7f4 100644 >>> --- a/convert/common.c >>> +++ b/convert/common.c >>> @@ -812,8 +812,10 @@ int make_convert_btrfs(int fd, struct >> btrfs_mkfs_config *cfg, >>> */ >>> ret = reserve_free_space(free_space, BTRFS_STRIPE_LEN, >>> &cfg->super_bytenr); >>> - if (ret < 0) >>> + if (ret < 0) { >>> + error("failed to reserve %d bytes from free space for >> temporary superblock", BTRFS_STRIPE_LEN); >> >> It would be awesome if we can output the free space. >> >> Just the largest portion is enough to show that we're hitting a real >> ENOSPC situation. > > Indeed, I'll send a v2 printing the free space tree when ENOSPC > happens. And it would be even better to mention the fragmentation problem in the man page for btrfs-convert. The fragmentation problem is a little too complex to explain in the error message nor usage. Although I guess the man page update could be another patch. Thanks, Qu > >> >> Thanks, >> Qu >>> goto out; >>> + } >>> >>> /* >>> * Then reserve system chunk space >>> @@ -823,12 +825,16 @@ int make_convert_btrfs(int fd, struct >> btrfs_mkfs_config *cfg, >>> */ >>> ret = reserve_free_space(free_space, >> BTRFS_MKFS_SYSTEM_GROUP_SIZE, >>> &sys_chunk_start); >>> - if (ret < 0) >>> + if (ret < 0) { >>> + error("failed to reserve %d bytes from free space for >> system chunk", BTRFS_MKFS_SYSTEM_GROUP_SIZE); >>> goto out; >>> + } >>> ret = reserve_free_space(free_space, >> BTRFS_CONVERT_META_GROUP_SIZE, >>> &meta_chunk_start); >>> - if (ret < 0) >>> + if (ret < 0) { >>> + error("failed to reserve %d bytes from free space for >> metadata chunk", BTRFS_CONVERT_META_GROUP_SIZE); >>> goto out; >>> + } >>> >>> /* >>> * Allocated meta/sys chunks will be mapped 1:1 with device >> offset. >>> >> >
diff --git a/convert/common.c b/convert/common.c index 048629df..6392e7f4 100644 --- a/convert/common.c +++ b/convert/common.c @@ -812,8 +812,10 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg, */ ret = reserve_free_space(free_space, BTRFS_STRIPE_LEN, &cfg->super_bytenr); - if (ret < 0) + if (ret < 0) { + error("failed to reserve %d bytes from free space for temporary superblock", BTRFS_STRIPE_LEN); goto out; + } /* * Then reserve system chunk space @@ -823,12 +825,16 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg, */ ret = reserve_free_space(free_space, BTRFS_MKFS_SYSTEM_GROUP_SIZE, &sys_chunk_start); - if (ret < 0) + if (ret < 0) { + error("failed to reserve %d bytes from free space for system chunk", BTRFS_MKFS_SYSTEM_GROUP_SIZE); goto out; + } ret = reserve_free_space(free_space, BTRFS_CONVERT_META_GROUP_SIZE, &meta_chunk_start); - if (ret < 0) + if (ret < 0) { + error("failed to reserve %d bytes from free space for metadata chunk", BTRFS_CONVERT_META_GROUP_SIZE); goto out; + } /* * Allocated meta/sys chunks will be mapped 1:1 with device offset.