On 06/03/2015 04:40 PM, Tsutomu Itoh wrote:
> On 2015/06/03 15:57, Dongsheng Yang wrote:
>> Currently, we can not clear a limitation on a qgroup. Although
>> there is a 'none' choice provided to user to do it, it does not
>> work well.
>>
>> It does not set the flag which user want to clear, then kernel
>> will never know what the user want to do at all.
>>
>> *Without this commit*
>> # ./btrfs qgroup show -re /mnt
>> qgroupid rfer excl max_rfer max_excl
>> -------- ---- ---- -------- --------
>> 0/5 2.19GiB 2.19GiB 5.00GiB none
>> 0/257 100.02MiB 100.02MiB none none
>> # ./btrfs qgroup limit none /mnt
>> # ./btrfs qgroup show -re /mnt
>> qgroupid rfer excl max_rfer max_excl
>> -------- ---- ---- -------- --------
>> 0/5 2.19GiB 2.19GiB 5.00GiB none
>> 0/257 100.02MiB 100.02MiB none none
>>
>> This patch will set the flag user want to clear and pass a
>> size=-1 to kernel. Then kernel will clear it correctly.
>>
>> *With this commit*
>> # ./btrfs qgroup show -re /mnt
>> qgroupid rfer excl max_rfer max_excl
>> -------- ---- ---- -------- --------
>> 0/5 2.19GiB 2.19GiB 5.00GiB none
>> 0/257 100.02MiB 100.02MiB none none
>> # ./btrfs qgroup limit none /mnt
>> # ./btrfs qgroup show -re /mnt
>> qgroupid rfer excl max_rfer max_excl
>> -------- ---- ---- -------- --------
>> 0/5 2.19GiB 2.19GiB none none
>> 0/257 100.02MiB 100.02MiB none none
>>
>> Reported-by: Tsutomu Itoh <t-itoh@xxxxxxxxxxxxxx>
>> Signed-off-by: Dongsheng Yang <yangds.fnst@xxxxxxxxxxxxxx>
>> ---
>> cmds-qgroup.c | 23 +++++++++++------------
>> 1 file changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/cmds-qgroup.c b/cmds-qgroup.c
>> index b073250..00cc089 100644
>> --- a/cmds-qgroup.c
>> +++ b/cmds-qgroup.c
>> @@ -110,9 +110,10 @@ static int parse_limit(const char *p, unsigned long long *s)
>> {
>> char *endptr;
>> unsigned long long size;
>> + unsigned long long CLEAR_VALUE = -1;
>
> Even if a negative value is specified, it doesn't become an error...
> So, it doesn't make an error of the following command.
>
> # btrfs qg lim -- -1 sub1
> # btrfs qg show -prce /test1
> qgroupid rfer excl max_rfer max_excl parent child
> -------- ---- ---- -------- -------- ------ -----
> 0/5 16.00KiB 16.00KiB none none --- ---
> 0/259 8.86GiB 7.88GiB none none --- ---
> # btrfs qg lim -- -2 sub1
> # btrfs qg show -prce /test1
> qgroupid rfer excl max_rfer max_excl parent child
> -------- ---- ---- -------- -------- ------ -----
> 0/5 16.00KiB 16.00KiB none none --- ---
> 0/259 8.86GiB 7.88GiB 16.00EiB none --- ---
Agreed, I understand your point here.
If we pass a negative value to limit command, we should Error out. But
currently, btrfs-progs are treating it as a unsigned value in parsing
it. Yes, that's a bit of confusing.
We need to cover it in parsing steps with another patch I think.
>
> Otherwise OK,
>
> Tested-by: Tsutomu Itoh <t-itoh@xxxxxxxxxxxxxx>
Thanx
Yang
>
>
>>
>> if (strcasecmp(p, "none") == 0) {
>> - *s = 0;
>> + *s = CLEAR_VALUE;
>> return 1;
>> }
>> size = strtoull(p, &endptr, 10);
>> @@ -406,17 +407,15 @@ static int cmd_qgroup_limit(int argc, char **argv)
>> }
>>
>> memset(&args, 0, sizeof(args));
>> - if (size) {
>> - if (compressed)
>> - args.lim.flags |= BTRFS_QGROUP_LIMIT_RFER_CMPR |
>> - BTRFS_QGROUP_LIMIT_EXCL_CMPR;
>> - if (exclusive) {
>> - args.lim.flags |= BTRFS_QGROUP_LIMIT_MAX_EXCL;
>> - args.lim.max_exclusive = size;
>> - } else {
>> - args.lim.flags |= BTRFS_QGROUP_LIMIT_MAX_RFER;
>> - args.lim.max_referenced = size;
>> - }
>> + if (compressed)
>> + args.lim.flags |= BTRFS_QGROUP_LIMIT_RFER_CMPR |
>> + BTRFS_QGROUP_LIMIT_EXCL_CMPR;
>> + if (exclusive) {
>> + args.lim.flags |= BTRFS_QGROUP_LIMIT_MAX_EXCL;
>> + args.lim.max_exclusive = size;
>> + } else {
>> + args.lim.flags |= BTRFS_QGROUP_LIMIT_MAX_RFER;
>> + args.lim.max_referenced = size;
>> }
>>
>> if (argc - optind == 2) {
>>
>
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
