Bug #13715
RadosClient: result code overflowed!
0%
Description
bool librados::RadosClient::pool_requires_alignment(int64_t pool_id)
{
int r = wait_for_osdmap();
if (r < 0) {
return r;
}
See above, the negative answers given by wait_for_osdmap() are directly outputted here,
which are therefore implicitly casted into bool type and may mislead caller
as we really don't know the answer yet.
The pool_required_alignment() method has the same problem:
uint64_t librados::RadosClient::pool_required_alignment(int64_t pool_id)
{
int r = wait_for_osdmap();
if (r < 0) {
return r;
}
If wait_for_osdmap() fails, as shown above, pool_required_alignment() can
produce large overflowed results, which might be disastrous if they are
unconditionally trusted by caller.
The possible solutions are as follows:
(1) Make sure that wait_for_map() never fails.If it does, then retry until
it succeeds.
(2) Explicitly cast error codes into context-safe result codes.
For example:
bool librados::RadosClient::pool_requires_alignment(int64_t pool_id)
{
int r = wait_for_osdmap();
if (r < 0) {
// lie to caller, we really don't know the answer yet.
return false;
}
(3) Refacter these two methods, let error codes can be safely outputed and
rightfully handled by caller.
For example:
int librados::RadosClient::pool_required_alignment(
int64_t pool_id, uint64_t *alignment)
{
int r = wait_for_osdmap();
if (r < 0)
{
return r;
}
const OSDMap *osdmap = objecter->get_osdmap_read();
*alignment = osdmap->have_pg_pool(pool_id) ?
osdmap->get_pg_pool(pool_id)->required_alignment() : 0;
objecter->put_osdmap_read();
return 0;
}
What's your suggestion?
Associated revisions
RadosClient: result code overflowed
pool_requires_alignment and pool_required_alignment may produce overflowed results and need to refactered.
Fixes: #13715
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
RadosClient: reimplement the pool alignment methods using the new ones
so we don't less duplicated code and the logic reveals the problem of the old methods in a more visible way.
Fixes: #13715
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
History
#1 Updated by xie xingguo over 8 years ago
BTW, RGWRados::get_required_alignment() shall test ioctx.pool_requires_alignment() first,
otherwise the answer is untrustful and might not be rightfully handled by caller.
#2 Updated by Sage Weil over 8 years ago
- Status changed from New to Resolved