Re: [PATCH] btrfs-progs: parse-utils: allow single number qgroup id

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

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