Skip to contentSkip to conversation

btrfs-progs: Minor qgroup subcommand usage update

PATCH Applied Qu Wenruo 2 participants 5 messages Aug 6, 2018 – Oct 25, 2018

Subsystems (2)
lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

cover btrfs-progs: Minor qgroup subcommand usage update

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(-)
Signature · 1 line
2.18.0
author
#1 lore raw

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

Just a cleanup to avoid abuse of int.

---
 cmds-qgroup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
cmds-qgroup.c +3 -3
index 93206900693d..7a505443d271 100644
@@ -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 */
Signature · 1 line
2.18.0
author
#2 lore raw

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

In reply to 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.

---
 Documentation/btrfs-qgroup.asciidoc | 4 ++--
 cmds-qgroup.c                       | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
Documentation/btrfs-qgroup.asciidoc +2 -2
index 3108457cc8e1..fee173860000 100644
@@ -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.
 
cmds-qgroup.c +1 -1
index 7a505443d271..094e5174fd7f 100644
@@ -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;
Signature · 1 line
2.18.0
author
#3 lore raw

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

In reply to 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.

---
 Documentation/btrfs-qgroup.asciidoc | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)
Documentation/btrfs-qgroup.asciidoc +21 -1
index fee173860000..d1c8fedc94d4 100644
@@ -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
Signature · 1 line
2.18.0
Applied
#4 lore raw
In reply to Qu Wenruo
Quoted 5 lines · 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)

Quoted 8 lines

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.