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

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

[Sidong Yang]

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");
+
+	if (!fi_args.num_devices)
+		error("num_devices = 0");
+
+	ret = sscanf(amount, "%d:%255s", &devid, newsize);
+
+	if (ret != 2)
+		error("invalid format");
+
+	dev_idx = -1;
+	for(i = 0; i < fi_args.num_devices; i++) {
+		if (di_args[i].devid == devid) {
+			dev_idx = i;
+			break;
+		}
+	}
+
+	if (dev_idx < 0)
+		error("cannot find devid : %d", devid);
+
+	if (!strcmp(newsize, "max")) {
+		res_str = "max";
+	} else {
+		if (strlen(newsize) < 3)
+			error("invalid format");
+
+		sign = newsize[0];
+		if (sign != '-' && sign != '+')
+			error("invalid format");
+
+		inc_bytes = parse_size(&newsize[1]);
+
+		res_bytes = 0;
+		if (sign == '+')
+			res_bytes = di_args[0].total_bytes + inc_bytes;
+		else if (sign == '-')
+			res_bytes = di_args[0].total_bytes - inc_bytes;
+		else
+			error("invalid format");
+
+		res_str = pretty_size_mode(res_bytes, UNITS_DEFAULT);
+	}
+
+	printf("Resize device id %d (%s) from %s to %s\n", devid, di_args[dev_idx].path,
+		   pretty_size_mode(di_args[dev_idx].total_bytes, UNITS_DEFAULT),
+		   res_str);
+
+	free(di_args);
+
 	memset(&args, 0, sizeof(args));
 	strncpy_null(args.name, amount);
 	res = ioctl(fd, BTRFS_IOC_RESIZE, &args);
-- 
2.25.1

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

[David Sterba]

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.

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

[Sidong Yang]

On Fri, Jan 15, 2021 at 12:48:59AM +0100, David Sterba wrote:

Hi David, Thanks fore review.

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

Thanks, I didn't know about error(). I'll fix it.
> 
> > +
> > +	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.

I have to fix the error messages for users to understand what's the error.

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

Okay, I'll make it to handle various cases.

Thanks!
Sidong