Re: [PATCH] btrfs-progs: convert: Mention which reserve_space call failed

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2878 bytes --]



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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]