Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/bluestore: prevent statfs available from going negative #20487

Merged
merged 2 commits into from Feb 21, 2018

Conversation

liewegas
Copy link
Member

Fixes: https://tracker.ceph.com/issues/23040
Signed-off-by: Sage Weil sage@redhat.com

@@ -6399,7 +6399,11 @@ int BlueStore::statfs(struct store_statfs_t *buf)
// part of our shared device is "free" according to BlueFS
// Don't include bluestore_bluefs_min because that space can't
// be used for any other purpose.
buf->available += bluefs->get_free(bluefs_shared_bdev) - cct->_conf->bluestore_bluefs_min;
int64_t shared_available = bluefs->get_free(bluefs_shared_bdev) -
cct->_conf->bluestore_bluefs_min;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO (if I understand bluestore_bluefs_min semantics properly) generally that's improper use of bluestore_bluefs_min. Consider two cases (when bluefs_shared_bdev = 1Gb):

  1. bluefs->get_total() = 2Gb and bluefs->get_free() = 0.5Gb. This should result in shared_available = 0.5Gb
  2. bluefs->get_total() = 2Gb and bluefs->get_free() = 1.5Gb => shared_available to be = 1Gb
    So looks like one should take get_total() into account when adjusting using bluestore_bluefs_min.
    I.e. something like
    shared_available = min(get_free(), get_total() - bluestore_bluefs_min);

@ifed01
Copy link
Contributor

ifed01 commented Feb 19, 2018

And as you're touching statfs function may be it makes sense to cherry-pick this commit
dd78698

We had some bug reports and corresponding discussion at dev-list about that a while ago....

… statfs method

Signed-off-by: Igor Fedotov <ifedotov@suse.com>
@liewegas liewegas merged commit f798b13 into ceph:master Feb 21, 2018
liewegas added a commit that referenced this pull request Feb 21, 2018
* refs/pull/20487/head:
	os/bluestore: do not account DB volume space in total one reported by statfs method
	os/bluestore: prevent statfs available from going negative

Reviewed-by: Igor Fedotov <ifedotov@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants