crimson/osd: OP_CALL does support RETURNVEC now.

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

diff --git a/src/crimson/common/errorator.h b/src/crimson/common/errorator.h
index b57c8407e5ceede0ecf8f3685f6842bba12a52a6..bdace987444b3ae60e2e725646167505ae54c46b 100644 (file)
--- a/src/crimson/common/errorator.h
+++ b/src/crimson/common/errorator.h
@@ -880,6 +880,7 @@ namespace ct_error {
     ct_error_code<std::errc::operation_not_supported>;
   using not_connected = ct_error_code<std::errc::not_connected>;
   using timed_out = ct_error_code<std::errc::timed_out>;
+  using value_too_large = ct_error_code<std::errc::value_too_large>;
 
   struct pass_further_all {
     template <class ErrorT>

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index bed7f00be742b3f240ed22ff42e9e71b1bd0eea1..039d39d1730c79e3c54dafc123c2745b01674ad1 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -89,29 +89,48 @@ OpsExecuter::call_errorator::future<> OpsExecuter::do_op_call(OSDOp& osd_op)
     [this, prev_rd, prev_wr, &osd_op, flags]
     (auto outcome) -> call_errorator::future<> {
       auto& [ret, outdata] = outcome;
+      osd_op.rval = ret;
+
       logger().debug("do_op_call: method returned ret={}, outdata.length()={}"
                      " while num_read={}, num_write={}",
                      ret, outdata.length(), num_read, num_write);
       if (num_read > prev_rd && !(flags & CLS_METHOD_RD)) {
         logger().error("method tried to read object but is not marked RD");
+        osd_op.rval = -EIO;
         return 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");
+        osd_op.rval = -EIO;
         return 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());
+      // ceph-osd has this implemented in `PrimaryLogPG::execute_ctx`,
+      // grep for `ignore_out_data`.
+      using crimson::common::local_conf;
+      if (op_info->allows_returnvec() &&
+          op_info->may_write() &&
+          ret >= 0 &&
+          outdata.length() > local_conf()->osd_max_write_op_reply_len) {
+        // the justification of this limit it to not inflate the pg log.
+        // that's the reason why we don't worry about pure reads.
+        logger().error("outdata overflow due to .length()={}, limit={}",
+                       outdata.length(),
+                       local_conf()->osd_max_write_op_reply_len);
+        osd_op.rval = -EOVERFLOW;
+        return crimson::ct_error::value_too_large::make();
+      }
+      // for write calls we never return data expect errors or RETURNVEC.
+      // please refer cls/cls_hello.cc to details.
+      if (!op_info->may_write() || op_info->allows_returnvec() || ret < 0) {
         osd_op.op.extent.length = outdata.length();
         osd_op.outdata.claim_append(outdata);
       }
       if (ret < 0) {
-        return crimson::stateful_ec{ std::error_code(-ret, std::generic_category()) };
+        return crimson::stateful_ec{
+          std::error_code(-ret, std::generic_category()) };
+      } else {
+        return seastar::now();
       }
-      return seastar::now();
     }
   );
 }

diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h
index 390bf9e23dc158f9ef6287b6574001fc78172985..93fb96dd6e4b663ae69940d59c9811b92226941d 100644 (file)
--- a/src/crimson/osd/ops_executer.h
+++ b/src/crimson/osd/ops_executer.h
@@ -42,7 +42,8 @@ class OpsExecuter {
     crimson::ct_error::invarg,
     crimson::ct_error::permission_denied,
     crimson::ct_error::operation_not_supported,
-    crimson::ct_error::input_output_error>;
+    crimson::ct_error::input_output_error,
+    crimson::ct_error::value_too_large>;
   using read_errorator = PGBackend::read_errorator;
   using get_attr_errorator = PGBackend::get_attr_errorator;
   using watch_errorator = crimson::errorator<
@@ -79,6 +80,7 @@ private:
   };
 
   ObjectContextRef obc;
+  const OpInfo* op_info;
   PG& pg;
   PGBackend& backend;
   Ref<MOSDOp> msg;
@@ -163,14 +165,15 @@ private:
   }
 
 public:
-  OpsExecuter(ObjectContextRef obc, PG& pg, Ref<MOSDOp> msg)
+  OpsExecuter(ObjectContextRef obc, const OpInfo* op_info, PG& pg, Ref<MOSDOp> msg)
     : obc(std::move(obc)),
+      op_info(op_info),
       pg(pg),
       backend(pg.get_backend()),
       msg(std::move(msg)) {
   }
   OpsExecuter(PG& pg, Ref<MOSDOp> msg)
