From 1f855456cae96c91a67c169d2333e333c3b59671 Mon Sep 17 00:00:00 2001 From: xiexingguo <258156334@qq.com> Date: Sat, 7 Nov 2015 15:30:03 +0800 Subject: [PATCH] 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 --- src/include/rados/librados.h | 29 ++++++++++++++++++++-- src/include/rados/librados.hpp | 2 ++ src/librados/RadosClient.cc | 44 ++++++++++++++++++++++++++++++++++ src/librados/RadosClient.h | 2 ++ src/librados/librados.cc | 34 ++++++++++++++++++++++++++ src/rgw/rgw_rados.cc | 22 ++++++++++++++++- src/tools/rados/rados.cc | 33 ++++++++++++++++++------- src/tracing/librados.tp | 36 ++++++++++++++++++++++++++++ 8 files changed, 191 insertions(+), 11 deletions(-) diff --git a/src/include/rados/librados.h b/src/include/rados/librados.h index 5d4ac753a9607..12d5970e0dc4c 100644 --- a/src/include/rados/librados.h +++ b/src/include/rados/librados.h @@ -809,8 +809,33 @@ CEPH_RADOS_API int rados_ioctx_pool_set_auid(rados_ioctx_t io, uint64_t auid); */ CEPH_RADOS_API int rados_ioctx_pool_get_auid(rados_ioctx_t io, uint64_t *auid); -CEPH_RADOS_API int rados_ioctx_pool_requires_alignment(rados_ioctx_t io); -CEPH_RADOS_API uint64_t rados_ioctx_pool_required_alignment(rados_ioctx_t io); +/* deprecated, use rados_ioctx_pool_requires_alignment2 instead */ +CEPH_RADOS_API int rados_ioctx_pool_requires_alignment(rados_ioctx_t io) + __attribute__((deprecated)); + +/** + * Test whether the specified pool requires alignment or not. + * + * @param io pool to query + * @param requires 1 if alignment is supported, 0 if not. + * @returns 0 on success, negative error code on failure + */ +CEPH_RADOS_API int rados_ioctx_pool_requires_alignment2(rados_ioctx_t io, + int *requires); + +/* deprecated, use rados_ioctx_pool_required_alignment2 instead */ +CEPH_RADOS_API uint64_t rados_ioctx_pool_required_alignment(rados_ioctx_t io) + __attribute__((deprecated)); + +/** + * Get the alignment flavor of a pool + * + * @param io pool to query + * @param alignment where to store the alignment flavor + * @returns 0 on success, negative error code on failure + */ +CEPH_RADOS_API int rados_ioctx_pool_required_alignment2(rados_ioctx_t io, + uint64_t *alignment); /** * Get the pool id of the io context diff --git a/src/include/rados/librados.hpp b/src/include/rados/librados.hpp index 52ce642934c88..faa5e2842d464 100644 --- a/src/include/rados/librados.hpp +++ b/src/include/rados/librados.hpp @@ -647,7 +647,9 @@ namespace librados std::string get_pool_name(); bool pool_requires_alignment(); + int pool_requires_alignment2(bool * requires); uint64_t pool_required_alignment(); + int pool_required_alignment2(uint64_t * alignment); // create an object int create(const std::string& oid, bool exclusive); diff --git a/src/librados/RadosClient.cc b/src/librados/RadosClient.cc index a01ae7d458136..09128e2a0bd1c 100644 --- a/src/librados/RadosClient.cc +++ b/src/librados/RadosClient.cc @@ -106,6 +106,28 @@ bool librados::RadosClient::pool_requires_alignment(int64_t pool_id) return ret; } +// a safer version of pool_requires_alignment +int librados::RadosClient::pool_requires_alignment2(int64_t pool_id, + bool *requires) +{ + if (!requires) + return -EINVAL; + + int r = wait_for_osdmap(); + if (r < 0) { + return r; + } + + const OSDMap *osdmap = objecter->get_osdmap_read(); + if (!osdmap->have_pg_pool(pool_id)) { + objecter->put_osdmap_read(); + return -ENOENT; + } + *requires = osdmap->get_pg_pool(pool_id)->requires_aligned_append(); + objecter->put_osdmap_read(); + return 0; +} + uint64_t librados::RadosClient::pool_required_alignment(int64_t pool_id) { int r = wait_for_osdmap(); @@ -120,6 +142,28 @@ uint64_t librados::RadosClient::pool_required_alignment(int64_t pool_id) return ret; } +// a safer version of pool_required_alignment +int librados::RadosClient::pool_required_alignment2(int64_t pool_id, + uint64_t *alignment) +{ + if (!alignment) + return -EINVAL; + + int r = wait_for_osdmap(); + if (r < 0) { + return r; + } + + const OSDMap *osdmap = objecter->get_osdmap_read(); + if (!osdmap->have_pg_pool(pool_id)) { + objecter->put_osdmap_read(); + return -ENOENT; + } + *alignment = osdmap->get_pg_pool(pool_id)->required_alignment(); + objecter->put_osdmap_read(); + return 0; +} + int librados::RadosClient::pool_get_auid(uint64_t pool_id, unsigned long long *auid) { int r = wait_for_osdmap(); diff --git a/src/librados/RadosClient.h b/src/librados/RadosClient.h index d44336f1977d8..a26e46f61fa1c 100644 --- a/src/librados/RadosClient.h +++ b/src/librados/RadosClient.h @@ -93,7 +93,9 @@ public: int get_fsid(std::string *s); int64_t lookup_pool(const char *name); bool pool_requires_alignment(int64_t pool_id); + int pool_requires_alignment2(int64_t pool_id, bool *requires); uint64_t pool_required_alignment(int64_t pool_id); + int pool_required_alignment2(int64_t pool_id, uint64_t *alignment); int pool_get_auid(uint64_t pool_id, unsigned long long *auid); int pool_get_name(uint64_t pool_id, std::string *auid); diff --git a/src/librados/librados.cc b/src/librados/librados.cc index 86badc2bc84b0..7c1c9c8d8c62e 100644 --- a/src/librados/librados.cc +++ b/src/librados/librados.cc @@ -1086,11 +1086,21 @@ bool librados::IoCtx::pool_requires_alignment() return io_ctx_impl->client->pool_requires_alignment(get_id()); } +int librados::IoCtx::pool_requires_alignment2(bool *requires) +{ + return io_ctx_impl->client->pool_requires_alignment2(get_id(), requires); +} + uint64_t librados::IoCtx::pool_required_alignment() { return io_ctx_impl->client->pool_required_alignment(get_id()); } +int librados::IoCtx::pool_required_alignment2(uint64_t *alignment) +{ + return io_ctx_impl->client->pool_required_alignment2(get_id(), alignment); +} + std::string librados::IoCtx::get_pool_name() { std::string s; @@ -3135,6 +3145,18 @@ extern "C" int rados_ioctx_pool_requires_alignment(rados_ioctx_t io) return retval; } +extern "C" int rados_ioctx_pool_requires_alignment2(rados_ioctx_t io, + int *requires) +{ + tracepoint(librados, rados_ioctx_pool_requires_alignment_enter2, io); + librados::IoCtxImpl *ctx = (librados::IoCtxImpl *)io; + int retval = ctx->client->pool_requires_alignment2(ctx->get_id(), + (bool *)requires); + tracepoint(librados, rados_ioctx_pool_requires_alignment_exit2, retval, + *requires); + return retval; +} + extern "C" uint64_t rados_ioctx_pool_required_alignment(rados_ioctx_t io) { tracepoint(librados, rados_ioctx_pool_required_alignment_enter, io); @@ -3144,6 +3166,18 @@ extern "C" uint64_t rados_ioctx_pool_required_alignment(rados_ioctx_t io) return retval; } +extern "C" int rados_ioctx_pool_required_alignment2(rados_ioctx_t io, + uint64_t *alignment) +{ + tracepoint(librados, rados_ioctx_pool_required_alignment_enter2, io); + librados::IoCtxImpl *ctx = (librados::IoCtxImpl *)io; + int retval = ctx->client->pool_required_alignment2(ctx->get_id(), + alignment); + tracepoint(librados, rados_ioctx_pool_required_alignment_exit2, retval, + *alignment); + return retval; +} + extern "C" void rados_ioctx_locator_set_key(rados_ioctx_t io, const char *key) { tracepoint(librados, rados_ioctx_locator_set_key_enter, io, key); diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index 45fde7a42bd8f..e4740d920d7b6 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -1401,7 +1401,27 @@ int RGWRados::get_required_alignment(rgw_bucket& bucket, uint64_t *alignment) return r; } - *alignment = ioctx.pool_required_alignment(); + bool requires; + r = ioctx.pool_requires_alignment2(&requires); + if (r < 0) { + ldout(cct, 0) << "ERROR: ioctx.pool_requires_alignment2() returned " + << r << dendl; + return r; + } + + if (!requires) { + *alignment = 0; + return 0; + } + + uint64_t align; + r = ioctx.pool_required_alignment2(&align); + if (r < 0) { + ldout(cct, 0) << "ERROR: ioctx.pool_required_alignment2() returned " + << r << dendl; + return r; + } + *alignment = align; return 0; } diff --git a/src/tools/rados/rados.cc b/src/tools/rados/rados.cc index 4f4b0863f7029..d852c304bbbdd 100644 --- a/src/tools/rados/rados.cc +++ b/src/tools/rados/rados.cc @@ -1405,14 +1405,31 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts, goto out; } - // align op_size - if (io_ctx.pool_requires_alignment()) { - const uint64_t align = io_ctx.pool_required_alignment(); - const uint64_t prev_op_size = op_size; - op_size = uint64_t((op_size + align - 1) / align) * align; - // Warn: if user specified and it was rounded - if (prev_op_size != default_op_size && prev_op_size != op_size) - cerr << "INFO: op_size has been rounded to " << op_size << std::endl; + // align op_size + { + bool requires; + ret = io_ctx.pool_requires_alignment2(&requires); + if (ret < 0) { + cerr << "error checking pool alignment requirement" + << cpp_strerror(ret) << std::endl; + goto out; + } + + if (requires) { + uint64_t align = 0; + ret = io_ctx.pool_required_alignment2(&align); + if (ret < 0) { + cerr << "error getting pool alignment" + << cpp_strerror(ret) << std::endl; + goto out; + } + + const uint64_t prev_op_size = op_size; + op_size = uint64_t((op_size + align - 1) / align) * align; + // Warn: if user specified and it was rounded + if (prev_op_size != default_op_size && prev_op_size != op_size) + cerr << "INFO: op_size has been rounded to " << op_size << std::endl; + } } // create striper interface diff --git a/src/tracing/librados.tp b/src/tracing/librados.tp index 0ba22ea0410fb..1b60afcbf66b4 100644 --- a/src/tracing/librados.tp +++ b/src/tracing/librados.tp @@ -1091,6 +1091,24 @@ TRACEPOINT_EVENT(librados, rados_ioctx_pool_requires_alignment_exit, ) ) +TRACEPOINT_EVENT(librados, rados_ioctx_pool_requires_alignment_enter2, + TP_ARGS( + rados_ioctx_t, ioctx), + TP_FIELDS( + ctf_integer_hex(rados_ioctx_t, ioctx, ioctx) + ) +) + +TRACEPOINT_EVENT(librados, rados_ioctx_pool_requires_alignment_exit2, + TP_ARGS( + int, retval, + int, requires), + TP_FIELDS( + ctf_integer(int, retval, retval) + ctf_integer(int, requires, requires) + ) +) + TRACEPOINT_EVENT(librados, rados_ioctx_pool_required_alignment_enter, TP_ARGS( rados_ioctx_t, ioctx), @@ -1107,6 +1125,24 @@ TRACEPOINT_EVENT(librados, rados_ioctx_pool_required_alignment_exit, ) ) +TRACEPOINT_EVENT(librados, rados_ioctx_pool_required_alignment_enter2, + TP_ARGS( + rados_ioctx_t, ioctx), + TP_FIELDS( + ctf_integer_hex(rados_ioctx_t, ioctx, ioctx) + ) +) + +TRACEPOINT_EVENT(librados, rados_ioctx_pool_required_alignment_exit2, + TP_ARGS( + int, retval, + uint64_t, alignment), + TP_FIELDS( + ctf_integer(int, retval, retval) + ctf_integer(uint64_t, alignment, alignment) + ) +) + TRACEPOINT_EVENT(librados, rados_ioctx_locator_set_key_enter, TP_ARGS( rados_ioctx_t, ioctx, -- 2.39.5