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

[Marcos Paulo de Souza <marcos@mpdesouza.com>]

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