* [PATCH] btrfs-progs: resize: automatically add devid if device is not specifically
@ 2022-07-17 15:08 Li Zhang
2022-07-17 17:39 ` Zygo Blaxell
0 siblings, 1 reply; 5+ messages in thread
From: Li Zhang @ 2022-07-17 15:08 UTC (permalink / raw)
To: linux-btrfs; +Cc: Li Zhang
related issues:
https://github.com/kdave/btrfs-progs/issues/470
[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 there is no special devid, the first devid is added automatically and check the maximum length of args passed to kernel space.
After patch, when resize filesystem without specified, it would resize the first device, the result is list as following.
$ sudo btrfs filesystem show /mnt/1/
Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
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
Resize device id 2 (/dev/loop2) from 15.00GiB to 14.00GiB
$ sudo btrfs filesystem show /mnt/1/
Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
Total devices 2 FS bytes used 144.00KiB
devid 2 size 14.00GiB used 1.16GiB path /dev/loop2
devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
---
cmds/filesystem.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 38 insertions(+), 11 deletions(-)
diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 7cd08fc..2e2414d 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, ':');
@@ -1137,6 +1142,13 @@ static int check_resize_args(const char *amount, const char *path) {
dev_idx = -1;
for(i = 0; i < fi_args.num_devices; i++) {
+ if (!devstr) {
+ memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
+ ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:", di_args[i].devid);
+ ret = snprintf(amount_dup + strlen(amount_dup),
+ BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
+ goto check;
+ }
if (di_args[i].devid == devid) {
dev_idx = i;
break;
@@ -1235,8 +1247,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 +1258,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 +1273,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 +1289,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 +1322,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 +1332,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] 5+ messages in thread
* Re: [PATCH] btrfs-progs: resize: automatically add devid if device is not specifically
2022-07-17 15:08 [PATCH] btrfs-progs: resize: automatically add devid if device is not specifically Li Zhang
@ 2022-07-17 17:39 ` Zygo Blaxell
2022-07-18 1:16 ` Qu Wenruo
0 siblings, 1 reply; 5+ messages in thread
From: Zygo Blaxell @ 2022-07-17 17:39 UTC (permalink / raw)
To: Li Zhang; +Cc: linux-btrfs
On Sun, Jul 17, 2022 at 11:08:23PM +0800, Li Zhang wrote:
> related issues:
> https://github.com/kdave/btrfs-progs/issues/470
>
> [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 there is no special devid, the first devid is added automatically and check the maximum length of args passed to kernel space.
> After patch, when resize filesystem without specified, it would resize the first device, the result is list as following.
>
> $ sudo btrfs filesystem show /mnt/1/
> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
> 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
> Resize device id 2 (/dev/loop2) from 15.00GiB to 14.00GiB
> $ sudo btrfs filesystem show /mnt/1/
> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
> Total devices 2 FS bytes used 144.00KiB
> devid 2 size 14.00GiB used 1.16GiB path /dev/loop2
> devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
Is that desirable behavior? I'd expect that if there are multiple
devices present, and I haven't specified which one to resize, that the
command would fail with an error, requiring me to specify which device I
want resized. Under that expectation, the current behavior of resizing
devid 1 by default is also incorrect.
If there's only one device, then 'btrfs fi resize -1G' should resize
that device, since no ambiguity is possible.
> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> ---
> cmds/filesystem.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index 7cd08fc..2e2414d 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, ':');
> @@ -1137,6 +1142,13 @@ static int check_resize_args(const char *amount, const char *path) {
>
> dev_idx = -1;
> for(i = 0; i < fi_args.num_devices; i++) {
> + if (!devstr) {
> + memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
> + ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:", di_args[i].devid);
> + ret = snprintf(amount_dup + strlen(amount_dup),
> + BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
> + goto check;
> + }
> if (di_args[i].devid == devid) {
> dev_idx = i;
> break;
> @@ -1235,8 +1247,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 +1258,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 +1273,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 +1289,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 +1322,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 +1332,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 [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: resize: automatically add devid if device is not specifically
2022-07-17 17:39 ` Zygo Blaxell
@ 2022-07-18 1:16 ` Qu Wenruo
2022-07-18 1:44 ` Qu Wenruo
0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2022-07-18 1:16 UTC (permalink / raw)
To: Zygo Blaxell, Li Zhang; +Cc: linux-btrfs
On 2022/7/18 01:39, Zygo Blaxell wrote:
> On Sun, Jul 17, 2022 at 11:08:23PM +0800, Li Zhang wrote:
>> related issues:
>> https://github.com/kdave/btrfs-progs/issues/470
>>
>> [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 there is no special devid, the first devid is added automatically and check the maximum length of args passed to kernel space.
>> After patch, when resize filesystem without specified, it would resize the first device, the result is list as following.
>>
>> $ sudo btrfs filesystem show /mnt/1/
>> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
>> 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
>> Resize device id 2 (/dev/loop2) from 15.00GiB to 14.00GiB
>> $ sudo btrfs filesystem show /mnt/1/
>> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
>> Total devices 2 FS bytes used 144.00KiB
>> devid 2 size 14.00GiB used 1.16GiB path /dev/loop2
>> devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
>
> Is that desirable behavior? I'd expect that if there are multiple
> devices present, and I haven't specified which one to resize, that the
> command would fail with an error, requiring me to specify which device I
> want resized. Under that expectation, the current behavior of resizing
> devid 1 by default is also incorrect.
I agree with Zygo.
If there is only one device, then we can definitely use the first device
we find.
If there are multiple devices, then it's better to output an error
message, provide candidate devids, and error out.
Thanks,
Qu
>
> If there's only one device, then 'btrfs fi resize -1G' should resize
> that device, since no ambiguity is possible.
>
>> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
>> ---
>> cmds/filesystem.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 38 insertions(+), 11 deletions(-)
>>
>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>> index 7cd08fc..2e2414d 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, ':');
>> @@ -1137,6 +1142,13 @@ static int check_resize_args(const char *amount, const char *path) {
>>
>> dev_idx = -1;
>> for(i = 0; i < fi_args.num_devices; i++) {
>> + if (!devstr) {
>> + memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
>> + ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:", di_args[i].devid);
>> + ret = snprintf(amount_dup + strlen(amount_dup),
>> + BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
>> + goto check;
>> + }
>> if (di_args[i].devid == devid) {
>> dev_idx = i;
>> break;
>> @@ -1235,8 +1247,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 +1258,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 +1273,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 +1289,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 +1322,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 +1332,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 [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: resize: automatically add devid if device is not specifically
2022-07-18 1:16 ` Qu Wenruo
@ 2022-07-18 1:44 ` Qu Wenruo
[not found] ` <CAAa-AGnn4ryjOW+LDVNrPjw2O2gZfGxLG_axzCXDvQZKKp+Qzw@mail.gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2022-07-18 1:44 UTC (permalink / raw)
To: Zygo Blaxell, Li Zhang; +Cc: linux-btrfs
On 2022/7/18 09:16, Qu Wenruo wrote:
>
>
> On 2022/7/18 01:39, Zygo Blaxell wrote:
>> On Sun, Jul 17, 2022 at 11:08:23PM +0800, Li Zhang wrote:
>>> related issues:
>>> https://github.com/kdave/btrfs-progs/issues/470
>>>
>>> [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 there is no special devid, the first devid is added automatically
>>> and check the maximum length of args passed to kernel space.
>>> After patch, when resize filesystem without specified, it would
>>> resize the first device, the result is list as following.
>>>
>>> $ sudo btrfs filesystem show /mnt/1/
>>> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
>>> 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
>>> Resize device id 2 (/dev/loop2) from 15.00GiB to 14.00GiB
>>> $ sudo btrfs filesystem show /mnt/1/
>>> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
>>> Total devices 2 FS bytes used 144.00KiB
>>> devid 2 size 14.00GiB used 1.16GiB path /dev/loop2
>>> devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
>>
>> Is that desirable behavior? I'd expect that if there are multiple
>> devices present, and I haven't specified which one to resize, that the
>> command would fail with an error, requiring me to specify which device I
>> want resized. Under that expectation, the current behavior of resizing
>> devid 1 by default is also incorrect.
>
> I agree with Zygo.
>
> If there is only one device, then we can definitely use the first device
> we find.
>
> If there are multiple devices, then it's better to output an error
> message, provide candidate devids, and error out.
And forgot to mention, after all these changes, we also need to update
the man page of "btrfs-filesystem" subcommand.
Thanks,
Qu
>
> Thanks,
> Qu
>
>>
>> If there's only one device, then 'btrfs fi resize -1G' should resize
>> that device, since no ambiguity is possible.
>>
>>> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
>>> ---
>>> cmds/filesystem.c | 49
>>> ++++++++++++++++++++++++++++++++++++++-----------
>>> 1 file changed, 38 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>>> index 7cd08fc..2e2414d 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, ':');
>>> @@ -1137,6 +1142,13 @@ static int check_resize_args(const char
>>> *amount, const char *path) {
>>>
>>> dev_idx = -1;
>>> for(i = 0; i < fi_args.num_devices; i++) {
>>> + if (!devstr) {
>>> + memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
>>> + ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:",
>>> di_args[i].devid);
>>> + ret = snprintf(amount_dup + strlen(amount_dup),
>>> + BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
>>> + goto check;
>>> + }
>>> if (di_args[i].devid == devid) {
>>> dev_idx = i;
>>> break;
>>> @@ -1235,8 +1247,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 +1258,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 +1273,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 +1289,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 +1322,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 +1332,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 [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: resize: automatically add devid if device is not specifically
[not found] ` <CAAa-AGnn4ryjOW+LDVNrPjw2O2gZfGxLG_axzCXDvQZKKp+Qzw@mail.gmail.com>
@ 2022-07-18 16:38 ` li zhang
0 siblings, 0 replies; 5+ messages in thread
From: li zhang @ 2022-07-18 16:38 UTC (permalink / raw)
To: Qu Wenruo, ce3g8jdj; +Cc: linux-btrfs
Yes, that makes sense, I'll correct this patch later.
I'll also try to fix this:
https://github.com/kdave/btrfs-progs/issues/471, add parameter all to
resize all devices.
li zhang <zhanglikernel@gmail.com> 于2022年7月19日周二 00:34写道：
>
> Yes, that makes sense, I'll correct this patch later.
> I'll also try to fix this:
> https://github.com/kdave/btrfs-progs/issues/471, add parameter all to
> resize all devices.
>
> Qu Wenruo <quwenruo.btrfs@gmx.com> 于2022年7月18日周一 09:44写道：
> >
> >
> >
> > On 2022/7/18 09:16, Qu Wenruo wrote:
> > >
> > >
> > > On 2022/7/18 01:39, Zygo Blaxell wrote:
> > >> On Sun, Jul 17, 2022 at 11:08:23PM +0800, Li Zhang wrote:
> > >>> related issues:
> > >>> https://github.com/kdave/btrfs-progs/issues/470
> > >>>
> > >>> [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 there is no special devid, the first devid is added automatically
> > >>> and check the maximum length of args passed to kernel space.
> > >>> After patch, when resize filesystem without specified, it would
> > >>> resize the first device, the result is list as following.
> > >>>
> > >>> $ sudo btrfs filesystem show /mnt/1/
> > >>> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
> > >>> 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
> > >>> Resize device id 2 (/dev/loop2) from 15.00GiB to 14.00GiB
> > >>> $ sudo btrfs filesystem show /mnt/1/
> > >>> Label: none uuid: 7b4827da-bc6e-42aa-b03d-52c2533dfe94
> > >>> Total devices 2 FS bytes used 144.00KiB
> > >>> devid 2 size 14.00GiB used 1.16GiB path /dev/loop2
> > >>> devid 3 size 15.00GiB used 1.16GiB path /dev/loop3
> > >>
> > >> Is that desirable behavior? I'd expect that if there are multiple
> > >> devices present, and I haven't specified which one to resize, that the
> > >> command would fail with an error, requiring me to specify which device I
> > >> want resized. Under that expectation, the current behavior of resizing
> > >> devid 1 by default is also incorrect.
> > >
> > > I agree with Zygo.
> > >
> > > If there is only one device, then we can definitely use the first device
> > > we find.
> > >
> > > If there are multiple devices, then it's better to output an error
> > > message, provide candidate devids, and error out.
> >
> > And forgot to mention, after all these changes, we also need to update
> > the man page of "btrfs-filesystem" subcommand.
> >
> > Thanks,
> > Qu
> >
> > >
> > > Thanks,
> > > Qu
> > >
> > >>
> > >> If there's only one device, then 'btrfs fi resize -1G' should resize
> > >> that device, since no ambiguity is possible.
> > >>
> > >>> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> > >>> ---
> > >>> cmds/filesystem.c | 49
> > >>> ++++++++++++++++++++++++++++++++++++++-----------
> > >>> 1 file changed, 38 insertions(+), 11 deletions(-)
> > >>>
> > >>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> > >>> index 7cd08fc..2e2414d 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, ':');
> > >>> @@ -1137,6 +1142,13 @@ static int check_resize_args(const char
> > >>> *amount, const char *path) {
> > >>>
> > >>> dev_idx = -1;
> > >>> for(i = 0; i < fi_args.num_devices; i++) {
> > >>> + if (!devstr) {
> > >>> + memset(amount_dup, 0, BTRFS_VOL_NAME_MAX);
> > >>> + ret = snprintf(amount_dup, BTRFS_VOL_NAME_MAX, "%llu:",
> > >>> di_args[i].devid);
> > >>> + ret = snprintf(amount_dup + strlen(amount_dup),
> > >>> + BTRFS_VOL_NAME_MAX - strlen(amount_dup), "%s", amount);
> > >>> + goto check;
> > >>> + }
> > >>> if (di_args[i].devid == devid) {
> > >>> dev_idx = i;
> > >>> break;
> > >>> @@ -1235,8 +1247,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 +1258,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 +1273,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 +1289,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 +1322,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 +1332,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 [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-18 16:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-17 15:08 [PATCH] btrfs-progs: resize: automatically add devid if device is not specifically Li Zhang
2022-07-17 17:39 ` Zygo Blaxell
2022-07-18 1:16 ` Qu Wenruo
2022-07-18 1:44 ` Qu Wenruo
[not found] ` <CAAa-AGnn4ryjOW+LDVNrPjw2O2gZfGxLG_axzCXDvQZKKp+Qzw@mail.gmail.com>
2022-07-18 16:38 ` 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.