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
osd: KeyValueStore: fix wrongly placed assert #7047
Conversation
please rebase |
Fixes: ceph#14178 Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
At present it will not be able to dump the current opened fds when an EMFILE error has encountered. Fixes: ceph#14176 Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
15ee22f
to
d714093
Compare
rebased. |
if (r < 0) | ||
break; | ||
unsigned ret = _do_transaction(**p, bt, handle); | ||
assert(ret == 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily agree with making this an assert(). Either _do_transaction() should have a void return or it could potentially return an error in the future, So we should leave the code the way it was.
👍 besides the 1 commit. |
d714093
to
378e856
Compare
Now that I looked at this again I see that both KeyValueStore::_do_transaction() and FileStore::_do_transaction() have the following as the last line: ''' I don't think we are ever going to return an error or a count, so let's change both to void and remove the FIXME. And fix the corresponding _do_transactions(), of course. |
378e856
to
22aa547
Compare
As _do_transaction will never be able to return a negative result code. It always return zero actually. Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@xiexingguo Do you mind making the same _do_transaction() change in FileStore? |
@dzafman I did make the same change in FileStore in another PR xiexingguo@2334e5d :-) Thank you for your reminder. |
osd: KeyValueStore: fix wrongly placed assert Reviewed-by: David Zafman <dzafman@redhat.com>
At present it will not be able to dump the current opened fds when an EMFILE error has encountered.
Fixes: #14176
Signed-off-by: xie xingguo xie.xingguo@zte.com.cn