ReplicatedPG: copy: switch out the CopyCallback interface

The tuple was already unwieldy with 4 members; I didn't want to add
more. Instead, create a new CopyResults struct which contains all the
object info and completion data, and pass the retval and a CopyResults*
in the CopyCallbackResults tuple.

2 files changed, 55 insertions, 45 deletions

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 7ac102c188a..4a7aaff4dc0 100644
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -4362,15 +4362,16 @@ void ReplicatedPG::_copy_some(ObjectContextRef obc, CopyOpRef cop)
 {
   dout(10) << __func__ << " " << obc << " " << cop << dendl;
   ObjectOperation op;
-  if (cop->user_version) {
-    op.assert_version(cop->user_version);
+  if (cop->results->user_version) {
+    op.assert_version(cop->results->user_version);
   } else {
     // we should learn the version after the first chunk, if we didn't know
     // it already!
     assert(cop->cursor.is_initial());
   }
   op.copy_get(&cop->cursor, cct->_conf->osd_copyfrom_max_chunk,
-	      &cop->size, &cop->mtime, &cop->category,
+	      &cop->results->object_size, &cop->results->mtime,
+	      &cop->results->category,
 	      &cop->attrs, &cop->data, &cop->omap,
 	      &cop->rval);
 
@@ -4382,7 +4383,7 @@ void ReplicatedPG::_copy_some(ObjectContextRef obc, CopyOpRef cop)
 				  new C_OnFinisher(fin,
 						   &osd->objecter_finisher),
 				  // discover the object version if we don't know it yet
-				  cop->user_version ? NULL : &cop->user_version);
+				  cop->results->user_version ? NULL : &cop->results->user_version);
   fin->tid = tid;
   cop->objecter_tid = tid;
   osd->objecter_lock.Unlock();
@@ -4405,7 +4406,6 @@ void ReplicatedPG::process_copy_chunk(hobject_t oid, tid_t tid, int r)
   ObjectContextRef obc = cop->obc;
   cop->objecter_tid = 0;
 
-  CopyResults results;
   if (r >= 0) {
     assert(cop->rval >= 0);
 
@@ -4433,12 +4433,11 @@ void ReplicatedPG::process_copy_chunk(hobject_t oid, tid_t tid, int r)
       _copy_some(obc, cop);
       return;
     }
-    _build_finish_copy_transaction(cop, results.get<3>());
-    results.get<1>() = cop->temp_cursor.data_offset;
+    _build_finish_copy_transaction(cop, cop->results->final_tx);
   }
 
   dout(20) << __func__ << " complete; committing" << dendl;
-  results.get<0>() = r;
+  CopyCallbackResults results(r, cop->results);
   cop->cb->complete(results);
 
   copy_ops.erase(obc->obs.oi.soid);
@@ -4505,8 +4504,8 @@ int ReplicatedPG::finish_copyfrom(OpContext *ctx)
   if (cb->is_temp_obj_used()) {
     ctx->discard_temp_oid = cb->temp_obj;
   }
-  ctx->op_t.swap(cb->results.get<3>());
-  ctx->op_t.append(cb->results.get<3>());
+  ctx->op_t.swap(cb->results->final_tx);
+  ctx->op_t.append(cb->results->final_tx);
 
   interval_set<uint64_t> ch;
   if (obs.oi.size > 0)
