| Message ID | 20170520083902.GA4204@ircssh-2.c.rugged-nimbus-611.internal (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
At 05/20/2017 04:39 PM, Sargun Dhillon wrote: > Previously, we were calling del_qgroup_item, and ignoring the return code > resulting in a potential to have divergent in-memory state without an > error. Perhaps, it makes sense to handle this error code, and put the > filesystem into a read only, or similar state. > > This patch only adds reporting of the error. > > Signed-off-by: Sargun Dhillon <sargun@sargun.me> > --- > fs/btrfs/qgroup.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index a2add44..7e772ba 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1303,6 +1303,8 @@ static int __btrfs_remove_qgroup(struct btrfs_trans_handle *trans, > return -EBUSY; > > ret = del_qgroup_item(trans, quota_root, qgroupid); > + if (ret) > + goto out; I think we should continue to remove in-memory qgroup relationship even we didn't find corresponding qgroup item. IIRC it's better to catch less severe error like -ENOENT and continue. Although normally previous find_qgroup_rb() should return -ENOENT before we continue to del_qgroup_item(), but catching -ENOENT here could make the code more robust. Thanks, Qu > > while (!list_empty(&qgroup->groups)) { > list = list_first_entry(&qgroup->groups, > -- 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
On Sun, May 21, 2017 at 6:14 PM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote: > > > At 05/20/2017 04:39 PM, Sargun Dhillon wrote: >> >> Previously, we were calling del_qgroup_item, and ignoring the return code >> resulting in a potential to have divergent in-memory state without an >> error. Perhaps, it makes sense to handle this error code, and put the >> filesystem into a read only, or similar state. >> >> This patch only adds reporting of the error. >> >> Signed-off-by: Sargun Dhillon <sargun@sargun.me> >> --- >> fs/btrfs/qgroup.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c >> index a2add44..7e772ba 100644 >> --- a/fs/btrfs/qgroup.c >> +++ b/fs/btrfs/qgroup.c >> @@ -1303,6 +1303,8 @@ static int __btrfs_remove_qgroup(struct >> btrfs_trans_handle *trans, >> return -EBUSY; >> ret = del_qgroup_item(trans, quota_root, qgroupid); >> + if (ret) >> + goto out; > > > I think we should continue to remove in-memory qgroup relationship even we > didn't find corresponding qgroup item. > > IIRC it's better to catch less severe error like -ENOENT and continue. > > Although normally previous find_qgroup_rb() should return -ENOENT before we > continue to del_qgroup_item(), but catching -ENOENT here could make the code > more robust. Agreed. Will fix this. I think it also return ENOMEM. > > Thanks, > Qu > > >> while (!list_empty(&qgroup->groups)) { >> list = list_first_entry(&qgroup->groups, >> > > -- 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/qgroup.c b/fs/btrfs/qgroup.c index a2add44..7e772ba 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1303,6 +1303,8 @@ static int __btrfs_remove_qgroup(struct btrfs_trans_handle *trans, return -EBUSY; ret = del_qgroup_item(trans, quota_root, qgroupid); + if (ret) + goto out; while (!list_empty(&qgroup->groups)) { list = list_first_entry(&qgroup->groups,
Previously, we were calling del_qgroup_item, and ignoring the return code resulting in a potential to have divergent in-memory state without an error. Perhaps, it makes sense to handle this error code, and put the filesystem into a read only, or similar state. This patch only adds reporting of the error. Signed-off-by: Sargun Dhillon <sargun@sargun.me> --- fs/btrfs/qgroup.c | 2 ++ 1 file changed, 2 insertions(+)