-    : OpsExecuter{ObjectContextRef(), pg, std::move(msg)}
+    : OpsExecuter{ObjectContextRef(), nullptr, pg, std::move(msg)}
   {}
 
   osd_op_errorator::future<> execute_osd_op(class OSDOp& osd_op);

diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc
index 3661d04273434b0b98ded02ba82643d44d810553..72f9f4396b307b5604b79dbca8837882cdb8f4d0 100644 (file)
--- a/src/crimson/osd/osd_operations/client_request.cc
+++ b/src/crimson/osd/osd_operations/client_request.cc
@@ -114,7 +114,7 @@ seastar::future<> ClientRequest::process_op(
       [this, &pg](auto obc) {
        return with_blocking_future(handle.enter(pp(pg).process)
        ).then([this, &pg, obc]() {
-         return pg.do_osd_ops(m, obc);
+         return pg.do_osd_ops(m, obc, op_info);
        }).then([this](Ref<MOSDOpReply> reply) {
          return conn->send(reply);
        });

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index fcef9c45aa19281808227f501cba0c5615720510..dcfa7c750064d3aac8f5aed03ecfb2ca854638d0 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -524,13 +524,14 @@ seastar::future<> PG::submit_transaction(ObjectContextRef&& obc,
 
 seastar::future<Ref<MOSDOpReply>> PG::do_osd_ops(
   Ref<MOSDOp> m,
-  ObjectContextRef obc)
+  ObjectContextRef obc,
+  const OpInfo &op_info)
 {
   using osd_op_errorator = OpsExecuter::osd_op_errorator;
   const auto oid = m->get_snapid() == CEPH_SNAPDIR ? m->get_hobj().get_head()
                                                    : m->get_hobj();
   auto ox =
-    std::make_unique<OpsExecuter>(obc, *this/* as const& */, m);
+    std::make_unique<OpsExecuter>(obc, &op_info, *this/* as const& */, m);
 
   return crimson::do_for_each(
     m->ops, [obc, m, ox = ox.get()](OSDOp& osd_op) {
@@ -563,9 +564,17 @@ seastar::future<Ref<MOSDOpReply>> PG::do_osd_ops(
                                    std::move(osd_op_p));
         }
       });
-  }).safe_then([m, obc, this, ox_deleter = std::move(ox)] {
-    auto reply = make_message<MOSDOpReply>(m.get(), 0, get_osdmap_epoch(),
-                                           0, false);
+  }).safe_then([this,
+                m,
+                obc,
+                ox_deleter = std::move(ox),
+                rvec = op_info.allows_returnvec()] {
+    auto result = m->ops.empty() || !rvec ? 0 : m->ops.back().rval.code;
+    auto reply = make_message<MOSDOpReply>(m.get(),
+                                           result,
+                                           get_osdmap_epoch(),
+                                           0,
+                                           false);
     reply->add_flags(CEPH_OSD_FLAG_ACK | CEPH_OSD_FLAG_ONDISK);
     logger().debug(
       "do_osd_ops: {} - object {} sending reply",

diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index aa5945f1abe67fcd7472e965b7699b751f461a49..a8f6ba4e3fb00f56a8b73d378e5c3e6b2ff75664 100644 (file)
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -481,7 +481,8 @@ private:
     PeeringCtx &rctx);
   seastar::future<Ref<MOSDOpReply>> do_osd_ops(
     Ref<MOSDOp> m,
-    ObjectContextRef obc);
+    ObjectContextRef obc,
+    const OpInfo &op_info);
   seastar::future<Ref<MOSDOpReply>> do_pg_ops(Ref<MOSDOp> m);
   seastar::future<> do_osd_op(
     ObjectState& os,

diff --git a/src/test/cls_hello/test_cls_hello.cc b/src/test/cls_hello/test_cls_hello.cc
index 4f1d90de4d3d13605b9a8ab363dd940cb0808af5..cb05bb77bf5e78d2efa2c7006eda726eee1070c0 100644 (file)
--- a/src/test/cls_hello/test_cls_hello.cc
+++ b/src/test/cls_hello/test_cls_hello.cc
@@ -118,7 +118,7 @@ TEST(ClsHello, WriteReturnData) {
     return;
   }
 
-  // this will return nothing -- not flag set
+  // this will return nothing -- no flag is set
   bufferlist in, out;
   ASSERT_EQ(0, ioctx.exec("myobject", "hello", "write_return_data", in, out));
   ASSERT_EQ(std::string(), std::string(out.c_str(), out.length()));