[PATCH 0/3] btrfs-progs: Minor qgroup subcommand usage update

[PATCH 0/3] btrfs-progs: Minor qgroup subcommand usage update

[Qu Wenruo]

It turns out even commit e5a6610c943b ("btrfs-progs: qgroup assign: add
option to schedule rescan") introduced the ability to auto rescan, it's
less known option and even some developer find it hard to understand.
(Well, the whole quota thing is already a little hard to understand)

Thus it makes more sense to make --rescan as default behavior, and add
more explanation on why quota rescan is needed, and under which case we
can skip the rescan.

Qu Wenruo (3):
  btrfs-progs: cmds-qgroup: Use bool to replace int for @rescan
  btrfs-progs: qgroup: make --rescan as the default behavior for assign
  btrfs-progs: Doc: Update btrfs-qgroup for the rescan condition

 Documentation/btrfs-qgroup.asciidoc | 26 +++++++++++++++++++++++---
 cmds-qgroup.c                       |  6 +++---
 2 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.18.0

[PATCH 1/3] btrfs-progs: cmds-qgroup: Use bool to replace int for @rescan

[Qu Wenruo]

Just a cleanup to avoid abuse of int.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds-qgroup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 93206900693d..7a505443d271 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -40,7 +40,7 @@ static int _cmd_qgroup_assign(int assign, int argc, char **argv,
 {
 	int ret = 0;
 	int fd;
-	int rescan = 0;
+	bool rescan = false;
 	char *path;
 	struct btrfs_ioctl_qgroup_assign_args args;
 	DIR *dirstream = NULL;
@@ -61,10 +61,10 @@ static int _cmd_qgroup_assign(int assign, int argc, char **argv,
 				break;
 			switch (c) {
 			case GETOPT_VAL_RESCAN:
-				rescan = 1;
+				rescan = true;
 				break;
 			case GETOPT_VAL_NO_RESCAN:
-				rescan = 0;
+				rescan = false;
 				break;
 			default:
 				/* Usage printed by the caller */
-- 
2.18.0

[PATCH 2/3] btrfs-progs: qgroup: make --rescan as the default behavior for assign

[Qu Wenruo]

Even though we have --rescan option, it still needs user to manually
specify it.

However even some developer is not aware of the option, it makes even
less sense for end-user.

So make it the default behavior.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Documentation/btrfs-qgroup.asciidoc | 4 ++--
 cmds-qgroup.c                       | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/btrfs-qgroup.asciidoc b/Documentation/btrfs-qgroup.asciidoc
index 3108457cc8e1..fee173860000 100644
--- a/Documentation/btrfs-qgroup.asciidoc
+++ b/Documentation/btrfs-qgroup.asciidoc
@@ -47,8 +47,8 @@ identified by <path>.
 `Options`
 +
 --rescan::::
-Automatically schedule quota rescan if the new qgroup assignment leads to
-quota inconsistency.
+(Default) Automatically schedule quota rescan if the new qgroup assignment
+leads to quota inconsistency.
 --no-rescan::::
 Explicitly ask not to do a rescan.
 
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 7a505443d271..094e5174fd7f 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -40,7 +40,7 @@ static int _cmd_qgroup_assign(int assign, int argc, char **argv,
 {
 	int ret = 0;
 	int fd;
-	bool rescan = false;
+	bool rescan = true;
 	char *path;
 	struct btrfs_ioctl_qgroup_assign_args args;
 	DIR *dirstream = NULL;
-- 
2.18.0

[PATCH 3/3] btrfs-progs: Doc: Update btrfs-qgroup for the rescan condition

[Qu Wenruo]

Add a new section, RESCAN CONDITION, to explain why and when we need a
full quota rescan when assigning/removing qgroup relationship.

Also, since 'remove' shares the same options of 'assign', add reference
to 'assign' options for 'remove' subcommand.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Documentation/btrfs-qgroup.asciidoc | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-qgroup.asciidoc b/Documentation/btrfs-qgroup.asciidoc
index fee173860000..d1c8fedc94d4 100644
--- a/Documentation/btrfs-qgroup.asciidoc
+++ b/Documentation/btrfs-qgroup.asciidoc
@@ -49,8 +49,10 @@ identified by <path>.
 --rescan::::
 (Default) Automatically schedule quota rescan if the new qgroup assignment
 leads to quota inconsistency.
+Check `RESCAN CONDITION` for more info.
 --no-rescan::::
-Explicitly ask not to do a rescan.
+Explicitly ask not to do a rescan, even if the assigning makes quota
+inconsistent.
 
 *create* <qgroupid> <path>::
 Create a subvolume quota group.
@@ -83,6 +85,10 @@ limit space exclusively assigned to this qgroup.
 *remove* <src> <dst> <path>::
 Remove the relationship between child qgroup <src> and parent qgroup <dst> in
 the btrfs filesystem identified by <path>.
++
+`Options`
++
+The same as *assign* subcommand.
 
 *show* [options] <path>::
 Show all qgroups in the btrfs filesystem identified by <path>.
@@ -131,6 +137,20 @@ If multiple <attr>s is given, use comma to separate.
 To retrieve information after updating the state of qgroups,
 force sync of the filesystem identified by <path> before getting information.
 
+RESCAN CONDITION
+----------------
+Btrfs quota only keep records of how many bytes are referred to ('rfer') and
+how many bytes are owned exclusively ('excl').
+It doesn't keep any record of which extent is owned exclusively or shared.
+
+This means when qgroup relationship changes, extents owners change and qgroups
+number are no longer consistent unless we do a full rescan.
+
+However there are cases we can avoid full rescan, if a subvolume whose 'rfer'
+number equals its 'excl' number, which means all bytes are exclusive owned,
+then assigning/removing this subvolume only needs to add/subtract 'rfer' number
+from its parent qgroup.
+
 EXIT STATUS
 -----------
 *btrfs qgroup* returns a zero exit status if it succeeds. Non zero is
-- 
2.18.0

Re: [PATCH 0/3] btrfs-progs: Minor qgroup subcommand usage update

[David Sterba]

On Mon, Aug 06, 2018 at 02:00:05PM +0800, Qu Wenruo wrote:
> It turns out even commit e5a6610c943b ("btrfs-progs: qgroup assign: add
> option to schedule rescan") introduced the ability to auto rescan, it's
> less known option and even some developer find it hard to understand.
> (Well, the whole quota thing is already a little hard to understand)
> 
> Thus it makes more sense to make --rescan as default behavior, and add
> more explanation on why quota rescan is needed, and under which case we
> can skip the rescan.
> 
> Qu Wenruo (3):
>   btrfs-progs: cmds-qgroup: Use bool to replace int for @rescan
>   btrfs-progs: qgroup: make --rescan as the default behavior for assign
>   btrfs-progs: Doc: Update btrfs-qgroup for the rescan condition

Applied, thanks. I reworded some parts, so don't be surprised that there
are changes.