* [PATCH] btrfs-progs: parse-utils: allow single number qgroup id
@ 2021-10-11 7:09 Qu Wenruo
2021-10-11 7:32 ` Nikolay Borisov
2021-10-11 13:21 ` David Sterba
0 siblings, 2 replies; 6+ messages in thread
From: Qu Wenruo @ 2021-10-11 7:09 UTC (permalink / raw)
To: linux-btrfs
[BUG]
Since btrfs-progs v5.14, fstests/btrfs/099 always fail with the
following output in 099.full:
...
# /usr/bin/btrfs quota enable /mnt/scratch
# /usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch
ERROR: invalid qgroupid or subvolume path: 5
failed: '/usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch'
[CAUSE]
Since commit cb5a542871f9 ("btrfs-progs: factor out plain qgroupid
parsing"), btrfs qgroup parser no longer accepts single number qgroup id
like "5" used in that test case.
That commit is not a plain refactor without functional change, but
removed a simple feature.
[FIX]
Add back the handling for single number qgroupid.
Fixes: cb5a542871f9 ("btrfs-progs: factor out plain qgroupid parsing")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
common/parse-utils.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/common/parse-utils.c b/common/parse-utils.c
index ad57b74a7b64..863ca9f25fd8 100644
--- a/common/parse-utils.c
+++ b/common/parse-utils.c
@@ -290,6 +290,12 @@ int parse_qgroupid(const char *str, u64 *qgroupid)
level = strtoull(str, &end, 10);
if (str == end)
return -EINVAL;
+ /* We accept single qgroupid like "5", to indicate "0/5"*/
+ if (end[0] == '\0') {
+ *qgroupid = level;
+ return 0;
+ }
+ /* Otherwise qgroupid must go like "1/256" */
if (end[0] != '/')
return -EINVAL;
str = end + 1;
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: parse-utils: allow single number qgroup id
2021-10-11 7:09 [PATCH] btrfs-progs: parse-utils: allow single number qgroup id Qu Wenruo
@ 2021-10-11 7:32 ` Nikolay Borisov
2021-10-11 13:21 ` David Sterba
1 sibling, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2021-10-11 7:32 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.10.21 г. 10:09, Qu Wenruo wrote:
> [BUG]
> Since btrfs-progs v5.14, fstests/btrfs/099 always fail with the
> following output in 099.full:
>
> ...
> # /usr/bin/btrfs quota enable /mnt/scratch
> # /usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch
> ERROR: invalid qgroupid or subvolume path: 5
> failed: '/usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch'
>
> [CAUSE]
> Since commit cb5a542871f9 ("btrfs-progs: factor out plain qgroupid
> parsing"), btrfs qgroup parser no longer accepts single number qgroup id
> like "5" used in that test case.
>
> That commit is not a plain refactor without functional change, but
> removed a simple feature.
>
> [FIX]
> Add back the handling for single number qgroupid.
>
> Fixes: cb5a542871f9 ("btrfs-progs: factor out plain qgroupid parsing")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Tested-by: Nikolay Borisov <nborisov@suse.com>
> ---
> common/parse-utils.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/common/parse-utils.c b/common/parse-utils.c
> index ad57b74a7b64..863ca9f25fd8 100644
> --- a/common/parse-utils.c
> +++ b/common/parse-utils.c
> @@ -290,6 +290,12 @@ int parse_qgroupid(const char *str, u64 *qgroupid)
> level = strtoull(str, &end, 10);
> if (str == end)
> return -EINVAL;
> + /* We accept single qgroupid like "5", to indicate "0/5"*/
> + if (end[0] == '\0') {
> + *qgroupid = level;
> + return 0;
> + }
> + /* Otherwise qgroupid must go like "1/256" */
> if (end[0] != '/')
> return -EINVAL;
> str = end + 1;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: parse-utils: allow single number qgroup id
2021-10-11 7:09 [PATCH] btrfs-progs: parse-utils: allow single number qgroup id Qu Wenruo
2021-10-11 7:32 ` Nikolay Borisov
@ 2021-10-11 13:21 ` David Sterba
2021-10-12 10:59 ` Qu Wenruo
1 sibling, 1 reply; 6+ messages in thread
From: David Sterba @ 2021-10-11 13:21 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, Oct 11, 2021 at 03:09:37PM +0800, Qu Wenruo wrote:
> [BUG]
> Since btrfs-progs v5.14, fstests/btrfs/099 always fail with the
> following output in 099.full:
>
> ...
> # /usr/bin/btrfs quota enable /mnt/scratch
> # /usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch
> ERROR: invalid qgroupid or subvolume path: 5
> failed: '/usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch'
>
> [CAUSE]
> Since commit cb5a542871f9 ("btrfs-progs: factor out plain qgroupid
> parsing"), btrfs qgroup parser no longer accepts single number qgroup id
> like "5" used in that test case.
>
> That commit is not a plain refactor without functional change, but
> removed a simple feature.
>
> [FIX]
> Add back the handling for single number qgroupid.
This unfortunately breaks something else, as it's changing the whole
parse_qgroupid helper to accept single value id, which is also used for
'qgroup create'.
There's another combined helper for parsing qgroup id and path, so we
may need to add another one that also accepts the subvolid, in commands
where this makes sense. The helper parse_qgroupid should really remain
as it is and parse the 'number/number' format.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: parse-utils: allow single number qgroup id
2021-10-11 13:21 ` David Sterba
@ 2021-10-12 10:59 ` Qu Wenruo
2021-10-13 12:42 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2021-10-12 10:59 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
On 2021/10/11 21:21, David Sterba wrote:
> On Mon, Oct 11, 2021 at 03:09:37PM +0800, Qu Wenruo wrote:
>> [BUG]
>> Since btrfs-progs v5.14, fstests/btrfs/099 always fail with the
>> following output in 099.full:
>>
>> ...
>> # /usr/bin/btrfs quota enable /mnt/scratch
>> # /usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch
>> ERROR: invalid qgroupid or subvolume path: 5
>> failed: '/usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch'
>>
>> [CAUSE]
>> Since commit cb5a542871f9 ("btrfs-progs: factor out plain qgroupid
>> parsing"), btrfs qgroup parser no longer accepts single number qgroup id
>> like "5" used in that test case.
>>
>> That commit is not a plain refactor without functional change, but
>> removed a simple feature.
>>
>> [FIX]
>> Add back the handling for single number qgroupid.
>
> This unfortunately breaks something else, as it's changing the whole
> parse_qgroupid helper to accept single value id, which is also used for
> 'qgroup create'.
For 'qgroup create' I think it's OK to accept single value id.
It's a handy shortcut for '0/<subvolid>'.
And for higher level qgroup, we need the level anyway.
>
> There's another combined helper for parsing qgroup id and path, so we
> may need to add another one that also accepts the subvolid, in commands
> where this makes sense. The helper parse_qgroupid should really remain
> as it is and parse the 'number/number' format.
>
Then I'm completely fine to update the test case to go "0/5".
But I'm not sure if this change would affect the existing user scripts.
(Considering there is no report yet I guess it's not that widely used
anyway?)
Thanks,
Qu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: parse-utils: allow single number qgroup id
2021-10-12 10:59 ` Qu Wenruo
@ 2021-10-13 12:42 ` David Sterba
2021-10-13 12:55 ` Qu Wenruo
0 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2021-10-13 12:42 UTC (permalink / raw)
To: Qu Wenruo; +Cc: dsterba, Qu Wenruo, linux-btrfs
On Tue, Oct 12, 2021 at 06:59:30PM +0800, Qu Wenruo wrote:
>
>
> On 2021/10/11 21:21, David Sterba wrote:
> > On Mon, Oct 11, 2021 at 03:09:37PM +0800, Qu Wenruo wrote:
> >> [BUG]
> >> Since btrfs-progs v5.14, fstests/btrfs/099 always fail with the
> >> following output in 099.full:
> >>
> >> ...
> >> # /usr/bin/btrfs quota enable /mnt/scratch
> >> # /usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch
> >> ERROR: invalid qgroupid or subvolume path: 5
> >> failed: '/usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch'
> >>
> >> [CAUSE]
> >> Since commit cb5a542871f9 ("btrfs-progs: factor out plain qgroupid
> >> parsing"), btrfs qgroup parser no longer accepts single number qgroup id
> >> like "5" used in that test case.
> >>
> >> That commit is not a plain refactor without functional change, but
> >> removed a simple feature.
> >>
> >> [FIX]
> >> Add back the handling for single number qgroupid.
> >
> > This unfortunately breaks something else, as it's changing the whole
> > parse_qgroupid helper to accept single value id, which is also used for
> > 'qgroup create'.
>
> For 'qgroup create' I think it's OK to accept single value id.
>
> It's a handy shortcut for '0/<subvolid>'.
The 0/subvolid qgroup cannot be created manually IIRC.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs-progs: parse-utils: allow single number qgroup id
2021-10-13 12:42 ` David Sterba
@ 2021-10-13 12:55 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2021-10-13 12:55 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
On 2021/10/13 20:42, David Sterba wrote:
> On Tue, Oct 12, 2021 at 06:59:30PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2021/10/11 21:21, David Sterba wrote:
>>> On Mon, Oct 11, 2021 at 03:09:37PM +0800, Qu Wenruo wrote:
>>>> [BUG]
>>>> Since btrfs-progs v5.14, fstests/btrfs/099 always fail with the
>>>> following output in 099.full:
>>>>
>>>> ...
>>>> # /usr/bin/btrfs quota enable /mnt/scratch
>>>> # /usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch
>>>> ERROR: invalid qgroupid or subvolume path: 5
>>>> failed: '/usr/bin/btrfs qgroup limit 134217728 5 /mnt/scratch'
>>>>
>>>> [CAUSE]
>>>> Since commit cb5a542871f9 ("btrfs-progs: factor out plain qgroupid
>>>> parsing"), btrfs qgroup parser no longer accepts single number qgroup id
>>>> like "5" used in that test case.
>>>>
>>>> That commit is not a plain refactor without functional change, but
>>>> removed a simple feature.
>>>>
>>>> [FIX]
>>>> Add back the handling for single number qgroupid.
>>>
>>> This unfortunately breaks something else, as it's changing the whole
>>> parse_qgroupid helper to accept single value id, which is also used for
>>> 'qgroup create'.
>>
>> For 'qgroup create' I think it's OK to accept single value id.
>>
>> It's a handy shortcut for '0/<subvolid>'.
>
> The 0/subvolid qgroup cannot be created manually IIRC.
>
They can, even with current kernel/progs.
Thanks,
Qu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-13 12:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 7:09 [PATCH] btrfs-progs: parse-utils: allow single number qgroup id Qu Wenruo
2021-10-11 7:32 ` Nikolay Borisov
2021-10-11 13:21 ` David Sterba
2021-10-12 10:59 ` Qu Wenruo
2021-10-13 12:42 ` David Sterba
2021-10-13 12:55 ` Qu Wenruo
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.