crimson/osd: OpsExecuter differentiates read and modifying operations.

This is necessary to pass ClsHello.BadMethods.

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

diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc
index 0a2938359821624dd40c2bcf4f9b703dba011a5a..d668dbd12b6a65cd6e07ed30efc1c0067d1ce16c 100644 (file)
--- a/src/crimson/osd/ops_executer.cc
+++ b/src/crimson/osd/ops_executer.cc
@@ -74,35 +74,31 @@ seastar::future<> OpsExecuter::do_op_call(OSDOp& osd_op)
 #endif
 
   logger().debug("calling method {}.{}", cname, mname);
-#if 0
-  int prev_rd = ctx->num_read;
-  int prev_wr = ctx->num_write;
-#endif
-
-
-  return seastar::async([this, &osd_op, method, indata=std::move(indata)]() mutable {
+  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 ceph::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 ceph::osd::input_output_error{};
+    }
+
+    // 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) {
       throw ceph::osd::make_error(ret);
     }
-#if 0
-       if (ctx->num_read > prev_rd && !(flags & CLS_METHOD_RD)) {
-         derr << "method " << cname << "." << mname << " tried to read object but is not marked RD" << dendl;
-         result = -EIO;
-         break;
-       }
-       if (ctx->num_write > prev_wr && !(flags & CLS_METHOD_WR)) {
-         derr << "method " << cname << "." << mname << " tried to update object but is not marked WR" << dendl;
-         result = -EIO;
-         break;
-       }
-#endif
-
-       logger().debug("method called response length={}", outdata.length());
-       osd_op.op.extent.length = outdata.length();
-       osd_op.outdata.claim_append(outdata);
   });
 
 }
@@ -152,7 +148,6 @@ OpsExecuter::do_pgnls(ceph::bufferlist& indata,
   });
 }
 
