| Message ID | eeaa9ab1-f6a5-9043-9b5e-cc9acf82ca47@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: qgroup: Remove qgroup item along with subvolume deletion | expand |
On 2018年08月03日 12:08, Misono Tomohiro wrote: > When qgroup is on, subvolume deletion does not remove qgroup items > of the subvolume (qgroup info, limits, relation) from quota tree and > they need to get removed manually by "btrfs qgroup destroy". > > Since level 0 qgroup cannot be used/inherited by any other subvolume, > let's remove them automatically when subvolume is deleted. > > Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> > --- > > I don't see any reason to keep these items after subvolume deletion, > but is there something I'm missing? > > fs/btrfs/inode.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 3f51ddc18f98..4ec60a1b53a3 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -4372,6 +4372,10 @@ int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry) > } > } > > + ret = btrfs_remove_qgroup(trans, dest->root_key.objectid); According to the caller, it only unlinks the subvolume without really delete the whole subvolume. I'm wondering if we should call btrfs_remove_qgroup() only after we have deleted the whole subvolume, e.g inside btrfs_drop_snapshot(). Thanks, Qu > + if (ret == -EINVAL || ret == -ENOENT) > + ret = 0; > + > out_end_trans: > trans->block_rsv = NULL; > trans->bytes_reserved = 0; >
On Fri, Aug 03, 2018 at 12:17:26PM +0800, Qu Wenruo wrote: > > >On 2018年08月03日 12:08, Misono Tomohiro wrote: >> When qgroup is on, subvolume deletion does not remove qgroup items >> of the subvolume (qgroup info, limits, relation) from quota tree and >> they need to get removed manually by "btrfs qgroup destroy". >> >> Since level 0 qgroup cannot be used/inherited by any other subvolume, >> let's remove them automatically when subvolume is deleted. >> >> Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> >> --- >> >> I don't see any reason to keep these items after subvolume deletion, >> but is there something I'm missing? >> >> fs/btrfs/inode.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >> index 3f51ddc18f98..4ec60a1b53a3 100644 >> --- a/fs/btrfs/inode.c >> +++ b/fs/btrfs/inode.c >> @@ -4372,6 +4372,10 @@ int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry) >> } >> } >> >> + ret = btrfs_remove_qgroup(trans, dest->root_key.objectid); > >According to the caller, it only unlinks the subvolume without really >delete the whole subvolume. > >I'm wondering if we should call btrfs_remove_qgroup() only after we have >deleted the whole subvolume, e.g inside btrfs_drop_snapshot(). I agree with Qu's point, because the ongoing online undelete subvolume will be able to recover the subvolume which is intact ondisk, including the qgroup.
On 2018/08/03 13:23, Lu Fengqi wrote: > On Fri, Aug 03, 2018 at 12:17:26PM +0800, Qu Wenruo wrote: >> >> >> On 2018年08月03日 12:08, Misono Tomohiro wrote: >>> When qgroup is on, subvolume deletion does not remove qgroup items >>> of the subvolume (qgroup info, limits, relation) from quota tree and >>> they need to get removed manually by "btrfs qgroup destroy". >>> >>> Since level 0 qgroup cannot be used/inherited by any other subvolume, >>> let's remove them automatically when subvolume is deleted. >>> >>> Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> >>> --- >>> >>> I don't see any reason to keep these items after subvolume deletion, >>> but is there something I'm missing? >>> >>> fs/btrfs/inode.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c >>> index 3f51ddc18f98..4ec60a1b53a3 100644 >>> --- a/fs/btrfs/inode.c >>> +++ b/fs/btrfs/inode.c >>> @@ -4372,6 +4372,10 @@ int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry) >>> } >>> } >>> >>> + ret = btrfs_remove_qgroup(trans, dest->root_key.objectid); >> >> According to the caller, it only unlinks the subvolume without really >> delete the whole subvolume. >> >> I'm wondering if we should call btrfs_remove_qgroup() only after we have >> deleted the whole subvolume, e.g inside btrfs_drop_snapshot(). > > I agree with Qu's point, because the ongoing online undelete subvolume will > be able to recover the subvolume which is intact ondisk, including the qgroup. > Thanks to both of you, I'll update the patch. Misono -- 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/fs/btrfs/inode.c b/fs/btrfs/inode.c index 3f51ddc18f98..4ec60a1b53a3 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4372,6 +4372,10 @@ int btrfs_delete_subvolume(struct inode *dir, struct dentry *dentry) } } + ret = btrfs_remove_qgroup(trans, dest->root_key.objectid); + if (ret == -EINVAL || ret == -ENOENT) + ret = 0; + out_end_trans: trans->block_rsv = NULL; trans->bytes_reserved = 0;
When qgroup is on, subvolume deletion does not remove qgroup items of the subvolume (qgroup info, limits, relation) from quota tree and they need to get removed manually by "btrfs qgroup destroy". Since level 0 qgroup cannot be used/inherited by any other subvolume, let's remove them automatically when subvolume is deleted. Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> --- I don't see any reason to keep these items after subvolume deletion, but is there something I'm missing? fs/btrfs/inode.c | 4 ++++ 1 file changed, 4 insertions(+)