From be5b25b67df730d96846074ad1c1d754dc508342 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 10 Apr 2012 15:24:49 -0700 Subject: [PATCH] filestore: fix collection_move guard We had a sequence like: 1- write A block 1 2- write A block 2 3- write A block 3 4- write A block 4 5- move A -> B - link B - unlink A - set guard on B - replay 3, 4, 5 with the result being B with only half of its content. The problem is that we destroyed the old link _and_ didn't guard the new content. Instead, set the guard before the link, and replay the unlink step here unconditionally. Fixes: #2178 Signed-off-by: Sage Weil --- src/os/FileStore.cc | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc index 46237fa10882c..1d9fbeb419f75 100644 --- a/src/os/FileStore.cc +++ b/src/os/FileStore.cc @@ -4453,19 +4453,40 @@ int FileStore::_collection_move(coll_t c, coll_t oldcid, const hobject_t& o, { dout(15) << "collection_move " << c << "/" << o << " from " << oldcid << "/" << o << dendl; - if (!_check_replay_guard(oldcid, o, spos)) - return 0; + /* + * 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 = lfn_link(oldcid, c, o); - if (r == 0 || (replaying && r == -EEXIST)) { - r = lfn_unlink(oldcid, o); + 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)); + } + } - // set guard on object so we don't do this again - 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; } -- 2.39.5