On Sun, May 21, 2017 at 5:59 PM, Qu Wenruo <quwen...@cn.fujitsu.com> wrote: > > > At 05/19/2017 05:07 PM, Sargun Dhillon wrote: >> >> I have some questions about the *intended* qgroups semantics, and why >> we allow certain operations: >> >> 1) Why can you create a level 0 qgroup for a non-existent subvolume? >> It looks like it just gets reset when you create the subvol anyway? >> Should we prevent this? > > > Personally speaking, I didn't see much meaning of allowing this, except > setting limit before creating the subvolume. > > But that can also be done by setting up higher level qgroup and attach new > subvolume to that higher level qgroup. > > IIRC there are some guys hoping to disable multi-level qgroup completely, if > one day we decide to do that, then current behavior may be quite important. > >> >> 2) Why do we allow deleting a level 0 qgroup for a currently existing >> subvolume? It seems to me that just setting the limit to 0, and/or >> removing relations would work equally well? Perhaps a new ioctl to >> clear the qgroup would make sense to do this. > > > At least I didn't see the meaning to allow deleting qgroup for existing > subvolume. > > It won't even improve any performance since we still need to do the time > consuming backref walk. > >> >> 3) Why don't we remove the associated level 0 qgroup when you remove >> the subvol with that id? > > This may be related to question 1). > Sometimes we may want to keep the limit? > > Despite of that, I didn't see any reason to keep the orphan level 0 qgroup. > >> >> 4) I noticed in qgroup.c, one of the outstanding items is to determine >> how to autocleanup. Are there any stated semantics around that, or >> opinions? > > > As long as we're going to stick with multi level qgroups, then autocleanup > seems to be a good idea. > > Thanks, > Qu > I propose the following changes: 1) We always cleanup level-0 qgroups by default, with no opt-out. I see absolutely no reason to keep these around.
2) We do not allow the creation of level-0 qgroups for (sub)volumes that do not exist. If people want to have a limit at creation time, they should create a higher level qgroup, and add the new qgroup to that one. 3) We add two new ioctls: qgroup_create_v2, and qgroup_delete_v2, and add a pr_warn_once message when we see usage of the old API. 4) Add a flag to the qgroup_delete_v2 ioctl, NO_SUBVOL_CHECK. If the flag is present, it will allow you to delete qgroups which reference active subvolumes. Opinions? >> >> PS: >> If the answer to any of these is "it just needs to be worked on" -- >> I'm currently poking around this code path for some enhancements we're >> doing for our usecase. I'm just trying to figure out how much of what >> I'm doing is generalizable. >> >> -Thanks, >> Sargun >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
