| Message ID | 1433314653-548-3-git-send-email-yangds.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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@jp.fujitsu.com> > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > --- > 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 --- --- Otherwise OK, Tested-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@jp.fujitsu.com> >> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> >> --- >> 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@jp.fujitsu.com> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 03, 2015 at 02:57:33PM +0800, 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. > ... > > Reported-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com> > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; 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) {
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@jp.fujitsu.com> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- cmds-qgroup.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)