@@ -4527,8 +4526,8 @@ int ReplicatedPG::finish_copyfrom(OpContext *ctx)
 void ReplicatedPG::cancel_copy(CopyOpRef cop)
 {
   dout(10) << __func__ << " " << cop->obc->obs.oi.soid
-	   << " from " << cop->src << " " << cop->oloc << " v" << cop->user_version
-	   << dendl;
+	   << " from " << cop->src << " " << cop->oloc
+	   << " v" << cop->results->user_version << dendl;
 
   // cancel objecter op, if we can
   if (cop->objecter_tid) {
@@ -4540,8 +4539,7 @@ void ReplicatedPG::cancel_copy(CopyOpRef cop)
   --cop->obc->copyfrom_readside;
 
   kick_object_context_blocked(cop->obc);
-  bool temp_obj_created = !cop->cursor.is_initial();
-  CopyResults result(-ECANCELED, 0, temp_obj_created, ObjectStore::Transaction());
+  CopyCallbackResults result(-ECANCELED, cop->results);
   cop->cb->complete(result);
 }
 
diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
index 4486c9fae2d..2ffc9e61066 100644
--- a/src/osd/ReplicatedPG.h
+++ b/src/osd/ReplicatedPG.h
@@ -96,19 +96,35 @@ public:
   struct OpContext;
   class CopyCallback;
 
+  /**
+   * CopyResults stores the object metadata of interest to a copy initiator.
+   */
+  struct CopyResults {
+    utime_t mtime; ///< the copy source's mtime
+    size_t object_size; ///< the copied object's size
+    bool started_temp_obj; ///< true if the callback needs to delete temp object
+    /**
+     * Final transaction; if non-empty the callback must execute it before any
+     * other accesses to the object (in order to complete the copy).
+     */
+    ObjectStore::Transaction final_tx;
+    string category; ///< The copy source's category
+    version_t user_version; ///< The copy source's user version
+    CopyResults() : object_size(0), started_temp_obj(false),
+	user_version(0) {}
+  };
+
   struct CopyOp {
     CopyCallback *cb;
     ObjectContextRef obc;
     hobject_t src;
     object_locator_t oloc;
-    version_t user_version;
+
+    CopyResults *results;
 
     tid_t objecter_tid;
 
     object_copy_cursor_t cursor;
-    uint64_t size;
-    utime_t mtime;
-    string category;
     map<string,bufferlist> attrs;
     bufferlist data;
     map<string,bufferlist> omap;
@@ -120,12 +136,15 @@ public:
 
     CopyOp(CopyCallback *cb_, ObjectContextRef _obc, hobject_t s, object_locator_t l,
            version_t v, const hobject_t& dest)
-      : cb(cb_), obc(_obc), src(s), oloc(l), user_version(v),
+      : cb(cb_), obc(_obc), src(s), oloc(l),
+        results(NULL),
 	objecter_tid(0),
-	size(0),
 	rval(-1),
 	temp_oid(dest)
-    {}
+    {
+      results = new CopyResults();
+      results->user_version = v;
+    }
   };
   typedef boost::shared_ptr<CopyOp> CopyOpRef;
 
@@ -135,29 +154,19 @@ public:
    * one and give an instance of the class to start_copy.
    *
    * The implementer is responsible for making sure that the CopyCallback
-   * can associate itself with the correct copy operation. The presence
-   * of the closing Transaction ensures that write operations can be performed
-   * atomically with the copy being completed (which doing them in separate
-   * transactions would not allow); if you are doing the copy for a read
-   * op you will have to generate a separate op to finish the copy with.
+   * can associate itself with the correct copy operation.
    */
-  /// return code, total object size, data in temp object?, final Transaction
-  typedef boost::tuple<int, size_t, bool, ObjectStore::Transaction> CopyResults;
-  class CopyCallback : public GenContext<CopyResults&> {
+  typedef boost::tuple<int, CopyResults*> CopyCallbackResults;
+  class CopyCallback : public GenContext<CopyCallbackResults> {
   protected:
     CopyCallback() {}
     /**
      * results.get<0>() is the return code: 0 for success; -ECANCELLED if
      * the operation was cancelled by the local OSD; -errno for other issues.
-     * results.get<1>() is the total size of the object (for updating pg stats)
-     * results.get<2>() indicates whether we have already written data to
-     * the temp object (so it needs to get cleaned up, if the return code
-     * indicates a failure)
-     * results.get<3>() is a Transaction; if non-empty you need to perform
-     * its results before any other accesses to the object in order to
-     * complete the copy.
+     * results.get<1>() is a pointer to a CopyResults object, which you are
+     * responsible for deleting.
      */
-    virtual void finish(CopyResults& results_) = 0;
+    virtual void finish(CopyCallbackResults results_) = 0;
 
   public:
     /// Provide the final size of the copied object to the CopyCallback
@@ -166,16 +175,18 @@ public:
 
   class CopyFromCallback: public CopyCallback {
   public:
-    CopyResults results;
+    CopyResults *results;
+    int retval;
     OpContext *ctx;
     hobject_t temp_obj;
     CopyFromCallback(OpContext *ctx_, const hobject_t& temp_obj_) :
-      ctx(ctx_), temp_obj(temp_obj_) {}
+      results(NULL), retval(0), ctx(ctx_), temp_obj(temp_obj_) {}
     ~CopyFromCallback() {}
 
-    virtual void finish(CopyResults& results_) {
-      results = results_;
-      int r = results.get<0>();
+    virtual void finish(CopyCallbackResults results_) {
+      results = results_.get<1>();
+      int r = results_.get<0>();
+      retval = r;
       if (r >= 0) {
 	ctx->pg->execute_ctx(ctx);
       }
@@ -186,11 +197,12 @@ public:
 	}
 	delete ctx;
       }
+      delete results;
     }
 
-    bool is_temp_obj_used() { return results.get<2>(); }
-    uint64_t get_data_size() { return results.get<1>(); }
-    int get_result() { return results.get<0>(); }
+    bool is_temp_obj_used() { return results->started_temp_obj; }
+    uint64_t get_data_size() { return results->object_size; }
+    int get_result() { return retval; }
   };
   friend class CopyFromCallback;