* [PATCH 1/3] btrfs: qgroup: return EINVAL if level of parent is not higher than child's. @ 2015-01-22 5:46 Dongsheng Yang 2015-01-22 5:46 ` [PATCH 2/3] btrfs: qgroup: allow user to remove qgroup which has parent but no child Dongsheng Yang 2015-01-22 5:46 ` [PATCH 3/3] btrfs: qgroup: destroy related qgroup when removing subvolume if needed Dongsheng Yang 0 siblings, 2 replies; 5+ messages in thread From: Dongsheng Yang @ 2015-01-22 5:46 UTC (permalink / raw) To: linux-btrfs; +Cc: Dongsheng Yang When we create a qgroup inheriting a qgroup, we need to check the level of them. Otherwise, there is a chance where a qgroup can inherit another qgroup at the same level. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/btrfs/qgroup.c | 6 ++++++ fs/btrfs/qgroup.h | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index 953befd..c2983a4 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -2221,6 +2221,12 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, ret = -EINVAL; goto out; } + + if ((srcgroup->qgroupid >> BTRFS_QGROUP_LEVEL_SHIFT) <= + (objectid >> BTRFS_QGROUP_LEVEL_SHIFT)) { + ret = -EINVAL; + goto out; + } ++i_qgroups; } } diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h index 18cc68c..67291ff 100644 --- a/fs/btrfs/qgroup.h +++ b/fs/btrfs/qgroup.h @@ -20,6 +20,15 @@ #define __BTRFS_QGROUP__ /* + * The ID of a qgroup consists of a level and a id, looks like: 1/3. + * The 1 is the level means this qgroup is in the level 1 and the 3 + * is the id of this qgroup in level 1. We use a u64 number store a + * qgroup ID, the first 16bits stands for the level and the last 48bits + * stands for the id. + */ +#define BTRFS_QGROUP_LEVEL_SHIFT 48 + +/* * A description of the operations, all of these operations only happen when we * are adding the 1st reference for that subvolume in the case of adding space * or on the last reference delete in the case of subtraction. The only -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] btrfs: qgroup: allow user to remove qgroup which has parent but no child. 2015-01-22 5:46 [PATCH 1/3] btrfs: qgroup: return EINVAL if level of parent is not higher than child's Dongsheng Yang @ 2015-01-22 5:46 ` Dongsheng Yang 2015-01-22 5:46 ` [PATCH 3/3] btrfs: qgroup: destroy related qgroup when removing subvolume if needed Dongsheng Yang 1 sibling, 0 replies; 5+ messages in thread From: Dongsheng Yang @ 2015-01-22 5:46 UTC (permalink / raw) To: linux-btrfs; +Cc: Dongsheng Yang When a qgroup has parents but no child, it should be removable in Theory. But currently, we can not remove it when it has either parent or child. Example: # btrfs quota enable /mnt # btrfs qgroup create 1/0 /mnt # btrfs qgroup create 2/0 /mnt # btrfs qgroup assign 1/0 2/0 /mnt # btrfs qgroup show -pcre /mnt qgroupid rfer excl max_rfer max_excl parent child -------- ---- ---- -------- -------- ------ ----- 0/5 16384 16384 0 0 --- --- 1/0 0 0 0 0 2/0 --- 2/0 0 0 0 0 --- 1/0 At this time, there is no subvol or qgroup depending on qgroup 1/0. Just a qgroup 2/0 is its parent, but 2/0 can work well without 1/0. So I think 1/0 should be removalbe. But: # btrfs qgroup destroy 1/0 /mnt ERROR: unable to destroy quota group: Device or resource busy This patch remove the check of qgroup->parent in removing it. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/btrfs/qgroup.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index c2983a4..c3b1e4f 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1048,7 +1048,7 @@ out: return ret; } -int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans, +int __del_qgroup_relation(struct btrfs_trans_handle *trans, struct btrfs_fs_info *fs_info, u64 src, u64 dst) { struct btrfs_root *quota_root; @@ -1058,7 +1058,6 @@ int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans, int ret = 0; int err; - mutex_lock(&fs_info->qgroup_ioctl_lock); quota_root = fs_info->quota_root; if (!quota_root) { ret = -EINVAL; @@ -1089,7 +1088,18 @@ exist: del_relation_rb(fs_info, src, dst); spin_unlock(&fs_info->qgroup_lock); out: + return ret; +} + +int btrfs_del_qgroup_relation(struct btrfs_trans_handle *trans, + struct btrfs_fs_info *fs_info, u64 src, u64 dst) +{ + int ret = 0; + + mutex_lock(&fs_info->qgroup_ioctl_lock); + ret = __del_qgroup_relation(trans, fs_info, src, dst); mutex_unlock(&fs_info->qgroup_ioctl_lock); + return ret; } @@ -1132,6 +1142,7 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, { struct btrfs_root *quota_root; struct btrfs_qgroup *qgroup; + struct btrfs_qgroup_list *list; int ret = 0; mutex_lock(&fs_info->qgroup_ioctl_lock); @@ -1146,15 +1157,24 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, ret = -ENOENT; goto out; } else { - /* check if there are no relations to this qgroup */ - if (!list_empty(&qgroup->groups) || - !list_empty(&qgroup->members)) { + /* check if there are no children of this qgroup */ + if (!list_empty(&qgroup->members)) { ret = -EBUSY; goto out; } } ret = del_qgroup_item(trans, quota_root, qgroupid); + while (!list_empty(&qgroup->groups)) { + list = list_first_entry(&qgroup->groups, + struct btrfs_qgroup_list, next_group); + ret = __del_qgroup_relation(trans, fs_info, + qgroupid, + list->group->qgroupid); + if (ret) + goto out; + } + spin_lock(&fs_info->qgroup_lock); del_qgroup_rb(quota_root->fs_info, qgroupid); spin_unlock(&fs_info->qgroup_lock); -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] btrfs: qgroup: destroy related qgroup when removing subvolume if needed. 2015-01-22 5:46 [PATCH 1/3] btrfs: qgroup: return EINVAL if level of parent is not higher than child's Dongsheng Yang 2015-01-22 5:46 ` [PATCH 2/3] btrfs: qgroup: allow user to remove qgroup which has parent but no child Dongsheng Yang @ 2015-01-22 5:46 ` Dongsheng Yang 2015-01-22 6:16 ` Wang Shilong 1 sibling, 1 reply; 5+ messages in thread From: Dongsheng Yang @ 2015-01-22 5:46 UTC (permalink / raw) To: linux-btrfs; +Cc: Dongsheng Yang When removing a subvol, if the related qgroup has no child qgroup, we should destroy it at the same time. Also remove the TODO entry in qgroup.c. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/btrfs/inode.c | 4 ++++ fs/btrfs/qgroup.c | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index e687bb0..31de211 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -59,6 +59,7 @@ #include "backref.h" #include "hash.h" #include "props.h" +#include "qgroup.h" struct btrfs_iget_args { struct btrfs_key *location; @@ -4029,6 +4030,9 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans, ret = btrfs_update_inode_fallback(trans, root, dir); if (ret) btrfs_abort_transaction(trans, root, ret); + ret = btrfs_remove_qgroup(trans, root->fs_info, objectid); + if (ret == -EBUSY) + ret = 0; out: btrfs_free_path(path); return ret; diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index c3b1e4f..303c078 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -35,7 +35,6 @@ #include "qgroup.h" /* TODO XXX FIXME - * - subvol delete -> delete when ref goes to 0? delete limits also? * - reorganize keys * - compressed * - sync -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] btrfs: qgroup: destroy related qgroup when removing subvolume if needed. 2015-01-22 5:46 ` [PATCH 3/3] btrfs: qgroup: destroy related qgroup when removing subvolume if needed Dongsheng Yang @ 2015-01-22 6:16 ` Wang Shilong 2015-01-22 9:46 ` Dongsheng Yang 0 siblings, 1 reply; 5+ messages in thread From: Wang Shilong @ 2015-01-22 6:16 UTC (permalink / raw) To: Dongsheng Yang; +Cc: linux-btrfs Hello, > > When removing a subvol, if the related qgroup has no child qgroup, we should destroy > it at the same time. Also remove the TODO entry in qgroup.c. My two cents here: Take a quick look at this, i am not sure whether this is right way to do it. Actually, i think we can only remove a subvolume qgroup safely when both refer and excl are 0, because for subvolume removal case, to make qgroup accounting right, we need walk tree. At this time, qgroup walking might have not finished, if we remove this qgroup directly, accounting for this qgroup will be skipped. Maybe we could remove such qgroup when at qgroup accounting, we find an qgroup’s refer and excl are 0, we could remove it, or we do it at the background.. Also, there is another risk, that is to say, if qgroup accounting not work right, So we might need gurantee that subvolume it going to be deleted or have been deleted at the same time, then we could remove qgroup safely etc. > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> > --- > fs/btrfs/inode.c | 4 ++++ > fs/btrfs/qgroup.c | 1 - > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index e687bb0..31de211 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -59,6 +59,7 @@ > #include "backref.h" > #include "hash.h" > #include "props.h" > +#include "qgroup.h" > > struct btrfs_iget_args { > struct btrfs_key *location; > @@ -4029,6 +4030,9 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans, > ret = btrfs_update_inode_fallback(trans, root, dir); > if (ret) > btrfs_abort_transaction(trans, root, ret); > + ret = btrfs_remove_qgroup(trans, root->fs_info, objectid); > + if (ret == -EBUSY) > + ret = 0; > out: > btrfs_free_path(path); > return ret; > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index c3b1e4f..303c078 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -35,7 +35,6 @@ > #include "qgroup.h" > > /* TODO XXX FIXME > - * - subvol delete -> delete when ref goes to 0? delete limits also? > * - reorganize keys > * - compressed > * - sync > -- > 1.8.4.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 Best Regards, Wang Shilong ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] btrfs: qgroup: destroy related qgroup when removing subvolume if needed. 2015-01-22 6:16 ` Wang Shilong @ 2015-01-22 9:46 ` Dongsheng Yang 0 siblings, 0 replies; 5+ messages in thread From: Dongsheng Yang @ 2015-01-22 9:46 UTC (permalink / raw) To: Wang Shilong; +Cc: linux-btrfs On 01/22/2015 02:16 PM, Wang Shilong wrote: > Hello, > >> When removing a subvol, if the related qgroup has no child qgroup, we should destroy >> it at the same time. Also remove the TODO entry in qgroup.c. > My two cents here: > > Take a quick look at this, i am not sure whether this is right way to do it. > > Actually, i think we can only remove a subvolume qgroup safely when both refer and excl are 0, > because for subvolume removal case, to make qgroup accounting right, we need walk tree. > > At this time, qgroup walking might have not finished, if we remove this qgroup directly, > accounting for this qgroup will be skipped. Good point! > > Maybe we could remove such qgroup when at qgroup accounting, we find an qgroup’s refer and excl > are 0, we could remove it, or we do it at the background.. > > Also, there is another risk, that is to say, if qgroup accounting not work right, So > we might need gurantee that subvolume it going to be deleted or have been deleted at the same time, > then we could remove qgroup safely etc. Yes, you are right. This is not a good timing to remove the qgroup directly. I need some more investigation for a better solution. As I have some emergency to deal with in this period, maybe the V2 will come a little late. Sorry about it. Thanx Yang > > >> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> >> --- >> fs/btrfs/inode.c | 4 ++++ >> fs/btrfs/qgroup.c | 1 - >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> index e687bb0..31de211 100644 >> --- a/fs/btrfs/inode.c >> +++ b/fs/btrfs/inode.c >> @@ -59,6 +59,7 @@ >> #include "backref.h" >> #include "hash.h" >> #include "props.h" >> +#include "qgroup.h" >> >> struct btrfs_iget_args { >> struct btrfs_key *location; >> @@ -4029,6 +4030,9 @@ int btrfs_unlink_subvol(struct btrfs_trans_handle *trans, >> ret = btrfs_update_inode_fallback(trans, root, dir); >> if (ret) >> btrfs_abort_transaction(trans, root, ret); >> + ret = btrfs_remove_qgroup(trans, root->fs_info, objectid); >> + if (ret == -EBUSY) >> + ret = 0; >> out: >> btrfs_free_path(path); >> return ret; >> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >> index c3b1e4f..303c078 100644 >> --- a/fs/btrfs/qgroup.c >> +++ b/fs/btrfs/qgroup.c >> @@ -35,7 +35,6 @@ >> #include "qgroup.h" >> >> /* TODO XXX FIXME >> - * - subvol delete -> delete when ref goes to 0? delete limits also? >> * - reorganize keys >> * - compressed >> * - sync >> -- >> 1.8.4.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 > Best Regards, > Wang Shilong > > . > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-22 9:49 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-22 5:46 [PATCH 1/3] btrfs: qgroup: return EINVAL if level of parent is not higher than child's Dongsheng Yang 2015-01-22 5:46 ` [PATCH 2/3] btrfs: qgroup: allow user to remove qgroup which has parent but no child Dongsheng Yang 2015-01-22 5:46 ` [PATCH 3/3] btrfs: qgroup: destroy related qgroup when removing subvolume if needed Dongsheng Yang 2015-01-22 6:16 ` Wang Shilong 2015-01-22 9:46 ` Dongsheng Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).