-// TODO: split the method accordingly to os' constness needs
 seastar::future<>
 OpsExecuter::do_osd_op(OSDOp& osd_op)
 {
@@ -164,27 +159,37 @@ OpsExecuter::do_osd_op(OSDOp& osd_op)
   case CEPH_OSD_OP_SYNC_READ:
     [[fallthrough]];
   case CEPH_OSD_OP_READ:
-    return backend.read(os->oi,
-                        op.extent.offset,
-                        op.extent.length,
-                        op.extent.truncate_size,
-                        op.extent.truncate_seq,
-                        op.flags).then([&osd_op](bufferlist bl) {
-      osd_op.rval = bl.length();
-      osd_op.outdata = std::move(bl);
-      return seastar::now();
+    return do_read_op([&osd_op] (auto& backend, const auto& os) {
+      return backend.read(os.oi,
+                          osd_op.op.extent.offset,
+                          osd_op.op.extent.length,
+                          osd_op.op.extent.truncate_size,
+                          osd_op.op.extent.truncate_seq,
+                          osd_op.op.flags).then(
+        [&osd_op](bufferlist bl) {
+          osd_op.rval = bl.length();
+          osd_op.outdata = std::move(bl);
+          return seastar::now();
+        });
     });
   case CEPH_OSD_OP_GETXATTR:
-    return backend.getxattr(os, osd_op);
+    return do_read_op([&osd_op] (auto& backend, const auto& os) {
+      return backend.getxattr(os, osd_op);
+    });
   case CEPH_OSD_OP_WRITE:
-    return backend.write(*os, osd_op, txn);
+    return do_write_op([&osd_op] (auto& backend, auto& os, auto& txn) {
+      return backend.write(os, osd_op, txn);
+    });
   case CEPH_OSD_OP_WRITEFULL:
-    // XXX: os = backend.write(std::move(os), ...) instead?
-    return backend.writefull(*os, osd_op, txn);
+    return do_write_op([&osd_op] (auto& backend, auto& os, auto& txn) {
+      return backend.writefull(os, osd_op, txn);
+    });
   case CEPH_OSD_OP_SETALLOCHINT:
     return seastar::now();
   case CEPH_OSD_OP_SETXATTR:
-    return backend.setxattr(*os, osd_op, txn);
+    return do_write_op([&osd_op] (auto& backend, auto& os, auto& txn) {
+      return backend.setxattr(os, osd_op, txn);
+    });
   case CEPH_OSD_OP_PGNLS:
     return do_pgnls(osd_op.indata, os->oi.soid.get_namespace(), op.pgls.count)
       .then([&osd_op](bufferlist bl) {
@@ -192,11 +197,16 @@ OpsExecuter::do_osd_op(OSDOp& osd_op)
        return seastar::now();
     });
   case CEPH_OSD_OP_DELETE:
-    return backend.remove(*os, txn);
+    return do_write_op([&osd_op] (auto& backend, auto& os, auto& txn) {
+      return backend.remove(os, txn);
+    });
   case CEPH_OSD_OP_CALL:
     return this->do_op_call(osd_op);
   case CEPH_OSD_OP_STAT:
-    return backend.stat(*os, osd_op);
+    // note: stat does not require RD
+    return do_const_op([&osd_op] (/* const */auto& backend, const auto& os) {
+      return backend.stat(os, osd_op);
+    });
   default:
     logger().warn("unknown op {}", ceph_osd_op_name(op.op));
     throw std::runtime_error(

diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h
index 4804e20c8b2a27ed7d17aed36240ca47db17c7e6..b5b1cd8b00185f41235e8fb85a3bdf57583785f9 100644 (file)
--- a/src/crimson/osd/ops_executer.h
+++ b/src/crimson/osd/ops_executer.h
@@ -33,12 +33,34 @@ class OpsExecuter {
   PGBackend& backend;
   ceph::os::Transaction txn;
 
+  size_t num_read = 0;    ///< count read ops
+  size_t num_write = 0;   ///< count update ops
+
   seastar::future<ceph::bufferlist> do_pgnls(
     ceph::bufferlist& indata,
     const std::string& nspace,
     uint64_t limit);
   seastar::future<> do_op_call(class OSDOp& osd_op);
 
+  template <class Func>
+  auto do_const_op(Func&& f) {
+    // TODO: pass backend as read-only
+    return std::forward<Func>(f)(backend, const_cast<const ObjectState&>(*os));
+  }
+
+  template <class Func>
+  auto do_read_op(Func&& f) {
+    ++num_read;
+    // TODO: pass backend as read-only
+    return std::forward<Func>(f)(backend, const_cast<const ObjectState&>(*os));
+  }
+
+  template <class Func>
+  auto do_write_op(Func&& f) {
+    ++num_write;
+    return std::forward<Func>(f)(backend, *os, txn);
+  }
+
 public:
   OpsExecuter(PGBackend::cached_os_t os, PG& pg)
     : os(std::move(os)), pg(pg), backend(pg.get_backend()) {

diff --git a/src/crimson/osd/pg_backend.cc b/src/crimson/osd/pg_backend.cc
index eb578199dca20e81e40b781ae6035b2165dd093a..e78c4ce4b93e6b623fdd6f8bc5936a0ae08291df 100644 (file)
--- a/src/crimson/osd/pg_backend.cc
+++ b/src/crimson/osd/pg_backend.cc
@@ -247,7 +247,7 @@ seastar::future<bufferlist> PGBackend::read(const object_info_t& oi,
 }
 
 seastar::future<> PGBackend::stat(
-  ObjectState& os,
+  const ObjectState& os,
   OSDOp& osd_op)
 {
   if (os.exists/* TODO: && !os.is_whiteout() */) {
@@ -398,8 +398,6 @@ seastar::future<> PGBackend::setxattr(
   const OSDOp& osd_op,
   ceph::os::Transaction& txn)
 {
-  //++ctx->num_write;
-
   if (local_conf()->osd_max_attr_size > 0 &&
       osd_op.op.xattr.value_len > local_conf()->osd_max_attr_size) {
     throw ceph::osd::make_error(-EFBIG);
@@ -429,7 +427,7 @@ seastar::future<> PGBackend::setxattr(
 }
 
 seastar::future<> PGBackend::getxattr(
-  ObjectState& os,
+  const ObjectState& os,
   OSDOp& osd_op)
 {
   std::string name;

diff --git a/src/crimson/osd/pg_backend.h b/src/crimson/osd/pg_backend.h
index 6a20e28dbfc574e2e5fd81487e29c0cdce24c14f..e88057757e2b2842da145a289f208584b7e0368d 100644 (file)
--- a/src/crimson/osd/pg_backend.h
+++ b/src/crimson/osd/pg_backend.h
@@ -48,7 +48,7 @@ public:
                                   uint32_t truncate_seq,
                                   uint32_t flags);
   seastar::future<> stat(
-    ObjectState& os,
+    const ObjectState& os,
     OSDOp& osd_op);
   seastar::future<> remove(
     ObjectState& os,
@@ -77,7 +77,7 @@ public:
     const OSDOp& osd_op,
     ceph::os::Transaction& trans);
   seastar::future<> getxattr(
-    ObjectState& os,
+    const ObjectState& os,
     OSDOp& osd_op);
 
   virtual void got_rep_op_reply(const MOSDRepOpReply&) {}