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>

diff --git a/src/include/rados/librados.h b/src/include/rados/librados.h
index 5d4ac753a9607b8679609d35982b60b97c13f0f0..12d5970e0dc4ca13511fab5569b76bb23739139f 100644 (file)
--- 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 52ce642934c88ca851c21f7305a9f5890838721a..faa5e2842d464267bff2cfbc4c22511cf30aed24 100644 (file)
--- 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 a01ae7d45813603789c97fd38dec202d0c2c75f5..09128e2a0bd1c0cc307196c11f8b294dc3f66aef 100644 (file)
--- 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 d44336f1977d830a20ab4717a87fa88cacaf2a0b..a26e46f61fa1cf04bc15db805cd84a6385207d02 100644 (file)
--- 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 86badc2bc84b044dc10a011cf0d50f621c55df3a..7c1c9c8d8c62e748cc3c7eb1c325085c510632fd 100644 (file)
--- 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 45fde7a42bd8fe815441377b40c9f9de24cd96ae..e4740d920d7b6d9f8a2e0212a9ba65f41a10abd5 100644 (file)
--- 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 4f4b0863f702967788d50d06503f99934b6c5718..d852c304bbbdd3933e1b558599b244a9fe155658 100644 (file)
--- 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 0ba22ea0410fbeec2c8317abe04725d871f3cc4d..1b60afcbf66b494379b390154ec7fc4b605596b6 100644 (file)
--- 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,