Skip to content

Fix issue of incorrect qgroup detection in #29835#30497

Closed
yongtang wants to merge 1 commit into
moby:masterfrom
yongtang:29835-fix
Closed

Fix issue of incorrect qgroup detection in #29835#30497
yongtang wants to merge 1 commit into
moby:masterfrom
yongtang:29835-fix

Conversation

@yongtang

@yongtang yongtang commented Jan 27, 2017

Copy link
Copy Markdown
Member

In #29835 BTRFS_IOC_INO_LOOKUP was used for qgroup detection.
However, this is not necessarily true because it only indicates the existance of tree id in btrfs.

Overall, the way to test the qgroup is to use btrfs qgroup show <path> and see if it returns something. The following code offers insight in checking qgroup:

https://github.com/kdave/btrfs-progs/blob/v4.9/qgroup.c#L1035

This fix tries to fix the issue by using BTRFS_IOC_TREE_SEARCH with BTRFS_QGROUP_STATUS_KEY instead.

In case qgroup is enabled, the search for BTRFS_QGROUP_STATUS_KEY should be valid.

Signed-off-by: Yong Tang [email protected]

/cc @cpuguy83 @tonistiigi @zhuguihua
Please take a look and sorry for the trouble and inconvenience the #29835 caused.

In 29835 `BTRFS_IOC_INO_LOOKUP` was used for qgroup detection.
However, this is not necessarily true because it only indicate
the existance of tree id.

This fix tries to fix the issue by using `BTRFS_IOC_TREE_SEARCH`
with `BTRFS_QGROUP_STATUS_KEY` instead. In case qgroup is enabled,
the search for `BTRFS_QGROUP_STATUS_KEY` should return valid.

Signed-off-by: Yong Tang <[email protected]>
@yongtang

Copy link
Copy Markdown
Member Author

Also, I am not familiar with Jenkins test setup for docker but it might make sense to add integration tests for storage drivers like btrfs. Not sure if it is possible though.

@vdemeester vdemeester added the status/failing-ci Indicates that the PR in its current state fails the test suite label Feb 7, 2017
@yongtang

Copy link
Copy Markdown
Member Author

This PR could be superseded by #29427.

@vdemeester

Copy link
Copy Markdown
Member

@yongtang should we close this one ? 👼

@yongtang

Copy link
Copy Markdown
Member Author

Thanks @vdemeester. I will close this PR for now. Ideally we want to make sure #29427 is merged before the release of 1.14.0, so that the issue could be addressed.

@yongtang yongtang closed this Feb 13, 2017
@yongtang
yongtang deleted the 29835-fix branch May 25, 2017 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/failing-ci Indicates that the PR in its current state fails the test suite status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants