From d8d0ab1a699221fc2601339b29fa0d1671b712b1 Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Wed, 18 Sep 2019 17:25:55 +0200 Subject: [PATCH] crimson/osd: erroratorize the do_op_call() path more. Signed-off-by: Radoslaw Zarzynski --- src/crimson/common/errorator.h | 6 ++- src/crimson/osd/ops_executer.cc | 83 +++++++++++++++++++-------------- src/crimson/osd/ops_executer.h | 8 +++- 3 files changed, 61 insertions(+), 36 deletions(-) diff --git a/src/crimson/common/errorator.h b/src/crimson/common/errorator.h index f98a2a9c147bd..bc5ea00653e06 100644 --- 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; diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc index 3646989462ada..6db62ecf38d63 100644 --- 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(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(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 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::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 f644520be4372..ad56e69a81bd2 100644 --- a/src/crimson/osd/ops_executer.h +++ b/src/crimson/osd/ops_executer.h @@ -65,7 +65,13 @@ class OpsExecuter { template auto with_effect(Context&& ctx, MainFunc&& main_func, EffectFunc&& effect_func); - using call_errorator = ceph::errorator; + 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 -- 2.39.5