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: misc fixes #10771

Merged
merged 4 commits into from Aug 21, 2016
Merged

Conversation

xiexingguo
Copy link
Member

We shall round nbytes up to be page-size(4096) aligned instead of
truncating it down, as syncing more is usually okay
while syncing less indicates potential data loss.

Signed-off-by: xie xingguo xie.xingguo@zte.com.cn

@liewegas
Copy link
Member

I think this is a poorly named function.. the comment says

// This asks the OS to initiate flushing the cached data to disk,
// without waiting for completion.

so RangeStartFlush would have been better. In any case, it's not a data integrity call, so I think it's okay as it.

What I want to avoid is flushing the same block twice...

file_map should be accessed under the protection
of the global lock.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
This assert is used to guarantee that we don't access violation
but currently has no effect.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@xiexingguo xiexingguo changed the title os/bluestore: fix incomplete sync of specified range os/bluestore: misc fixes Aug 19, 2016
@xiexingguo
Copy link
Member Author

xiexingguo commented Aug 19, 2016

@liewegas Dropped that change and appended more changes.
Also make sure it passed local test:

2016-08-19 17:23:14.297700 7fc61433c640  1 bluestore(store_test_temp_dir) fsck finish with 0 errors
==> rm -r store_test_temp_dir
[       OK ] ObjectStore/StoreTest.TooManyBlobsTest/2 (21392 ms)
[----------] 60 tests from ObjectStore/StoreTest (1962746 ms total)

[----------] Global test environment tear-down
[==========] 60 tests from 1 test case ran. (1962746 ms total)
[  PASSED  ] 60 tests.

derr << __func__ << " allocated: " << allocated << \
" offset: " << offset << " length: " << length << dendl;
derr << __func__ << " allocated: 0x" << std::hex << allocated
<< " offset: 0x" << offset << " length: 0x" << length << std::dec
Copy link
Member

Choose a reason for hiding this comment

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

please include the std::dec at the end

Copy link
Member Author

Choose a reason for hiding this comment

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

@liewegas Sir, I did... The std::dec lies at the end of this line.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, yes, sorry about that!

@liewegas
Copy link
Member

otherwise lgtm, thanks!

@liewegas liewegas merged commit 07a06d1 into ceph:master Aug 21, 2016
@xiexingguo xiexingguo deleted the xxg-wip-fix-bluerockenv branch August 22, 2016 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants