* [PATCH] btrfs-progs: fix btrfs resize failed.
@ 2022-07-23 12:02 Li Zhang
2022-07-23 23:21 ` Qu Wenruo
0 siblings, 1 reply; 3+ messages in thread
From: Li Zhang @ 2022-07-23 12:02 UTC (permalink / raw)
To: linux-btrfs; +Cc: Li Zhang
related issuse:
https://github.com/kdave/btrfs-progs/issues/470
V1 link:
https://www.spinics.net/lists/linux-btrfs/msg126661.html
[BUG]
If there is no devid=1, when the user uses the btrfs file system tool,
the following error will be reported,
$ sudo btrfs filesystem show /mnt/1
Label: none uuid: 64dc0f68-9afa-4465-9ea1-2bbebfdb6cec
Total devices 2 FS bytes used 704.00KiB
devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
$ sudo btrfs filesystem resize -1G /mnt/1
ERROR: cannot find devid: 1
ERROR: unable to resize '/mnt/1': No such device
[CAUSE]
If the user does not specify the devid id explicitly,
btrfs will use the default devid 1, so it will report an error when dev 1 is missing.
[FIX]
If the file system contains multiple devices, output an error message to the user.
If the filesystem has only one device, resize should automatically add the unique devid.
[RESULT]
$ sudo btrfs filesystem show /mnt/1/
Label: none uuid: 2025e6ae-0b6d-40b4-8685-3e7e9fc9b2c2
Total devices 2 FS bytes used 144.00KiB
devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
$ sudo btrfs filesystem resize -1G /mnt/1
ERROR: The file system has multiple devices, please specify devid exactly.
ERROR: The device information list is as follows.
devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
$ sudo btrfs device delete 2 /mnt/1/
$ sudo btrfs filesystem show /mnt/1/
Label: none uuid: 2025e6ae-0b6d-40b4-8685-3e7e9fc9b2c2
Total devices 1 FS bytes used 144.00KiB
devid 3 size 15.00GiB used 1.28GiB path /dev/loop3
$ sudo btrfs filesystem resize -1G /mnt/1
Resize device id 3 (/dev/loop3) from 15.00GiB to 14.00GiB
Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
---
cmds/filesystem.c | 63 ++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 51 insertions(+), 12 deletions(-)
diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 7cd08fc..c26ba2b 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -1087,7 +1087,8 @@ static const char * const cmd_filesystem_resize_usage[] = {
NULL
};
-static int check_resize_args(const char *amount, const char *path) {
+static int check_resize_args(char * const amount, const char *path)
+{
struct btrfs_ioctl_fs_info_args fi_args;
struct btrfs_ioctl_dev_info_args *di_args = NULL;
int ret, i, dev_idx = -1;
@@ -1102,7 +1103,8 @@ static int check_resize_args(const char *amount, const char *path) {
if (ret) {
error("unable to retrieve fs info");
- return 1;
+ ret = 1;
+ goto out;
}
if (!fi_args.num_devices) {
@@ -1112,11 +1114,14 @@ static int check_resize_args(const char *amount, const char *path) {
}
ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%s", amount);
+check:
if (strlen(amount) != ret) {
error("newsize argument is too long");
ret = 1;
goto out;
}
+ if (strcmp(amount, amount_dup) != 0)
+ strcpy(amount, amount_dup);
sizestr = amount_dup;
devstr = strchr(sizestr, ':');
@@ -1133,6 +1138,24 @@ static int check_resize_args(const char *amount, const char *path) {
ret = 1;
goto out;
}
+ } else if (fi_args.num_devices != 1) {
+ error("The file system has multiple devices, please specify devid exactly.");
+ error("The device information list is as follows.");
+ for (i = 0; i < fi_args.num_devices; i++) {
+ fprintf(stderr, "\tdevid %4llu size %s used %s path %s\n",
+ di_args[i].devid,
+ pretty_size_mode(di_args[i].total_bytes, UNITS_DEFAULT),
+ pretty_size_mode(di_args[i].bytes_used, UNITS_DEFAULT),
+ di_args[i].path);
+ }
+ ret = 1;
+ goto out;
+ } else {
+ memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
+ ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:", di_args[0].devid);
+ ret = snprintf(amount_dup + strlen(amount_dup),
+ BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
+ goto check;
}
dev_idx = -1;
@@ -1200,10 +1223,11 @@ static int check_resize_args(const char *amount, const char *path) {
di_args[dev_idx].path,
pretty_size_mode(di_args[dev_idx].total_bytes, UNITS_DEFAULT),
res_str);
+ ret = 1;
out:
free(di_args);
- return 0;
+ return ret;
}
static int cmd_filesystem_resize(const struct cmd_struct *cmd,
@@ -1235,8 +1259,10 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
}
}
- if (check_argc_exact(argc - optind, 2))
- return 1;
+ if (check_argc_exact(argc - optind, 2)) {
+ ret = 1;
+ goto out;
+ }
amount = argv[optind];
path = argv[optind + 1];
@@ -1244,7 +1270,8 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
len = strlen(amount);
if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
error("resize value too long (%s)", amount);
- return 1;
+ ret = 1;
+ goto out;
}
cancel = (strcmp("cancel", amount) == 0);
@@ -1258,7 +1285,8 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
"directories as argument. Passing file containing a btrfs image\n"
"would resize the underlying filesystem instead of the image.\n");
}
- return 1;
+ ret = 1;
+ goto out;
}
/*
@@ -1273,14 +1301,22 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
error(
"unable to check status of exclusive operation: %m");
close_file_or_dir(fd, dirstream);
- return 1;
+ goto out;
}
}
+ amount = (char *)malloc(BTRFS_VOL_NAME_MAX);
+ if (!amount) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ strcpy(amount, argv[optind]);
+
ret = check_resize_args(amount, path);
if (ret != 0) {
close_file_or_dir(fd, dirstream);
- return 1;
+ ret = 1;
+ goto free_amount;
}
memset(&args, 0, sizeof(args));
@@ -1298,7 +1334,7 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
error("unable to resize '%s': %m", path);
break;
}
- return 1;
+ ret = 1;
} else if (res > 0) {
const char *err_str = btrfs_err_str(res);
@@ -1308,9 +1344,12 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
error("resizing of '%s' failed: unknown error %d",
path, res);
}
- return 1;
+ ret = 1;
}
- return 0;
+free_amount:
+ free(amount);
+out:
+ return ret;
}
static DEFINE_SIMPLE_COMMAND(filesystem_resize, "resize");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs-progs: fix btrfs resize failed.
2022-07-23 12:02 [PATCH] btrfs-progs: fix btrfs resize failed Li Zhang
@ 2022-07-23 23:21 ` Qu Wenruo
2022-07-24 9:33 ` li zhang
0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2022-07-23 23:21 UTC (permalink / raw)
To: Li Zhang, linux-btrfs
On 2022/7/23 20:02, Li Zhang wrote:
> related issuse:
> https://github.com/kdave/btrfs-progs/issues/470
>
> V1 link:
> https://www.spinics.net/lists/linux-btrfs/msg126661.html
>
> [BUG]
> If there is no devid=1, when the user uses the btrfs file system tool,
> the following error will be reported,
>
> $ sudo btrfs filesystem show /mnt/1
> Label: none uuid: 64dc0f68-9afa-4465-9ea1-2bbebfdb6cec
> Total devices 2 FS bytes used 704.00KiB
> devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
> devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
> $ sudo btrfs filesystem resize -1G /mnt/1
> ERROR: cannot find devid: 1
> ERROR: unable to resize '/mnt/1': No such device
>
> [CAUSE]
> If the user does not specify the devid id explicitly,
> btrfs will use the default devid 1, so it will report an error when dev 1 is missing.
>
> [FIX]
> If the file system contains multiple devices, output an error message to the user.
>
> If the filesystem has only one device, resize should automatically add the unique devid.
>
> [RESULT]
>
> $ sudo btrfs filesystem show /mnt/1/
> Label: none uuid: 2025e6ae-0b6d-40b4-8685-3e7e9fc9b2c2
> Total devices 2 FS bytes used 144.00KiB
> devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
> devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
>
> $ sudo btrfs filesystem resize -1G /mnt/1
> ERROR: The file system has multiple devices, please specify devid exactly.
> ERROR: The device information list is as follows.
> devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
> devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
>
> $ sudo btrfs device delete 2 /mnt/1/
>
> $ sudo btrfs filesystem show /mnt/1/
> Label: none uuid: 2025e6ae-0b6d-40b4-8685-3e7e9fc9b2c2
> Total devices 1 FS bytes used 144.00KiB
> devid 3 size 15.00GiB used 1.28GiB path /dev/loop3
>
> $ sudo btrfs filesystem resize -1G /mnt/1
> Resize device id 3 (/dev/loop3) from 15.00GiB to 14.00GiB
The new output and logic looks pretty good to me, but still some small
nitpicks.
>
> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> ---
> cmds/filesystem.c | 63 ++++++++++++++++++++++++++++++++++++++++++++-----------
Shouldn't we also update the man page of "btrfs-filesystem"?
Mostly to make it explicit when we can skip the devid/device path argument.
> 1 file changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index 7cd08fc..c26ba2b 100644
> --- a/cmds/filesystem.c
> +++ b/cmds/filesystem.c
> @@ -1087,7 +1087,8 @@ static const char * const cmd_filesystem_resize_usage[] = {
> NULL
> };
>
> -static int check_resize_args(const char *amount, const char *path) {
> +static int check_resize_args(char * const amount, const char *path)
> +{
> struct btrfs_ioctl_fs_info_args fi_args;
> struct btrfs_ioctl_dev_info_args *di_args = NULL;
> int ret, i, dev_idx = -1;
> @@ -1102,7 +1103,8 @@ static int check_resize_args(const char *amount, const char *path) {
>
> if (ret) {
> error("unable to retrieve fs info");
> - return 1;
> + ret = 1;
> + goto out;
> }
>
> if (!fi_args.num_devices) {
> @@ -1112,11 +1114,14 @@ static int check_resize_args(const char *amount, const char *path) {
> }
>
> ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%s", amount);
> +check:
> if (strlen(amount) != ret) {
> error("newsize argument is too long");
> ret = 1;
> goto out;
> }
> + if (strcmp(amount, amount_dup) != 0)
> + strcpy(amount, amount_dup);
>
> sizestr = amount_dup;
> devstr = strchr(sizestr, ':');
> @@ -1133,6 +1138,24 @@ static int check_resize_args(const char *amount, const char *path) {
> ret = 1;
> goto out;
> }
> + } else if (fi_args.num_devices != 1) {
> + error("The file system has multiple devices, please specify devid exactly.");
> + error("The device information list is as follows.");
> + for (i = 0; i < fi_args.num_devices; i++) {
> + fprintf(stderr, "\tdevid %4llu size %s used %s path %s\n",
> + di_args[i].devid,
> + pretty_size_mode(di_args[i].total_bytes, UNITS_DEFAULT),
> + pretty_size_mode(di_args[i].bytes_used, UNITS_DEFAULT),
> + di_args[i].path);
> + }
> + ret = 1;
> + goto out;
> + } else {
> + memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
> + ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:", di_args[0].devid);
> + ret = snprintf(amount_dup + strlen(amount_dup),
> + BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
> + goto check;
> }
>
> dev_idx = -1;
> @@ -1200,10 +1223,11 @@ static int check_resize_args(const char *amount, const char *path) {
> di_args[dev_idx].path,
> pretty_size_mode(di_args[dev_idx].total_bytes, UNITS_DEFAULT),
> res_str);
> + ret = 1;
Previously if we reach this path, we should have everything prepared and
return 0.
But now it always return 1, is that expected? Especially the only caller
will just error out if the return value is not 0.
>
> out:
> free(di_args);
> - return 0;
> + return ret;
In fact, this turns out to be an existing bug, there are quite a lot of
existing paths setting ret to 1, and goto out to return error.
Those error paths are not properly handled previously.
Thus changing the out: label to return @ret is in fact a bug fix.
Not sure if we need to split the patch into two, one to fix the return
value first, then introduce the new behavior.
> }
>
> static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> @@ -1235,8 +1259,10 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> }
> }
>
> - if (check_argc_exact(argc - optind, 2))
> - return 1;
> + if (check_argc_exact(argc - optind, 2)) {
> + ret = 1;
> + goto out;
> + }
>
> amount = argv[optind];
> path = argv[optind + 1];
> @@ -1244,7 +1270,8 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> len = strlen(amount);
> if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
> error("resize value too long (%s)", amount);
> - return 1;
> + ret = 1;
> + goto out;
This change doesn't look necessary, the new out label is just returning
the @ret value, not really worthy a new label.
> }
>
> cancel = (strcmp("cancel", amount) == 0);
> @@ -1258,7 +1285,8 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> "directories as argument. Passing file containing a btrfs image\n"
> "would resize the underlying filesystem instead of the image.\n");
> }
> - return 1;
> + ret = 1;
> + goto out;
The same here.
> }
>
> /*
> @@ -1273,14 +1301,22 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> error(
> "unable to check status of exclusive operation: %m");
> close_file_or_dir(fd, dirstream);
> - return 1;
> + goto out;
And here.
> }
> }
>
> + amount = (char *)malloc(BTRFS_VOL_NAME_MAX);
No need to do the type cast, (void *) can be assigned to any pointer
type without the need to type cast.
> + if (!amount) {
> + ret = -ENOMEM;
> + goto out;
The same here too.
Thanks,
Qu
> + }
> + strcpy(amount, argv[optind]);
> +
> ret = check_resize_args(amount, path);
> if (ret != 0) {
> close_file_or_dir(fd, dirstream);
> - return 1;
> + ret = 1;
> + goto free_amount;
> }
>
> memset(&args, 0, sizeof(args));
> @@ -1298,7 +1334,7 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> error("unable to resize '%s': %m", path);
> break;
> }
> - return 1;
> + ret = 1;
> } else if (res > 0) {
> const char *err_str = btrfs_err_str(res);
>
> @@ -1308,9 +1344,12 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> error("resizing of '%s' failed: unknown error %d",
> path, res);
> }
> - return 1;
> + ret = 1;
> }
> - return 0;
> +free_amount:
> + free(amount);
> +out:
> + return ret;
> }
> static DEFINE_SIMPLE_COMMAND(filesystem_resize, "resize");
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs-progs: fix btrfs resize failed.
2022-07-23 23:21 ` Qu Wenruo
@ 2022-07-24 9:33 ` li zhang
0 siblings, 0 replies; 3+ messages in thread
From: li zhang @ 2022-07-24 9:33 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
sorry, my fault, this code
@@ -1200,10 +1223,11 @@ static int check_resize_args(const char
*amount, const char *path) {
di_args[dev_idx].path,
pretty_size_mode(di_args[dev_idx].total_bytes, UNITS_DEFAULT),
res_str);
+ ret = 1;
should be
ret = 0
And for new label out, I think you are right, it doesn't need a label
that does nothing, I will correct my patch.
thanks,
Li
Qu Wenruo <wqu@suse.com> 于2022年7月24日周日 07:21写道：
>
>
>
> On 2022/7/23 20:02, Li Zhang wrote:
> > related issuse:
> > https://github.com/kdave/btrfs-progs/issues/470
> >
> > V1 link:
> > https://www.spinics.net/lists/linux-btrfs/msg126661.html
> >
> > [BUG]
> > If there is no devid=1, when the user uses the btrfs file system tool,
> > the following error will be reported,
> >
> > $ sudo btrfs filesystem show /mnt/1
> > Label: none uuid: 64dc0f68-9afa-4465-9ea1-2bbebfdb6cec
> > Total devices 2 FS bytes used 704.00KiB
> > devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
> > devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
> > $ sudo btrfs filesystem resize -1G /mnt/1
> > ERROR: cannot find devid: 1
> > ERROR: unable to resize '/mnt/1': No such device
> >
> > [CAUSE]
> > If the user does not specify the devid id explicitly,
> > btrfs will use the default devid 1, so it will report an error when dev 1 is missing.
> >
> > [FIX]
> > If the file system contains multiple devices, output an error message to the user.
> >
> > If the filesystem has only one device, resize should automatically add the unique devid.
> >
> > [RESULT]
> >
> > $ sudo btrfs filesystem show /mnt/1/
> > Label: none uuid: 2025e6ae-0b6d-40b4-8685-3e7e9fc9b2c2
> > Total devices 2 FS bytes used 144.00KiB
> > devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
> > devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
> >
> > $ sudo btrfs filesystem resize -1G /mnt/1
> > ERROR: The file system has multiple devices, please specify devid exactly.
> > ERROR: The device information list is as follows.
> > devid 2 size 15.00GiB used 1.16GiB path /dev/loop2
> > devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
> >
> > $ sudo btrfs device delete 2 /mnt/1/
> >
> > $ sudo btrfs filesystem show /mnt/1/
> > Label: none uuid: 2025e6ae-0b6d-40b4-8685-3e7e9fc9b2c2
> > Total devices 1 FS bytes used 144.00KiB
> > devid 3 size 15.00GiB used 1.28GiB path /dev/loop3
> >
> > $ sudo btrfs filesystem resize -1G /mnt/1
> > Resize device id 3 (/dev/loop3) from 15.00GiB to 14.00GiB
>
> The new output and logic looks pretty good to me, but still some small
> nitpicks.
>
> >
> > Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> > ---
> > cmds/filesystem.c | 63 ++++++++++++++++++++++++++++++++++++++++++++-----------
>
> Shouldn't we also update the man page of "btrfs-filesystem"?
> Mostly to make it explicit when we can skip the devid/device path argument.
>
> > 1 file changed, 51 insertions(+), 12 deletions(-)
> >
> > diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> > index 7cd08fc..c26ba2b 100644
> > --- a/cmds/filesystem.c
> > +++ b/cmds/filesystem.c
> > @@ -1087,7 +1087,8 @@ static const char * const cmd_filesystem_resize_usage[] = {
> > NULL
> > };
> >
> > -static int check_resize_args(const char *amount, const char *path) {
> > +static int check_resize_args(char * const amount, const char *path)
> > +{
> > struct btrfs_ioctl_fs_info_args fi_args;
> > struct btrfs_ioctl_dev_info_args *di_args = NULL;
> > int ret, i, dev_idx = -1;
> > @@ -1102,7 +1103,8 @@ static int check_resize_args(const char *amount, const char *path) {
> >
> > if (ret) {
> > error("unable to retrieve fs info");
> > - return 1;
> > + ret = 1;
> > + goto out;
> > }
> >
> > if (!fi_args.num_devices) {
> > @@ -1112,11 +1114,14 @@ static int check_resize_args(const char *amount, const char *path) {
> > }
> >
> > ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%s", amount);
> > +check:
> > if (strlen(amount) != ret) {
> > error("newsize argument is too long");
> > ret = 1;
> > goto out;
> > }
> > + if (strcmp(amount, amount_dup) != 0)
> > + strcpy(amount, amount_dup);
> >
> > sizestr = amount_dup;
> > devstr = strchr(sizestr, ':');
> > @@ -1133,6 +1138,24 @@ static int check_resize_args(const char *amount, const char *path) {
> > ret = 1;
> > goto out;
> > }
> > + } else if (fi_args.num_devices != 1) {
> > + error("The file system has multiple devices, please specify devid exactly.");
> > + error("The device information list is as follows.");
> > + for (i = 0; i < fi_args.num_devices; i++) {
> > + fprintf(stderr, "\tdevid %4llu size %s used %s path %s\n",
> > + di_args[i].devid,
> > + pretty_size_mode(di_args[i].total_bytes, UNITS_DEFAULT),
> > + pretty_size_mode(di_args[i].bytes_used, UNITS_DEFAULT),
> > + di_args[i].path);
> > + }
> > + ret = 1;
> > + goto out;
> > + } else {
> > + memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
> > + ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:", di_args[0].devid);
> > + ret = snprintf(amount_dup + strlen(amount_dup),
> > + BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
> > + goto check;
> > }
> >
> > dev_idx = -1;
> > @@ -1200,10 +1223,11 @@ static int check_resize_args(const char *amount, const char *path) {
> > di_args[dev_idx].path,
> > pretty_size_mode(di_args[dev_idx].total_bytes, UNITS_DEFAULT),
> > res_str);
> > + ret = 1;
>
> Previously if we reach this path, we should have everything prepared and
> return 0.
>
> But now it always return 1, is that expected? Especially the only caller
> will just error out if the return value is not 0.
>
> >
> > out:
> > free(di_args);
> > - return 0;
> > + return ret;
>
> In fact, this turns out to be an existing bug, there are quite a lot of
> existing paths setting ret to 1, and goto out to return error.
>
> Those error paths are not properly handled previously.
>
> Thus changing the out: label to return @ret is in fact a bug fix.
>
> Not sure if we need to split the patch into two, one to fix the return
> value first, then introduce the new behavior.
>
> > }
> >
> > static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> > @@ -1235,8 +1259,10 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> > }
> > }
> >
> > - if (check_argc_exact(argc - optind, 2))
> > - return 1;
> > + if (check_argc_exact(argc - optind, 2)) {
> > + ret = 1;
> > + goto out;
> > + }
> >
> > amount = argv[optind];
> > path = argv[optind + 1];
> > @@ -1244,7 +1270,8 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> > len = strlen(amount);
> > if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
> > error("resize value too long (%s)", amount);
> > - return 1;
> > + ret = 1;
> > + goto out;
>
> This change doesn't look necessary, the new out label is just returning
> the @ret value, not really worthy a new label.
>
> > }
> >
> > cancel = (strcmp("cancel", amount) == 0);
> > @@ -1258,7 +1285,8 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> > "directories as argument. Passing file containing a btrfs image\n"
> > "would resize the underlying filesystem instead of the image.\n");
> > }
> > - return 1;
> > + ret = 1;
> > + goto out;
>
> The same here.
>
> > }
> >
> > /*
> > @@ -1273,14 +1301,22 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> > error(
> > "unable to check status of exclusive operation: %m");
> > close_file_or_dir(fd, dirstream);
> > - return 1;
> > + goto out;
>
> And here.
>
> > }
> > }
> >
> > + amount = (char *)malloc(BTRFS_VOL_NAME_MAX);
>
> No need to do the type cast, (void *) can be assigned to any pointer
> type without the need to type cast.
>
> > + if (!amount) {
> > + ret = -ENOMEM;
> > + goto out;
>
> The same here too.
>
> Thanks,
> Qu
>
> > + }
> > + strcpy(amount, argv[optind]);
> > +
> > ret = check_resize_args(amount, path);
> > if (ret != 0) {
> > close_file_or_dir(fd, dirstream);
> > - return 1;
> > + ret = 1;
> > + goto free_amount;
> > }
> >
> > memset(&args, 0, sizeof(args));
> > @@ -1298,7 +1334,7 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> > error("unable to resize '%s': %m", path);
> > break;
> > }
> > - return 1;
> > + ret = 1;
> > } else if (res > 0) {
> > const char *err_str = btrfs_err_str(res);
> >
> > @@ -1308,9 +1344,12 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd,
> > error("resizing of '%s' failed: unknown error %d",
> > path, res);
> > }
> > - return 1;
> > + ret = 1;
> > }
> > - return 0;
> > +free_amount:
> > + free(amount);
> > +out:
> > + return ret;
> > }
> > static DEFINE_SIMPLE_COMMAND(filesystem_resize, "resize");
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-24 9:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-23 12:02 [PATCH] btrfs-progs: fix btrfs resize failed Li Zhang
2022-07-23 23:21 ` Qu Wenruo
2022-07-24 9:33 ` li zhang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.