Objecter, librados: use only ObjectOperation form of sparse_read internally

This will be used when exposing an ObjectOperation version of sparse_read()
to the librados user, and there's no reason to duplicate code for creating
and handling it. Add a wrapper Context for handling the lifecycle of the
::ObjectOperation.

This cleans up the synchronous version of sparse_read quite a bit by
using the general operate_read() instead of duplicating decoding and
a bunch of sync boilerplate.

Move handling the decoding of a sparse_read into the Objecter, with
the rest of the decoding of rados operations. librados shouldn't be
the only user of the Objecter that can understand sparse_reads.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>

diff --git a/src/librados/IoCtxImpl.cc b/src/librados/IoCtxImpl.cc
index 741a581e4458ec0b3d5b9432f38e0d198f35121f..d64eeea2dca8f33034daf9399960e3abdc56ac73 100644 (file)
--- a/src/librados/IoCtxImpl.cc
+++ b/src/librados/IoCtxImpl.cc
@@ -775,6 +775,17 @@ int librados::IoCtxImpl::aio_read(const object_t oid, AioCompletionImpl *c,
   return 0;
 }
 
+class C_ObjectOperation : public Context {
+public:
+  ::ObjectOperation m_ops;
+  C_ObjectOperation(Context *c) : m_ctx(c) {}
+  virtual void finish(int r) {
+    m_ctx->complete(r);
+  }
+private:
+  Context *m_ctx;
+};
+
 int librados::IoCtxImpl::aio_sparse_read(const object_t oid,
                                         AioCompletionImpl *c,
                                         std::map<uint64_t,uint64_t> *m,
@@ -784,16 +795,19 @@ int librados::IoCtxImpl::aio_sparse_read(const object_t oid,
   if (len > (size_t) INT_MAX)
     return -EDOM;
 
-  C_aio_sparse_read_Ack *onack = new C_aio_sparse_read_Ack(c, data_bl, m);
-  eversion_t ver;
+  Context *nested = new C_aio_Ack(c);
+  C_ObjectOperation *onack = new C_ObjectOperation(nested);
 
+  c->is_read = true;
   c->io = this;
   c->pbl = NULL;
 
+  onack->m_ops.sparse_read(off, len, m, data_bl, NULL);
+
   Mutex::Locker l(*lock);
-  objecter->sparse_read(oid, oloc,
-                off, len, snapid, &c->bl, 0,
-                onack);
+  objecter->read(oid, oloc,
+                onack->m_ops, snap_seq, NULL, 0,
+                onack, &c->objver);
   return 0;
 }
 
@@ -1207,33 +1221,15 @@ int librados::IoCtxImpl::sparse_read(const object_t& oid,
   if (len > (size_t) INT_MAX)
     return -EDOM;
 
-  bufferlist bl;
-
-  Mutex mylock("IoCtxImpl::read::mylock");
-  Cond cond;
-  bool done;
   int r;
-  Context *onack = new C_SafeCond(&mylock, &cond, &done, &r);
-
-  lock->Lock();
-  objecter->sparse_read(oid, oloc,
-                       off, len, snap_seq, &bl, 0,
-                       onack);
-  lock->Unlock();
-
-  mylock.Lock();
-  while (!done)
-    cond.Wait(mylock);
-  mylock.Unlock();
-  ldout(client->cct, 10) << "Objecter returned from read r=" << r << dendl;
+  ::ObjectOperation rd;
+  prepare_assert_ops(&rd);
+  rd.sparse_read(off, len, &m, &data_bl, NULL);
 
+  r = operate_read(oid, &rd, NULL);
   if (r < 0)
     return r;
 
-  bufferlist::iterator iter = bl.begin();
-  ::decode(m, iter);
-  ::decode(data_bl, iter);
-
   return m.size();
 }
 
@@ -1648,36 +1644,6 @@ void librados::IoCtxImpl::C_aio_stat_Ack::finish(int r)
   c->put_unlock();
 }
 
-/////////////////////// C_aio_sparse_read_Ack //////////////////////////
-
-librados::IoCtxImpl::C_aio_sparse_read_Ack::C_aio_sparse_read_Ack(AioCompletionImpl *_c,
-                                                                 bufferlist *data,
-                                                                 std::map<uint64_t, uint64_t> *extents)
-  : c(_c), data_bl(data), m(extents)
-{
-  c->get();
-}
-
-void librados::IoCtxImpl::C_aio_sparse_read_Ack::finish(int r)
-{
-  c->lock.Lock();
-  c->rval = r;
-  c->ack = true;
-  c->cond.Signal();
-
-  bufferlist::iterator iter = c->bl.begin();
-  if (r >= 0) {
-    ::decode(*m, iter);
-    ::decode(*data_bl, iter);
-  }
-
-  if (c->callback_complete) {
-    c->io->client->finisher.queue(new C_AioComplete(c));
-  }
-
-  c->put_unlock();
-}
-
 //////////////////////////// C_aio_Safe ////////////////////////////////
 
 librados::IoCtxImpl::C_aio_Safe::C_aio_Safe(AioCompletionImpl *_c) : c(_c)

