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