crimson/osd: erroratorize the do_op_call() path more.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

diff --git a/src/crimson/common/errorator.h b/src/crimson/common/errorator.h
index f98a2a9c147bddff9980d273fef2a4a5c9b868ec..bc5ea00653e065d58f7044bb33a767e677971137 100644 (file)
--- a/src/crimson/common/errorator.h
+++ b/src/crimson/common/errorator.h
@@ -13,7 +13,9 @@ namespace _impl {
     invarg,
     enodata,
     input_output_error,
-    object_corrupted
+    object_corrupted,
+    permission_denied,
+    operation_not_supported
   };
 }
 
@@ -590,6 +592,8 @@ namespace ct_error {
   using invarg = unthrowable_wrapper<_impl::ct_error, _impl::ct_error::invarg>;
   using input_output_error = unthrowable_wrapper<_impl::ct_error, _impl::ct_error::input_output_error>;
   using object_corrupted = unthrowable_wrapper<_impl::ct_error, _impl::ct_error::object_corrupted>;
+  using permission_denied = unthrowable_wrapper<_impl::ct_error, _impl::ct_error::permission_denied>;
+  using operation_not_supported = unthrowable_wrapper<_impl::ct_error, _impl::ct_error::operation_not_supported>;
 }
 
 using stateful_errc = stateful_error_t<std::errc>;

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index 3646989462ada0ef33aba79c749bf2b68bbf2c71..6db62ecf38d633815480887e873d8669240fc334 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -37,7 +37,7 @@ OpsExecuter::call_errorator::future<> OpsExecuter::do_op_call(OSDOp& osd_op)
     bp.copy(osd_op.op.cls.indata_len, indata);
   } catch (buffer::error&) {
     logger().warn("call unable to decode class + method + indata");
-    throw crimson::osd::invalid_argument{};
+    return crimson::ct_error::invarg::make();
   }
 
   // NOTE: opening a class can actually result in dlopen(), and thus
@@ -48,23 +48,23 @@ OpsExecuter::call_errorator::future<> OpsExecuter::do_op_call(OSDOp& osd_op)
   if (r) {
     logger().warn("class {} open got {}", cname, cpp_strerror(r));
     if (r == -ENOENT) {
-      throw crimson::osd::operation_not_supported{};
+      return crimson::ct_error::operation_not_supported::make();
     } else if (r == -EPERM) {
       // propagate permission errors
-      throw crimson::osd::permission_denied{};
+      return crimson::ct_error::permission_denied::make();
     }
-    throw crimson::osd::input_output_error{};
+    return crimson::ct_error::input_output_error::make();
   }
 
   ClassHandler::ClassMethod* method = cls->get_method(mname);
   if (!method) {
     logger().warn("call method {}.{} does not exist", cname, mname);
-    throw crimson::osd::operation_not_supported{};
+    return crimson::ct_error::operation_not_supported::make();
   }
 
   const auto flags = method->get_flags();
   if (!os->exists && (flags & CLS_METHOD_WR) == 0) {
-    throw crimson::osd::object_not_found{};
+    return crimson::ct_error::enoent::make();
   }
 
 #if 0
@@ -74,36 +74,43 @@ OpsExecuter::call_errorator::future<> OpsExecuter::do_op_call(OSDOp& osd_op)
 #endif
 
   logger().debug("calling method {}.{}", cname, mname);
