filestore: replay collection_move using add+remove

This approximates the buggy collection_move.  It is still buggy.  It is
only there to replay old journals.

Rip out buggy (and now unused) collection_move code.

For the record, the problem there is that a crash between setting the guard
and unlinking the old name will not remove the old name on replay because
the guard for the link stage is indistinguishable from that for the unlink
stage.

Signed-off-by: Sage Weil <sage.weil@dreamhost.com>

2 files changed, 4 insertions, 46 deletions

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 894bf55b92d..1ce320474e4 100644
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -2669,7 +2669,10 @@ unsigned FileStore::_do_transaction(Transaction& t, uint64_t op_seq, int trans_n
 	coll_t ocid = i.get_cid();
 	coll_t ncid = i.get_cid();
 	hobject_t oid = i.get_oid();
-	r = _collection_move(ocid, ncid, oid, spos);
+	r = _collection_add(ocid, ncid, oid, spos);
+	if (r == 0 &&
+	    _check_replay_guard(ocid, oid, spos))
+	  r = _collection_remove(ocid, oid);
       }
       break;
 
@@ -4457,49 +4460,6 @@ int FileStore::_collection_remove(coll_t c, const hobject_t& o)
   return r;
 }
 
-int FileStore::_collection_move(coll_t c, coll_t oldcid, const hobject_t& o,
-				const SequencerPosition& spos) 
-{
-  dout(15) << "collection_move " << c << "/" << o << " from " << oldcid << "/" << o << dendl;
-
-  /*
-   * Treat this as two distinct steps:
-   *
-   *  1) link to new location, with read from old location
-   *  2) remove old location
-   *
-   * We have to make sure the first part is stable in the new location
-   * before we remove it from the old location.
-   *
-   * Because we set the guard between link and unlink, replay the
-   * unlink even if the guard is set.
-   */
-
-  int r = 0;
-  if (_check_replay_guard(c, o, spos)) {
-    r = lfn_link(oldcid, c, o);
-    if (replaying && r == -EEXIST)
-      r = 0;
-    if (r == 0) {
-      // set guard on object so we don't do this again.  set this
-      // _before_ we unlink so that if we crash now we don't unlink old
-      // copy _and_ clobber the new copy due to the lack of a guard.
-      int fd = lfn_open(c, o, 0);
-      assert(fd >= 0);
-      _set_replay_guard(fd, spos);
-      TEMP_FAILURE_RETRY(::close(fd));
-    }
-  }
-
-  if (r == 0 &&
-      _check_replay_guard(oldcid, o, spos)) {
-    r = lfn_unlink(oldcid, o);
-  }
-
-  dout(10) << "collection_move " << c << "/" << o << " from " << oldcid << "/" << o << " = " << r << dendl;
-  return r;
-}
-
 int FileStore::_omap_clear(coll_t cid, const hobject_t &hoid) {
   dout(15) << __func__ << " " << cid << "/" << hoid << dendl;
   IndexedPath path;
diff --git a/src/os/FileStore.h b/src/os/FileStore.h
index 62430a5d67c..fcf0aa085f7 100644
--- a/src/os/FileStore.h
+++ b/src/os/FileStore.h
@@ -427,8 +427,6 @@ public:
   int _collection_add(coll_t c, coll_t ocid, const hobject_t& o,
 		      const SequencerPosition& spos);
   int _collection_remove(coll_t c, const hobject_t& o);
-  int _collection_move(coll_t c, coll_t ocid, const hobject_t& o,
-		       const SequencerPosition& spos);
 
 private:
   // omap