Re: [PATCH] btrfs-progs: filesystem-resize: make output more readable

[David Sterba <dsterba@suse.cz>]

On Wed, Dec 16, 2020 at 03:42:40AM +0000, Sidong Yang wrote:
> This patch make output of filesystem-resize command more readable and
> give detail information for users. This patch provides more information
> about filesystem like below.
> 
> Before:
> Resize '/mnt' of '1:-1G'
> 
> After:
> Resize device id 1 (/dev/vdb) from 4.00GiB to 3.00GiB
> 
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
>  cmds/filesystem.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index fac612b2..53e775b7 100644
> --- a/cmds/filesystem.c
> +++ b/cmds/filesystem.c
> @@ -1084,6 +1084,14 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
>  	int ret;
>  	struct stat st;
>  	bool enqueue = false;
> +	struct btrfs_ioctl_fs_info_args fi_args;
> +	struct btrfs_ioctl_dev_info_args *di_args = NULL;
> +	char newsize[256];
> +	char sign;
> +	u64 inc_bytes;
> +	u64 res_bytes;
> +	int i, devid, dev_idx;
> +	const char *res_str;
>  
>  	optind = 0;
>  	while (1) {
> @@ -1142,7 +1150,58 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
>  		return 1;
>  	}
>  
> -	printf("Resize '%s' of '%s'\n", path, amount);
> +	ret = get_fs_info(path, &fi_args, &di_args);
> +	if (ret)
> +		error("unable to retrieve fs info");

The helper 'error' is to just print the message so the code has to
change flow to an exit otherwise it would continue, which is what we
don't want here.

> +
> +	if (!fi_args.num_devices)
> +		error("num_devices = 0");

Same and everywhere below. Also the error message is too cryptic, think
that there's a human reading that so it should say what's the error,
like "No devices found". Which would be a weird and likely impossible
error anyway but it's good that it's handled.

> +
> +	ret = sscanf(amount, "%d:%255s", &devid, newsize);
> +
> +	if (ret != 2)
> +		error("invalid format");

I'm not sure this covers all the possibilities the resize format
provides. The "%d:" part is not mandatory and there doesn't need to be
":" at all, eg when it's "max" or any number.

There are some examples in manual page of btrfs-filesystem so would be
good if we have at least that covered by tests.