-  return seastar::async([this, &osd_op, flags, method, indata=std::move(indata)]() mutable {
-    ceph::bufferlist outdata;
-    const auto prev_rd = num_read;
-    const auto prev_wr = num_write;
-    const auto ret = method->exec(reinterpret_cast<cls_method_context_t>(this),
-                                  indata, outdata);
-    if (num_read > prev_rd && !(flags & CLS_METHOD_RD)) {
-      logger().error("method tried to read object but is not marked RD");
-      throw crimson::osd::input_output_error{};
-    }
-    if (num_write > prev_wr && !(flags & CLS_METHOD_WR)) {
-      logger().error("method tried to update object but is not marked WR");
-      throw crimson::osd::input_output_error{};
+  return seastar::async(
+    [this, method, indata=std::move(indata)]() mutable {
+      ceph::bufferlist outdata;
+      auto cls_context = reinterpret_cast<cls_method_context_t>(this);
+      const auto ret = method->exec(cls_context, indata, outdata);
+      return std::make_pair(ret, std::move(outdata));
     }
+  ).then(
+    [prev_rd = num_read, prev_wr = num_write, this, &osd_op, flags]
+    (auto outcome) {
+      auto& [ret, outdata] = outcome;
+
+      if (num_read > prev_rd && !(flags & CLS_METHOD_RD)) {
+        logger().error("method tried to read object but is not marked RD");
+        return call_errorator::make_plain_exception_future<>(
+          crimson::ct_error::input_output_error::make());
+      }
+      if (num_write > prev_wr && !(flags & CLS_METHOD_WR)) {
+        logger().error("method tried to update object but is not marked WR");
+        return call_errorator::make_plain_exception_future<>(
+          crimson::ct_error::input_output_error::make());
+      }
 
-    // for write calls we never return data expect errors. For details refer
-    // to cls/cls_hello.cc.
-    if (ret < 0 || (flags & CLS_METHOD_WR) == 0) {
-      logger().debug("method called response length={}", outdata.length());
-      osd_op.op.extent.length = outdata.length();
-      osd_op.outdata.claim_append(outdata);
-    }
-    return ret;
-  }).then([] (const int ret) {
-    if (ret < 0) {
-      return call_errorator::make_plain_exception_future<>(
-        ceph::stateful_errint{ ret });
+      // for write calls we never return data expect errors. For details refer
+      // to cls/cls_hello.cc.
+      if (ret < 0 || (flags & CLS_METHOD_WR) == 0) {
+        logger().debug("method called response length={}", outdata.length());
+        osd_op.op.extent.length = outdata.length();
+        osd_op.outdata.claim_append(outdata);
+      }
+      if (ret < 0) {
+        return call_errorator::make_plain_exception_future<>(
+          crimson::stateful_errint{ ret });
+      }
+      return seastar::now();
     }
-    return seastar::now();
-  });
+  );
 }
 
 static inline std::unique_ptr<const PGLSFilter> get_pgls_filter(
@@ -416,7 +423,15 @@ OpsExecuter::execute_osd_op(OSDOp& osd_op)
         return seastar::now();
       }, ceph::stateful_errint::handle([] (int err) {
         // TODO: implement the handler. NOP for now.
-      }));
+      }), crimson::ct_error::input_output_error::handle([] {
+        // TODO: implement the handler. NOP for now.
+      }),
+      crimson::errorator<crimson::ct_error::enoent,
+                         crimson::ct_error::input_output_error,
+                         crimson::ct_error::operation_not_supported,
+                         crimson::ct_error::permission_denied,
+                         crimson::ct_error::invarg>::discard_all{}
+      );
   case CEPH_OSD_OP_STAT:
     // note: stat does not require RD
     return do_const_op([&osd_op] (/* const */auto& backend, const auto& os) {

diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h
index f644520be4372707bc61da0f33da3f14778dfd56..ad56e69a81bd2ee81382e7da5cef48ea731de915 100644 (file)
--- a/src/crimson/osd/ops_executer.h
+++ b/src/crimson/osd/ops_executer.h
@@ -65,7 +65,13 @@ class OpsExecuter {
   template <class Context, class MainFunc, class EffectFunc>
   auto with_effect(Context&& ctx, MainFunc&& main_func, EffectFunc&& effect_func);
 
-  using call_errorator = ceph::errorator<ceph::stateful_errint>;
+  using call_errorator = crimson::errorator<
+    crimson::stateful_errint,
+    crimson::ct_error::enoent,
+    crimson::ct_error::invarg,
+    crimson::ct_error::permission_denied,
+    crimson::ct_error::operation_not_supported,
+    crimson::ct_error::input_output_error>;
   call_errorator::future<> do_op_call(class OSDOp& osd_op);
 
   template <class Func>