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>
xiexingguo
force-pushed
the
xxg-wip-14176
branch
from
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.
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. |
xiexingguo
force-pushed
the
xxg-wip-14176
branch
from
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. |
xiexingguo
force-pushed
the
xxg-wip-14176
branch
from
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>
ghost
changed the title
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