librbd: snapshot rollback should block writes

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/operation/SnapshotRollbackRequest.cc b/src/librbd/operation/SnapshotRollbackRequest.cc
index f7df78be35e36808598ce9f63941d58716e7a401..e1e472e25141dbd7131e711b845e157d2881e888 100644 (file)
--- a/src/librbd/operation/SnapshotRollbackRequest.cc
+++ b/src/librbd/operation/SnapshotRollbackRequest.cc
@@ -5,6 +5,7 @@
 #include "include/rados/librados.hpp"
 #include "common/dout.h"
 #include "common/errno.h"
+#include "librbd/AioImageRequestWQ.h"
 #include "librbd/AsyncObjectThrottle.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/ObjectMap.h"
@@ -21,40 +22,17 @@
 namespace librbd {
 namespace operation {
 
-namespace {
+using util::create_context_callback;
+using util::create_rados_safe_callback;
 
-template <typename I>
-std::ostream& operator<<(std::ostream& os,
-                         const typename SnapshotRollbackRequest<I>::State& state) {
-  switch(state) {
-  case SnapshotRollbackRequest<I>::STATE_RESIZE_IMAGE:
-    os << "RESIZE_IMAGE";
-    break;
-  case SnapshotRollbackRequest<I>::STATE_ROLLBACK_OBJECT_MAP:
-    os << "ROLLBACK_OBJECT_MAP";
-    break;
-  case SnapshotRollbackRequest<I>::STATE_ROLLBACK_OBJECTS:
-    os << "ROLLBACK_OBJECTS";
-    break;
-  case SnapshotRollbackRequest<I>::STATE_REFRESH_OBJECT_MAP:
-    os << "REFRESH_OBJECT_MAP";
-    break;
-  case SnapshotRollbackRequest<I>::STATE_INVALIDATE_CACHE:
-    os << "INVALIDATE_CACHE";
-    break;
-  default:
-    os << "UNKNOWN (" << static_cast<uint32_t>(state) << ")";
-    break;
-  }
-  return os;
-}
+namespace {
 
 template <typename I>
-class C_RollbackObject : public C_AsyncObjectThrottle<> {
+class C_RollbackObject : public C_AsyncObjectThrottle<I> {
 public:
-  C_RollbackObject(AsyncObjectThrottle<> &throttle, I *image_ctx,
+  C_RollbackObject(AsyncObjectThrottle<I> &throttle, I *image_ctx,
                    uint64_t snap_id, uint64_t object_num)
-    : C_AsyncObjectThrottle(throttle, *image_ctx), m_snap_id(snap_id),
+    : C_AsyncObjectThrottle<I>(throttle, *image_ctx), m_snap_id(snap_id),
       m_object_num(object_num) {
   }
 
@@ -97,57 +75,52 @@ SnapshotRollbackRequest<I>::SnapshotRollbackRequest(I &image_ctx,
 
 template <typename I>
 SnapshotRollbackRequest<I>::~SnapshotRollbackRequest() {
+  I &image_ctx = this->m_image_ctx;
+  if (m_blocking_writes) {
+    image_ctx.aio_work_queue->unblock_writes();
+  }
   delete m_object_map;
 }
 
 template <typename I>
 void SnapshotRollbackRequest<I>::send_op() {
-  send_resize_image();
+  send_block_writes();
 }
 
 template <typename I>
-bool SnapshotRollbackRequest<I>::should_complete(int r) {
+void SnapshotRollbackRequest<I>::send_block_writes() {
   I &image_ctx = this->m_image_ctx;
   CephContext *cct = image_ctx.cct;
-  ldout(cct, 5) << this << " " << __func__ << ": state=" << m_state << ", "
-                << "r=" << r << dendl;
-  if (r < 0) {
-    lderr(cct) << "encountered error: " << cpp_strerror(r) << dendl;
-    return true;
-  }
+  ldout(cct, 5) << this << " " << __func__ << dendl;
 
-  RWLock::RLocker owner_lock(image_ctx.owner_lock);
-  bool finished = false;
-  switch (m_state) {
-  case STATE_RESIZE_IMAGE:
-    send_rollback_object_map();
-    break;
-  case STATE_ROLLBACK_OBJECT_MAP:
-    send_rollback_objects();
-    break;
-  case STATE_ROLLBACK_OBJECTS:
-    finished = send_refresh_object_map();
-    break;
-  case STATE_REFRESH_OBJECT_MAP:
-    finished = send_invalidate_cache();
-    break;
-  case STATE_INVALIDATE_CACHE:
-    finished = true;
-    break;
-  default:
-    assert(false);
-    break;
+  m_blocking_writes = true;
+  image_ctx.aio_work_queue->block_writes(create_context_callback<
+    SnapshotRollbackRequest<I>,
+    &SnapshotRollbackRequest<I>::handle_block_writes>(this));
+}
+
+template <typename I>
+Context *SnapshotRollbackRequest<I>::handle_block_writes(int *result) {
+  I &image_ctx = this->m_image_ctx;
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
+
+  if (*result < 0) {
+    lderr(cct) << "failed to block writes: " << cpp_strerror(*result) << dendl;
+    return this->create_context_finisher();
   }
-  return finished;
+
+  send_resize_image();
+  return nullptr;
 }
 
 template <typename I>
 void SnapshotRollbackRequest<I>::send_resize_image() {
   I &image_ctx = this->m_image_ctx;
-  assert(image_ctx.owner_lock.is_locked());
 
   uint64_t current_size;
   {
+    RWLock::RLocker owner_locker(image_ctx.owner_lock);
     RWLock::RLocker snap_locker(image_ctx.snap_lock);
     current_size = image_ctx.get_image_size(CEPH_NOSNAP);
   }
@@ -159,29 +132,47 @@ void SnapshotRollbackRequest<I>::send_resize_image() {
 
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << dendl;
-  m_state = STATE_RESIZE_IMAGE;
 
-  ResizeRequest<I> *req = new ResizeRequest<I>(image_ctx,
-                                               this->create_callback_context(),
-                                               m_snap_size, m_no_op_prog_ctx);
+  Context *ctx = create_context_callback<
+    SnapshotRollbackRequest<I>,
+    &SnapshotRollbackRequest<I>::handle_resize_image>(this);
+  ResizeRequest<I> *req = ResizeRequest<I>::create(image_ctx, ctx, m_snap_size,
+                                                   m_no_op_prog_ctx, 0, true);
   req->send();
 }
 
+template <typename I>
+Context *SnapshotRollbackRequest<I>::handle_resize_image(int *result) {
+  I &image_ctx = this->m_image_ctx;
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
+
+  if (*result < 0) {
+    lderr(cct) << "failed to resize image for rollback: "
+               << cpp_strerror(*result) << dendl;
+    return this->create_context_finisher();
+  }
+
+  send_rollback_object_map();
+  return nullptr;
+}
+
 template <typename I>
 void SnapshotRollbackRequest<I>::send_rollback_object_map() {
   I &image_ctx = this->m_image_ctx;
-  assert(image_ctx.owner_lock.is_locked());
 
   {
+    RWLock::RLocker owner_locker(image_ctx.owner_lock);
     RWLock::RLocker snap_locker(image_ctx.snap_lock);
     RWLock::WLocker object_map_lock(image_ctx.object_map_lock);
     if (image_ctx.object_map != nullptr) {
       CephContext *cct = image_ctx.cct;
       ldout(cct, 5) << this << " " << __func__ << dendl;
-      m_state = STATE_ROLLBACK_OBJECT_MAP;
 
-      image_ctx.object_map->rollback(m_snap_id,
-                                     this->create_callback_context());
+      Context *ctx = create_context_callback<
+        SnapshotRollbackRequest<I>,
+        &SnapshotRollbackRequest<I>::handle_rollback_object_map>(this);
+      image_ctx.object_map->rollback(m_snap_id, ctx);
       return;
     }
   }
@@ -190,14 +181,23 @@ void SnapshotRollbackRequest<I>::send_rollback_object_map() {
 }
 
 template <typename I>
-void SnapshotRollbackRequest<I>::send_rollback_objects() {
+Context *SnapshotRollbackRequest<I>::handle_rollback_object_map(int *result) {
   I &image_ctx = this->m_image_ctx;
-  assert(image_ctx.owner_lock.is_locked());
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
+  assert(*result == 0);
+  send_rollback_objects();
+  return nullptr;
+}
+
+template <typename I>
+void SnapshotRollbackRequest<I>::send_rollback_objects() {
+  I &image_ctx = this->m_image_ctx;
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << dendl;
-  m_state = STATE_ROLLBACK_OBJECTS;
 
+  RWLock::RLocker owner_locker(image_ctx.owner_lock);
   uint64_t num_objects;
   {
     RWLock::RLocker snap_locker(image_ctx.snap_lock);
@@ -205,62 +205,105 @@ void SnapshotRollbackRequest<I>::send_rollback_objects() {
                                            image_ctx.get_current_size());
   }
 
-  Context *ctx = this->create_callback_context();
+  Context *ctx = create_context_callback<
+    SnapshotRollbackRequest<I>,
+    &SnapshotRollbackRequest<I>::handle_rollback_objects>(this);
   typename AsyncObjectThrottle<I>::ContextFactory context_factory(
     boost::lambda::bind(boost::lambda::new_ptr<C_RollbackObject<I> >(),
       boost::lambda::_1, &image_ctx, m_snap_id, boost::lambda::_2));
-  AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<>(
+  AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
     this, image_ctx, context_factory, ctx, &m_prog_ctx, 0, num_objects);
   throttle->start_ops(image_ctx.concurrent_management_ops);
 }
 
 template <typename I>
-bool SnapshotRollbackRequest<I>::send_refresh_object_map() {
+Context *SnapshotRollbackRequest<I>::handle_rollback_objects(int *result) {
   I &image_ctx = this->m_image_ctx;
-  assert(image_ctx.owner_lock.is_locked());
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
+
+  if (*result < 0) {
+    lderr(cct) << "failed to rollback objects: " << cpp_strerror(*result)
+               << dendl;
+    return this->create_context_finisher();
+  }
 
-  if (image_ctx.object_map == nullptr) {
+  return send_refresh_object_map();
+}
+
+template <typename I>
+Context *SnapshotRollbackRequest<I>::send_refresh_object_map() {
+  I &image_ctx = this->m_image_ctx;
+
+  bool object_map_enabled;
+  {
+    RWLock::RLocker owner_locker(image_ctx.owner_lock);
+    RWLock::RLocker snap_locker(image_ctx.snap_lock);
+    object_map_enabled = (image_ctx.object_map != nullptr);
+  }
+  if (!object_map_enabled) {
     return send_invalidate_cache();
   }
 
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << dendl;
-  m_state = STATE_REFRESH_OBJECT_MAP;
 
   m_object_map = image_ctx.create_object_map(CEPH_NOSNAP);
 
-  image_ctx.owner_lock.put_read();
-  Context *ctx = this->create_callback_context();
+  Context *ctx = create_context_callback<
+    SnapshotRollbackRequest<I>,
+    &SnapshotRollbackRequest<I>::handle_refresh_object_map>(this);
   m_object_map->open(ctx);
-  image_ctx.owner_lock.get_read();
+  return nullptr;
+}
 
-  return false;
+template <typename I>
+Context *SnapshotRollbackRequest<I>::handle_refresh_object_map(int *result) {
+  I &image_ctx = this->m_image_ctx;
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
+
+  assert(*result == 0);
+  return send_invalidate_cache();
 }
 
 template <typename I>
-bool SnapshotRollbackRequest<I>::send_invalidate_cache() {
+Context *SnapshotRollbackRequest<I>::send_invalidate_cache() {
   I &image_ctx = this->m_image_ctx;
-  assert(image_ctx.owner_lock.is_locked());
 
   apply();
-
   if (image_ctx.object_cacher == NULL) {
-    return true;
+    return this->create_context_finisher();
   }
 
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << dendl;
-  m_state = STATE_INVALIDATE_CACHE;
 
-  image_ctx.invalidate_cache(this->create_callback_context());
-  return false;
+  Context *ctx = create_context_callback<
+    SnapshotRollbackRequest<I>,
+    &SnapshotRollbackRequest<I>::handle_invalidate_cache>(this);
+  image_ctx.invalidate_cache(ctx);
+  return nullptr;
+}
+
+template <typename I>
+Context *SnapshotRollbackRequest<I>::handle_invalidate_cache(int *result) {
+  I &image_ctx = this->m_image_ctx;
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
+
+  if (*result < 0) {
+    lderr(cct) << "failed to invalidate cache: " << cpp_strerror(*result)
+               << dendl;
+  }
+  return this->create_context_finisher();
 }
 
 template <typename I>
 void SnapshotRollbackRequest<I>::apply() {
   I &image_ctx = this->m_image_ctx;
 
-  assert(image_ctx.owner_lock.is_locked());
+  RWLock::RLocker owner_locker(image_ctx.owner_lock);
   RWLock::WLocker snap_locker(image_ctx.snap_lock);
   if (image_ctx.object_map != nullptr) {
     std::swap(m_object_map, image_ctx.object_map);

diff --git a/src/librbd/operation/SnapshotRollbackRequest.h b/src/librbd/operation/SnapshotRollbackRequest.h
index 5c6589a9317feade80e6c1c5d64ed7edc239af1d..53fb85b2fa3bfd6fe52e453062d8d4317c32acb3 100644 (file)
--- a/src/librbd/operation/SnapshotRollbackRequest.h
+++ b/src/librbd/operation/SnapshotRollbackRequest.h
@@ -27,24 +27,27 @@ public:
    * @verbatim
    *
    * <start> ---------\
-   *  .               |
-   *  .               v
-   *  .         STATE_RESIZE_IMAGE
-   *  .               |
-   *  . (skip path)   v
-   *  . . . . > STATE_ROLLBACK_OBJECT_MAP
-   *  .               |
-   *  .               v
-   *  . . . . > STATE_ROLLBACK_OBJECTS . . . . . . . . . . .
-   *                  |                                    .
-   *                  v                                    .
-   *            STATE_REFRESH_OBJECT_MAP  (skip if object  .
-   *                  |                    map disabled)   .
-   *                  v                                    .
-   *            STATE_INVALIDATE_CACHE                     .
-   *                  |                                    .
-   *                  v                                    .
-   *              <finish> < . . . . . . . . . . . . . . . .
+   *                  |
+   *                  v
+   *            STATE_BLOCK_WRITES
+   *                  |
+   *                  v
+   *            STATE_RESIZE_IMAGE (skip if resize not
+   *                  |             required)
+   *                  v
+   *            STATE_ROLLBACK_OBJECT_MAP (skip if object
+   *                  |                    map disabled)
+   *                  v
+   *            STATE_ROLLBACK_OBJECTS
+   *                  |
+   *                  v
+   *            STATE_REFRESH_OBJECT_MAP  (skip if object
+   *                  |                    map disabled)
+   *                  v
+   *            STATE_INVALIDATE_CACHE (skip if cache
+   *                  |                 disabled)
+   *                  v
+   *              <finish>
    *
    * @endverbatim
    *
@@ -52,13 +55,6 @@ public:
    * The _ROLLBACK_OBJECT_MAP state is skipped if the object map isn't enabled.
    * The _INVALIDATE_CACHE state is skipped if the cache isn't enabled.
    */
-  enum State {
-    STATE_RESIZE_IMAGE,
-    STATE_ROLLBACK_OBJECT_MAP,
-    STATE_ROLLBACK_OBJECTS,
-    STATE_REFRESH_OBJECT_MAP,
-    STATE_INVALIDATE_CACHE
-  };
 
   SnapshotRollbackRequest(ImageCtxT &image_ctx, Context *on_finish,
                           const std::string &snap_name, uint64_t snap_id,
@@ -67,7 +63,9 @@ public:
 
 protected:
   virtual void send_op();
-  virtual bool should_complete(int r);
+  virtual bool should_complete(int r) {
+    return true;
+  }
 
   virtual journal::Event create_event(uint64_t op_tid) const {
     return journal::SnapRollbackEvent(op_tid, m_snap_name);
@@ -80,15 +78,27 @@ private:
   ProgressContext &m_prog_ctx;
 
   NoOpProgressContext m_no_op_prog_ctx;
-  State m_state;
 
+  bool m_blocking_writes = false;
   decltype(ImageCtxT::object_map) m_object_map;
 
+  void send_block_writes();
+  Context *handle_block_writes(int *result);
+
   void send_resize_image();
+  Context *handle_resize_image(int *result);
+
   void send_rollback_object_map();
+  Context *handle_rollback_object_map(int *result);
+
   void send_rollback_objects();
-  bool send_refresh_object_map();
-  bool send_invalidate_cache();
+  Context *handle_rollback_objects(int *result);
+
+  Context *send_refresh_object_map();
+  Context *handle_refresh_object_map(int *result);
+
+  Context *send_invalidate_cache();
+  Context *handle_invalidate_cache(int *result);
 
   void apply();
 };