diff --git a/src/librados/IoCtxImpl.h b/src/librados/IoCtxImpl.h
index 21e586543c71e9ba17aa2afc43143e69cdee1ea0..538c2f1d357ad1bd8f1819000dcd1614871adb81 100644 (file)
--- a/src/librados/IoCtxImpl.h
+++ b/src/librados/IoCtxImpl.h
@@ -156,16 +156,6 @@ struct librados::IoCtxImpl {
     void finish(int r);
   };
 
-  struct C_aio_sparse_read_Ack : public Context {
-    AioCompletionImpl *c;
-    bufferlist *data_bl;
-    std::map<uint64_t, uint64_t> *m;
-    C_aio_sparse_read_Ack(AioCompletionImpl *_c,
-                         bufferlist *data,
-                         std::map<uint64_t, uint64_t> *extents);
-    void finish(int r);
-  };
-
   struct C_aio_Safe : public Context {
     AioCompletionImpl *c;
     C_aio_Safe(AioCompletionImpl *_c);

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index d041e0688b91c32efca9e5d28c62e3f29edede8e..e17e36a12986c6a401bdea87a89d13dcaaa44f86 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -236,6 +236,40 @@ struct ObjectOperation {
     out_bl[p] = pbl;
     out_rval[p] = prval;
   }
+
+  struct C_ObjectOperation_sparse_read : public Context {
+    bufferlist bl;
+    bufferlist *data_bl;
+    std::map<uint64_t, uint64_t> *extents;
+    int *prval;
+    C_ObjectOperation_sparse_read(bufferlist *data_bl,
+                                 std::map<uint64_t, uint64_t> *extents,
+                                 int *prval)
+      : data_bl(data_bl), extents(extents), prval(prval) {}
+    void finish(int r) {
+      bufferlist::iterator iter = bl.begin();
+      if (r >= 0) {
+       try {
+         ::decode(*extents, iter);
+         ::decode(*data_bl, iter);
+       } catch (buffer::error& e) {
+         if (prval)
+           *prval = -EIO;
+       }
+      }
+    }
+  };
+  void sparse_read(uint64_t off, uint64_t len, std::map<uint64_t,uint64_t> *m,
+                  bufferlist *data_bl, int *prval) {
+    bufferlist bl;
+    add_data(CEPH_OSD_OP_SPARSE_READ, off, len, bl);
+    unsigned p = ops.size() - 1;
+    C_ObjectOperation_sparse_read *h =
+      new C_ObjectOperation_sparse_read(data_bl, m, prval);
+    out_bl[p] = &h->bl;
+    out_handler[p] = h;
+    out_rval[p] = prval;
+  }
   void write(uint64_t off, bufferlist& bl) {
     add_data(CEPH_OSD_OP_WRITE, off, bl.length(), bl);
   }
@@ -1283,23 +1317,6 @@ private:
     o->outbl = pbl;
     return op_submit(o);
   }
-  tid_t sparse_read(const object_t& oid, const object_locator_t& oloc,
-            uint64_t off, uint64_t len, snapid_t snap, bufferlist *pbl, int flags,
-            Context *onfinish,
-            eversion_t *objver = NULL, ObjectOperation *extra_ops = NULL) {
-    vector<OSDOp> ops;
-    int i = init_ops(ops, 1, extra_ops);
-    ops[i].op.op = CEPH_OSD_OP_SPARSE_READ;
-    ops[i].op.extent.offset = off;
-    ops[i].op.extent.length = len;
-    ops[i].op.extent.truncate_size = 0;
-    ops[i].op.extent.truncate_seq = 0;
-    Op *o = new Op(oid, oloc, ops, flags | global_op_flags | CEPH_OSD_FLAG_READ, onfinish, 0, objver);
-    o->snapid = snap;
-    o->outbl = pbl;
-    return op_submit(o);
-  }
-
   tid_t getxattr(const object_t& oid, const object_locator_t& oloc,
             const char *name, snapid_t snap, bufferlist *pbl, int flags,
             Context *onfinish,