From 6fe9be8de391aa239fc82f5483508d762a44cb99 Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Wed, 7 Dec 2016 16:28:22 -0500
Subject: [PATCH 1/5] librbd: update in-memory object map after on-disk update
 committed

Concurrent IO to the same object would previously result in the first
IO pausing to update the object map while the other IO would proceed
to directly update the object before the object map state was properly
updated.

Fixes: http://tracker.ceph.com/issues/16176
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 378b810cbaadb1a12a7f0d21ed9a33e2a9640f55)

Conflicts:
	test/librbd/object_map/test_mock_UpdateRequest.cc: resize op signature
---
 src/librbd/object_map/Request.cc              |  5 +---
 src/librbd/object_map/ResizeRequest.cc        |  4 ++-
 src/librbd/object_map/UpdateRequest.cc        | 29 ++++++++++---------
 .../object_map/test_mock_UpdateRequest.cc     |  6 +++-
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/librbd/object_map/Request.cc b/src/librbd/object_map/Request.cc
index 1725cbf907541..e54aed0b21e0a 100644
--- a/src/librbd/object_map/Request.cc
+++ b/src/librbd/object_map/Request.cc
@@ -28,10 +28,7 @@ bool Request::should_complete(int r) {
       return invalidate();
     }
 
-    {
-      RWLock::WLocker l2(m_image_ctx.object_map_lock);
-      finish_request();
-    }
+    finish_request();
     return true;
 
   case STATE_INVALIDATE:
diff --git a/src/librbd/object_map/ResizeRequest.cc b/src/librbd/object_map/ResizeRequest.cc
index e574a18472966..5698fc80f7bf2 100644
--- a/src/librbd/object_map/ResizeRequest.cc
+++ b/src/librbd/object_map/ResizeRequest.cc
@@ -48,10 +48,12 @@ void ResizeRequest::send() {
 }
 
 void ResizeRequest::finish_request() {
-  CephContext *cct = m_image_ctx.cct;
+  RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
 
+  CephContext *cct = m_image_ctx.cct;
   ldout(cct, 5) << this << " resizing in-memory object map: "
 		<< m_num_objs << dendl;
+
   resize(m_object_map, m_num_objs, m_default_object_state);
 }
 
diff --git a/src/librbd/object_map/UpdateRequest.cc b/src/librbd/object_map/UpdateRequest.cc
index 51dbc48827f5b..3edf32aa67fa7 100644
--- a/src/librbd/object_map/UpdateRequest.cc
+++ b/src/librbd/object_map/UpdateRequest.cc
@@ -33,20 +33,6 @@ void UpdateRequest::send() {
 		 << "->" << static_cast<uint32_t>(m_new_state)
 		 << dendl;
 
-  // rebuilding the object map might update on-disk only
-  if (m_snap_id == m_image_ctx.snap_id) {
-    assert(m_image_ctx.object_map_lock.is_wlocked());
-    for (uint64_t object_no = m_start_object_no;
-         object_no < MIN(m_end_object_no, m_object_map.size());
-         ++object_no) {
-      uint8_t state = m_object_map[object_no];
-      if (!m_current_state || state == *m_current_state ||
-          (*m_current_state == OBJECT_EXISTS && state == OBJECT_EXISTS_CLEAN)) {
-        m_object_map[object_no] = m_new_state;
-      }
-    }
-  }
-
   librados::ObjectWriteOperation op;
   if (m_snap_id == CEPH_NOSNAP) {
     rados::cls::lock::assert_locked(&op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, "", "");
@@ -61,8 +47,23 @@ void UpdateRequest::send() {
 }
 
 void UpdateRequest::finish_request() {
+  RWLock::RLocker snap_locker(m_image_ctx.snap_lock);
+  RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
   ldout(m_image_ctx.cct, 20) << this << " on-disk object map updated"
                              << dendl;
+
+  // rebuilding the object map might update on-disk only
+  if (m_snap_id == m_image_ctx.snap_id) {
+    for (uint64_t object_no = m_start_object_no;
+         object_no < MIN(m_end_object_no, m_object_map.size());
+         ++object_no) {
+      uint8_t state = m_object_map[object_no];
+      if (!m_current_state || state == *m_current_state ||
+          (*m_current_state == OBJECT_EXISTS && state == OBJECT_EXISTS_CLEAN)) {
+        m_object_map[object_no] = m_new_state;
+      }
+    }
+  }
 }
 
 } // namespace object_map
diff --git a/src/test/librbd/object_map/test_mock_UpdateRequest.cc b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
index 45879f2f83e60..67ace6249116f 100644
--- a/src/test/librbd/object_map/test_mock_UpdateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
@@ -8,6 +8,7 @@
 #include "librbd/ImageState.h"
 #include "librbd/internal.h"
 #include "librbd/ObjectMap.h"
+#include "librbd/Operations.h"
 #include "librbd/object_map/UpdateRequest.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -56,10 +57,13 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateInMemory) {
 
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  librbd::NoOpProgressContext no_progress;
+  ASSERT_EQ(0, ictx->operations->resize(4 << ictx->order, no_progress));
   ASSERT_EQ(0, acquire_exclusive_lock(*ictx));
 
   ceph::BitVector<2> object_map;
-  object_map.resize(1024);
+  object_map.resize(4);
   for (uint64_t i = 0; i < object_map.size(); ++i) {
     object_map[i] = i % 4;
   }

From c53df3780dd9221cfe602c09651eeee06046ebeb Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Wed, 7 Dec 2016 22:41:56 -0500
Subject: [PATCH 2/5] librbd: clean up object map update interface

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 477ae54a568bd1081fd2c77b570b04dd1b983cd9)

Conflicts:
	src/librbd/AioObjectRequest.cc: trivial resolution
	src/librbd/ObjectMap.cc: trivial resolution
	src/librbd/operation/TrimRequest.cc: removed optimizations
	src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc: trivial resolution
---
 src/librbd/AioObjectRequest.cc                | 45 ++++++---------
 src/librbd/CopyupRequest.cc                   | 10 ++--
 src/librbd/ObjectMap.cc                       | 55 ++++++++-----------
 src/librbd/ObjectMap.h                        | 49 +++++++++++++----
 src/librbd/operation/TrimRequest.cc           | 44 +++++----------
 src/test/librbd/mock/MockObjectMap.h          | 21 ++++++-
 .../image_sync/test_mock_ObjectCopyRequest.cc | 21 +++----
 .../image_sync/ObjectCopyRequest.cc           |  9 +--
 8 files changed, 130 insertions(+), 124 deletions(-)

diff --git a/src/librbd/AioObjectRequest.cc b/src/librbd/AioObjectRequest.cc
index cf7617640b2c2..36ec0e7334cac 100644
--- a/src/librbd/AioObjectRequest.cc
+++ b/src/librbd/AioObjectRequest.cc
@@ -454,12 +454,10 @@ void AbstractAioObjectWrite::send() {
 void AbstractAioObjectWrite::send_pre() {
   assert(m_ictx->owner_lock.is_locked());
 
-  bool write = false;
   {
     RWLock::RLocker snap_lock(m_ictx->snap_lock);
     if (m_ictx->object_map == nullptr) {
       m_object_exist = true;
-      write = true;
     } else {
       // should have been flushed prior to releasing lock
       assert(m_ictx->exclusive_lock->is_lock_owner());
@@ -469,27 +467,20 @@ void AbstractAioObjectWrite::send_pre() {
       pre_object_map_update(&new_state);
 
       RWLock::WLocker object_map_locker(m_ictx->object_map_lock);
-      if (m_ictx->object_map->update_required(m_object_no, new_state)) {
-        ldout(m_ictx->cct, 20) << "send_pre " << this << " " << m_oid << " "
-                               << m_object_off << "~" << m_object_len
-                               << dendl;
-        m_state = LIBRBD_AIO_WRITE_PRE;
-
-        Context *ctx = util::create_context_callback<AioObjectRequest>(this);
-        bool updated = m_ictx->object_map->aio_update(m_object_no, new_state,
-                                                      {}, ctx);
-        assert(updated);
-      } else {
-        write = true;
+      ldout(m_ictx->cct, 20) << "send_pre " << this << " " << m_oid << " "
+                             << m_object_off << "~" << m_object_len
+                             << dendl;
+      m_state = LIBRBD_AIO_WRITE_PRE;
+
+      if (m_ictx->object_map->aio_update<AioObjectRequest>(
+            CEPH_NOSNAP, m_object_no, new_state, {}, this)) {
+        return;
       }
     }
   }
 
-  // avoid possible recursive lock attempts
-  if (write) {
-    // no object map update required
-    send_write();
-  }
+  // no object map update required
+  send_write();
 }
 
 bool AbstractAioObjectWrite::send_post() {
@@ -503,20 +494,16 @@ bool AbstractAioObjectWrite::send_post() {
   assert(m_ictx->exclusive_lock->is_lock_owner());
 
   RWLock::WLocker object_map_locker(m_ictx->object_map_lock);
-  if (!m_ictx->object_map->update_required(m_object_no, OBJECT_NONEXISTENT)) {
-    return true;
-  }
-
   ldout(m_ictx->cct, 20) << "send_post " << this << " " << m_oid << " "
                          << m_object_off << "~" << m_object_len << dendl;
   m_state = LIBRBD_AIO_WRITE_POST;
 
-  Context *ctx = util::create_context_callback<AioObjectRequest>(this);
-  bool updated = m_ictx->object_map->aio_update(m_object_no,
-                                                OBJECT_NONEXISTENT,
-      			                  OBJECT_PENDING, ctx);
-  assert(updated);
-  return false;
+  if (m_ictx->object_map->aio_update<AioObjectRequest>(
+        CEPH_NOSNAP, m_object_no, OBJECT_NONEXISTENT, OBJECT_PENDING, this)) {
+    return false;
+  }
+
+  return true;
 }
 
 void AbstractAioObjectWrite::send_write() {
diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index b95544b28494a..d9750b6e52438 100644
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -46,9 +46,8 @@ class UpdateObjectMap : public C_AsyncObjectThrottle<> {
       RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
       assert(m_image_ctx.exclusive_lock->is_lock_owner());
       assert(m_image_ctx.object_map != nullptr);
-      bool sent = m_image_ctx.object_map->aio_update(m_object_no, OBJECT_EXISTS,
-                                                     boost::optional<uint8_t>(),
-                                                     this);
+      bool sent = m_image_ctx.object_map->aio_update<Context>(
+        CEPH_NOSNAP, m_object_no, OBJECT_EXISTS, {}, this);
       return (sent ? 0 : 1);
     }
 
@@ -64,8 +63,9 @@ class UpdateObjectMap : public C_AsyncObjectThrottle<> {
       return 1;
     }
 
-    m_image_ctx.object_map->aio_update(snap_id, m_object_no, m_object_no + 1,
-                                       state, boost::optional<uint8_t>(), this);
+    bool sent = m_image_ctx.object_map->aio_update<Context>(
+      snap_id, m_object_no, state, {}, this);
+    assert(sent);
     return 0;
   }
 
diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index b5d659ef8a2e8..e1e7220d4ccb5 100644
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -1,5 +1,6 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
+
 #include "librbd/ObjectMap.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/ImageCtx.h"
@@ -187,26 +188,17 @@ void ObjectMap::aio_resize(uint64_t new_size, uint8_t default_object_state,
   req->send();
 }
 
-bool ObjectMap::aio_update(uint64_t object_no, uint8_t new_state,
-			   const boost::optional<uint8_t> &current_state,
-			   Context *on_finish)
-{
-  return aio_update(object_no, object_no + 1, new_state, current_state,
-		    on_finish);
-}
-
-bool ObjectMap::aio_update(uint64_t start_object_no, uint64_t end_object_no,
-			   uint8_t new_state,
+void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no,
+                           uint64_t end_object_no, uint8_t new_state,
                            const boost::optional<uint8_t> &current_state,
-                           Context *on_finish)
-{
+                           Context *on_finish) {
   assert(m_image_ctx.snap_lock.is_locked());
   assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0);
-  assert(m_image_ctx.owner_lock.is_locked());
+  assert(snap_id != CEPH_NOSNAP || m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.image_watcher != NULL);
   assert(m_image_ctx.exclusive_lock == nullptr ||
          m_image_ctx.exclusive_lock->is_lock_owner());
-  assert(m_image_ctx.object_map_lock.is_wlocked());
+  assert(snap_id != CEPH_NOSNAP || m_image_ctx.object_map_lock.is_wlocked());
   assert(start_object_no < end_object_no);
 
   CephContext *cct = m_image_ctx.cct;
@@ -215,29 +207,26 @@ bool ObjectMap::aio_update(uint64_t start_object_no, uint64_t end_object_no,
                  << (current_state ?
                        stringify(static_cast<uint32_t>(*current_state)) : "")
 		 << "->" << static_cast<uint32_t>(new_state) << dendl;
-  if (end_object_no > m_object_map.size()) {
-    ldout(cct, 20) << "skipping update of invalid object map" << dendl;
-    return false;
-  }
+  if (snap_id == CEPH_NOSNAP) {
+    if (end_object_no > m_object_map.size()) {
+      ldout(cct, 20) << "skipping update of invalid object map" << dendl;
+      m_image_ctx.op_work_queue->queue(on_finish, 0);
+      return;
+    }
 
-  for (uint64_t object_no = start_object_no; object_no < end_object_no;
-       ++object_no) {
-    uint8_t state = m_object_map[object_no];
-    if ((!current_state || state == *current_state ||
-          (*current_state == OBJECT_EXISTS && state == OBJECT_EXISTS_CLEAN)) &&
-        state != new_state) {
-      aio_update(m_snap_id, start_object_no, end_object_no, new_state,
-                 current_state, on_finish);
-      return true;
+    uint64_t object_no;
+    for (object_no = start_object_no; object_no < end_object_no; ++object_no) {
+      if (update_required(object_no, new_state)) {
+        break;
+      }
+    }
+    if (object_no == end_object_no) {
+      ldout(cct, 20) << "object map update not required" << dendl;
+      m_image_ctx.op_work_queue->queue(on_finish, 0);
+      return;
     }
   }
-  return false;
-}
 
-void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no,
-                           uint64_t end_object_no, uint8_t new_state,
-                           const boost::optional<uint8_t> &current_state,
-                           Context *on_finish) {
   object_map::UpdateRequest *req = new object_map::UpdateRequest(
     m_image_ctx, &m_object_map, snap_id, start_object_no, end_object_no,
     new_state, current_state, on_finish);
diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h
index 5d99180e771a3..44ddaf0548a8b 100644
--- a/src/librbd/ObjectMap.h
+++ b/src/librbd/ObjectMap.h
@@ -1,5 +1,6 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
+
 #ifndef CEPH_LIBRBD_OBJECT_MAP_H
 #define CEPH_LIBRBD_OBJECT_MAP_H
 
@@ -7,6 +8,7 @@
 #include "include/fs_types.h"
 #include "include/rbd/object_map_types.h"
 #include "common/bit_vector.hpp"
+#include "librbd/Utils.h"
 #include <boost/optional.hpp>
 
 class Context;
@@ -39,23 +41,44 @@ class ObjectMap {
   void close(Context *on_finish);
 
   bool object_may_exist(uint64_t object_no) const;
-  bool update_required(uint64_t object_no, uint8_t new_state);
 
   void aio_save(Context *on_finish);
   void aio_resize(uint64_t new_size, uint8_t default_object_state,
 		  Context *on_finish);
-  bool aio_update(uint64_t object_no, uint8_t new_state,
-		  const boost::optional<uint8_t> &current_state,
-		  Context *on_finish);
-  bool aio_update(uint64_t start_object_no, uint64_t end_object_no,
-		  uint8_t new_state,
-		  const boost::optional<uint8_t> &current_state,
-		  Context *on_finish);
 
-  void aio_update(uint64_t snap_id, uint64_t start_object_no,
+  template <typename T, void(T::*MF)(int) = &T::complete>
+  bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint8_t new_state,
+                  const boost::optional<uint8_t> &current_state,
+                  T *callback_object) {
+    return aio_update<T, MF>(snap_id, start_object_no, start_object_no + 1,
+                             new_state, current_state, callback_object);
+  }
+
+  template <typename T, void(T::*MF)(int) = &T::complete>
+  bool aio_update(uint64_t snap_id, uint64_t start_object_no,
                   uint64_t end_object_no, uint8_t new_state,
                   const boost::optional<uint8_t> &current_state,
-                  Context *on_finish);
+                  T *callback_object) {
+    assert(start_object_no < end_object_no);
+    if (snap_id == CEPH_NOSNAP) {
+      uint64_t object_no;
+      for (object_no = start_object_no; object_no < end_object_no;
+           ++object_no) {
+        if (update_required(object_no, new_state)) {
+          break;
+        }
+      }
+
+      if (object_no == end_object_no) {
+        return false;
+      }
+    }
+
+    aio_update(snap_id, start_object_no, end_object_no, new_state,
+               current_state,
+               util::create_context_callback<T, MF>(callback_object));
+    return true;
+  }
 
   void rollback(uint64_t snap_id, Context *on_finish);
   void snapshot_add(uint64_t snap_id, Context *on_finish);
@@ -66,6 +89,12 @@ class ObjectMap {
   ceph::BitVector<2> m_object_map;
   uint64_t m_snap_id;
 
+  void aio_update(uint64_t snap_id, uint64_t start_object_no,
+                  uint64_t end_object_no, uint8_t new_state,
+                  const boost::optional<uint8_t> &current_state,
+                  Context *on_finish);
+  bool update_required(uint64_t object_no, uint8_t new_state);
+
 };
 
 } // namespace librbd
diff --git a/src/librbd/operation/TrimRequest.cc b/src/librbd/operation/TrimRequest.cc
index 3992fb75e21c2..e99e0e95a21b7 100644
--- a/src/librbd/operation/TrimRequest.cc
+++ b/src/librbd/operation/TrimRequest.cc
@@ -254,12 +254,9 @@ void TrimRequest<I>::send_pre_remove() {
     return;
   }
 
-  bool remove_objects = false;
   {
     RWLock::RLocker snap_locker(image_ctx.snap_lock);
-    if (image_ctx.object_map == nullptr) {
-      remove_objects = true;
-    } else {
+    if (image_ctx.object_map != nullptr) {
       ldout(image_ctx.cct, 5) << this << " send_pre_remove: "
 				<< " delete_start=" << m_delete_start
 				<< " num_objects=" << m_num_objects << dendl;
@@ -268,22 +265,17 @@ void TrimRequest<I>::send_pre_remove() {
       assert(image_ctx.exclusive_lock->is_lock_owner());
 
       // flag the objects as pending deletion
-      Context *ctx = this->create_callback_context();
       RWLock::WLocker object_map_locker(image_ctx.object_map_lock);
-      if (!image_ctx.object_map->aio_update(m_delete_start, m_num_objects,
-					    OBJECT_PENDING, OBJECT_EXISTS,
-                                            ctx)) {
-        delete ctx;
-        remove_objects = true;
+      if (image_ctx.object_map->template aio_update<AsyncRequest<I> >(
+            CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_PENDING,
+            OBJECT_EXISTS, this)) {
+        return;
       }
     }
   }
 
-  // avoid possible recursive lock attempts
-  if (remove_objects) {
-    // no object map update required
-    send_remove_objects();
-  }
+  // no object map update required
+  send_remove_objects();
 }
 
 template <typename I>
@@ -291,12 +283,9 @@ void TrimRequest<I>::send_post_remove() {
   I &image_ctx = this->m_image_ctx;
   assert(image_ctx.owner_lock.is_locked());
 
-  bool clean_boundary = false;
   {
     RWLock::RLocker snap_locker(image_ctx.snap_lock);
-    if (image_ctx.object_map == nullptr) {
-      clean_boundary = true;
-    } else {
+    if (image_ctx.object_map != nullptr) {
       ldout(image_ctx.cct, 5) << this << " send_post_remove: "
           		        << " delete_start=" << m_delete_start
           		        << " num_objects=" << m_num_objects << dendl;
@@ -305,22 +294,17 @@ void TrimRequest<I>::send_post_remove() {
       assert(image_ctx.exclusive_lock->is_lock_owner());
 
       // flag the pending objects as removed
-      Context *ctx = this->create_callback_context();
       RWLock::WLocker object_map_locker(image_ctx.object_map_lock);
-      if (!image_ctx.object_map->aio_update(m_delete_start, m_num_objects,
-					    OBJECT_NONEXISTENT,
-					    OBJECT_PENDING, ctx)) {
-        delete ctx;
-	clean_boundary = true;
+      if (image_ctx.object_map->template aio_update<AsyncRequest<I> >(
+            CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_NONEXISTENT,
+            OBJECT_PENDING, this)) {
+        return;
       }
     }
   }
 
-  // avoid possible recursive lock attempts
-  if (clean_boundary) {
-    // no object map update required
-    send_clean_boundary();
-  }
+  // no object map update required
+  send_clean_boundary();
 }
 
 template <typename I>
diff --git a/src/test/librbd/mock/MockObjectMap.h b/src/test/librbd/mock/MockObjectMap.h
index 25d14ed3c085c..dd274237908cd 100644
--- a/src/test/librbd/mock/MockObjectMap.h
+++ b/src/test/librbd/mock/MockObjectMap.h
@@ -5,6 +5,7 @@
 #define CEPH_TEST_LIBRBD_MOCK_OBJECT_MAP_H
 
 #include "common/RWLock.h"
+#include "librbd/Utils.h"
 #include "gmock/gmock.h"
 
 namespace librbd {
@@ -17,7 +18,25 @@ struct MockObjectMap {
 
   MOCK_METHOD3(aio_resize, void(uint64_t new_size, uint8_t default_object_state,
                                 Context *on_finish));
-  MOCK_METHOD6(aio_update, void(uint64_t snap_id, uint64_t start_object_no,
+
+  template <typename T, void(T::*MF)(int)>
+  bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint8_t new_state,
+                  const boost::optional<uint8_t> &current_state,
+                  T *callback_object) {
+    return aio_update<T, MF>(snap_id, start_object_no, start_object_no + 1,
+                             new_state, current_state, callback_object);
+  }
+
+  template <typename T, void(T::*MF)(int)>
+  bool aio_update(uint64_t snap_id, uint64_t start_object_no,
+                  uint64_t end_object_no, uint8_t new_state,
+                  const boost::optional<uint8_t> &current_state,
+                  T *callback_object) {
+    return aio_update(snap_id, start_object_no, end_object_no, new_state,
+                      current_state,
+                      util::create_context_callback<T, MF>(callback_object));
+  }
+  MOCK_METHOD6(aio_update, bool(uint64_t snap_id, uint64_t start_object_no,
                                 uint64_t end_object_no, uint8_t new_state,
                                 const boost::optional<uint8_t> &current_state,
                                 Context *on_finish));
diff --git a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc
index b018f16c21b30..5a2fa9e5a65c6 100644
--- a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc
+++ b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc
@@ -174,17 +174,18 @@ class TestMockImageSyncObjectCopyRequest : public TestMockFixture {
     if (mock_image_ctx.image_ctx->object_map != nullptr) {
       auto &expect = EXPECT_CALL(mock_object_map, aio_update(snap_id, 0, 1, state, _, _));
       if (r < 0) {
-        expect.WillOnce(WithArg<5>(Invoke([this, r](Context *ctx) {
-            m_threads->work_queue->queue(ctx, r);
-          })));
+        expect.WillOnce(DoAll(WithArg<5>(Invoke([this, r](Context *ctx) {
+                                  m_threads->work_queue->queue(ctx, r);
+                                })),
+                              Return(true)));
       } else {
-        expect.WillOnce(WithArg<5>(Invoke([&mock_image_ctx, snap_id, state, r](Context *ctx) {
-            assert(mock_image_ctx.image_ctx->snap_lock.is_locked());
-            assert(mock_image_ctx.image_ctx->object_map_lock.is_wlocked());
-            mock_image_ctx.image_ctx->object_map->aio_update(snap_id, 0, 1,
-                                                             state,
-                                                             boost::none, ctx);
-          })));
+        expect.WillOnce(DoAll(WithArg<5>(Invoke([&mock_image_ctx, snap_id, state, r](Context *ctx) {
+                                  assert(mock_image_ctx.image_ctx->snap_lock.is_locked());
+                                  assert(mock_image_ctx.image_ctx->object_map_lock.is_wlocked());
+                                  mock_image_ctx.image_ctx->object_map->aio_update<Context>(
+                                    snap_id, 0, 1, state, boost::none, ctx);
+                                })),
+                              Return(true)));
       }
     }
   }
diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
index 9975b5bf81576..5c024738e1f92 100644
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
@@ -255,14 +255,11 @@ void ObjectCopyRequest<I>::send_update_object_map() {
            << dendl;
 
   RWLock::WLocker object_map_locker(m_local_image_ctx->object_map_lock);
-  Context *ctx = create_context_callback<
+  bool sent = m_local_image_ctx->object_map->template aio_update<
     ObjectCopyRequest<I>, &ObjectCopyRequest<I>::handle_update_object_map>(
+      snap_object_state.first, m_object_number, snap_object_state.second, {},
       this);
-  m_local_image_ctx->object_map->aio_update(snap_object_state.first,
-                                            m_object_number,
-                                            m_object_number + 1,
-                                            snap_object_state.second,
-                                            boost::none, ctx);
+  assert(sent);
   m_local_image_ctx->snap_lock.put_read();
 }
 

From 5d306fd015d3e6d0aa35f368d3ca6cde6e7ca77b Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 8 Dec 2016 18:27:33 -0500
Subject: [PATCH 3/5] librbd: convert ObjectMap to template for unit testing

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit ea7b30a4fb105e052603c55ac2dc2aca11e66545)

Conflicts:
	src/librbd/image/CreateRequest.cc: not in jewel
	src/librbd/internal.cc: trivial resolution
	src/librbd/object_map/CreateRequest.cc: not in jewel
	src/librbd/object_map/RemoveRequest.cc: not in jewel
	src/test/librbd/test_ObjectMap.cc: trivial resolution
---
 src/librbd/DiffIterate.cc                     |  4 +-
 src/librbd/ImageCtx.cc                        |  4 +-
 src/librbd/ImageCtx.h                         |  6 +-
 src/librbd/ObjectMap.cc                       | 68 ++++++++++++-------
 src/librbd/ObjectMap.h                        | 11 ++-
 src/librbd/Operations.cc                      |  4 +-
 src/librbd/image/SetSnapRequest.cc            |  2 +-
 src/librbd/image/SetSnapRequest.h             |  4 +-
 src/librbd/internal.cc                        | 16 ++---
 src/librbd/object_map/LockRequest.cc          |  6 +-
 src/librbd/object_map/RefreshRequest.cc       |  8 +--
 src/librbd/object_map/RefreshRequest.h        |  6 ++
 src/librbd/object_map/ResizeRequest.cc        |  2 +-
 .../object_map/SnapshotCreateRequest.cc       |  6 +-
 .../object_map/SnapshotRemoveRequest.cc       |  6 +-
 .../object_map/SnapshotRollbackRequest.cc     |  5 +-
 src/librbd/object_map/UnlockRequest.cc        |  2 +-
 src/librbd/object_map/UnlockRequest.h         |  4 ++
 src/librbd/object_map/UpdateRequest.cc        | 10 ++-
 src/librbd/object_map/UpdateRequest.h         | 16 ++++-
 .../object_map/test_mock_LockRequest.cc       |  9 ++-
 .../object_map/test_mock_RefreshRequest.cc    |  8 ++-
 .../object_map/test_mock_ResizeRequest.cc     |  2 +-
 .../test_mock_SnapshotCreateRequest.cc        | 10 +--
 .../test_mock_SnapshotRemoveRequest.cc        |  6 +-
 .../test_mock_SnapshotRollbackRequest.cc      | 10 +--
 .../object_map/test_mock_UnlockRequest.cc     |  3 +-
 .../object_map/test_mock_UpdateRequest.cc     | 12 ++--
 src/test/librbd/test_ObjectMap.cc             | 12 ++--
 src/test/librbd/test_internal.cc              |  2 +-
 .../test_mock_SnapshotCreateRequest.cc        |  4 +-
 .../image_sync/SnapshotCreateRequest.cc       |  2 +-
 32 files changed, 165 insertions(+), 105 deletions(-)

diff --git a/src/librbd/DiffIterate.cc b/src/librbd/DiffIterate.cc
index 9b0a3ac3daa86..9a229dd834196 100644
--- a/src/librbd/DiffIterate.cc
+++ b/src/librbd/DiffIterate.cc
@@ -382,8 +382,8 @@ int DiffIterate::diff_object_map(uint64_t from_snap_id, uint64_t to_snap_id,
     }
 
     BitVector<2> object_map;
-    std::string oid(ObjectMap::object_map_name(m_image_ctx.id,
-                                               current_snap_id));
+    std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id,
+                                                 current_snap_id));
     r = cls_client::object_map_load(&m_image_ctx.md_ctx, oid, &object_map);
     if (r < 0) {
       lderr(cct) << "diff_object_map: failed to load object map " << oid
diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 4226d42747138..863a1235e4aa8 100644
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -995,8 +995,8 @@ struct C_InvalidateCache : public Context {
     return new ExclusiveLock<ImageCtx>(*this);
   }
 
-  ObjectMap *ImageCtx::create_object_map(uint64_t snap_id) {
-    return new ObjectMap(*this, snap_id);
+  ObjectMap<ImageCtx> *ImageCtx::create_object_map(uint64_t snap_id) {
+    return new ObjectMap<ImageCtx>(*this, snap_id);
   }
 
   Journal<ImageCtx> *ImageCtx::create_journal() {
diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index b8a3bf6cf24a8..c1cec019d8088 100644
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -45,7 +45,7 @@ namespace librbd {
   template <typename> class ImageWatcher;
   template <typename> class Journal;
   class LibrbdAdminSocketHook;
-  class ObjectMap;
+  template <typename> class ObjectMap;
   template <typename> class Operations;
   class LibrbdWriteback;
 
@@ -141,7 +141,7 @@ namespace librbd {
     Operations<ImageCtx> *operations;
 
     ExclusiveLock<ImageCtx> *exclusive_lock;
-    ObjectMap *object_map;
+    ObjectMap<ImageCtx> *object_map;
 
     xlist<operation::ResizeRequest<ImageCtx>*> resize_reqs;
 
@@ -287,7 +287,7 @@ namespace librbd {
     void apply_metadata(const std::map<std::string, bufferlist> &meta);
 
     ExclusiveLock<ImageCtx> *create_exclusive_lock();
-    ObjectMap *create_object_map(uint64_t snap_id);
+    ObjectMap<ImageCtx> *create_object_map(uint64_t snap_id);
     Journal<ImageCtx> *create_journal();
 
     void clear_pending_completions();
diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index e1e7220d4ccb5..9d57ea8b5a124 100644
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -30,17 +30,20 @@
 
 namespace librbd {
 
-ObjectMap::ObjectMap(ImageCtx &image_ctx, uint64_t snap_id)
+template <typename I>
+ObjectMap<I>::ObjectMap(I &image_ctx, uint64_t snap_id)
   : m_image_ctx(image_ctx), m_snap_id(snap_id)
 {
 }
 
-int ObjectMap::remove(librados::IoCtx &io_ctx, const std::string &image_id) {
+template <typename I>
+int ObjectMap<I>::remove(librados::IoCtx &io_ctx, const std::string &image_id) {
   return io_ctx.remove(object_map_name(image_id, CEPH_NOSNAP));
 }
 
-std::string ObjectMap::object_map_name(const std::string &image_id,
-				       uint64_t snap_id) {
+template <typename I>
+std::string ObjectMap<I>::object_map_name(const std::string &image_id,
+				          uint64_t snap_id) {
   std::string oid(RBD_OBJECT_MAP_PREFIX + image_id);
   if (snap_id != CEPH_NOSNAP) {
     std::stringstream snap_suffix;
@@ -51,26 +54,30 @@ std::string ObjectMap::object_map_name(const std::string &image_id,
   return oid;
 }
 
-bool ObjectMap::is_compatible(const file_layout_t& layout, uint64_t size) {
+template <typename I>
+bool ObjectMap<I>::is_compatible(const file_layout_t& layout, uint64_t size) {
   uint64_t object_count = Striper::get_num_objects(layout, size);
   return (object_count <= cls::rbd::MAX_OBJECT_MAP_OBJECT_COUNT);
 }
 
-ceph::BitVector<2u>::Reference ObjectMap::operator[](uint64_t object_no)
+template <typename I>
+ceph::BitVector<2u>::Reference ObjectMap<I>::operator[](uint64_t object_no)
 {
   assert(m_image_ctx.object_map_lock.is_wlocked());
   assert(object_no < m_object_map.size());
   return m_object_map[object_no];
 }
 
-uint8_t ObjectMap::operator[](uint64_t object_no) const
+template <typename I>
+uint8_t ObjectMap<I>::operator[](uint64_t object_no) const
 {
   assert(m_image_ctx.object_map_lock.is_locked());
   assert(object_no < m_object_map.size());
   return m_object_map[object_no];
 }
 
-bool ObjectMap::object_may_exist(uint64_t object_no) const
+template <typename I>
+bool ObjectMap<I>::object_may_exist(uint64_t object_no) const
 {
   assert(m_image_ctx.snap_lock.is_locked());
 
@@ -91,7 +98,8 @@ bool ObjectMap::object_may_exist(uint64_t object_no) const
   return exists;
 }
 
-bool ObjectMap::update_required(uint64_t object_no, uint8_t new_state) {
+template <typename I>
+bool ObjectMap<I>::update_required(uint64_t object_no, uint8_t new_state) {
   assert(m_image_ctx.object_map_lock.is_wlocked());
   uint8_t state = (*this)[object_no];
 
@@ -103,24 +111,26 @@ bool ObjectMap::update_required(uint64_t object_no, uint8_t new_state) {
   return true;
 }
 
-void ObjectMap::open(Context *on_finish) {
-  object_map::RefreshRequest<> *req = new object_map::RefreshRequest<>(
+template <typename I>
+void ObjectMap<I>::open(Context *on_finish) {
+  auto req = object_map::RefreshRequest<I>::create(
     m_image_ctx, &m_object_map, m_snap_id, on_finish);
   req->send();
 }
 
-void ObjectMap::close(Context *on_finish) {
+template <typename I>
+void ObjectMap<I>::close(Context *on_finish) {
   if (m_snap_id != CEPH_NOSNAP) {
     m_image_ctx.op_work_queue->queue(on_finish, 0);
     return;
   }
 
-  object_map::UnlockRequest<> *req = new object_map::UnlockRequest<>(
-    m_image_ctx, on_finish);
+  auto req = object_map::UnlockRequest<I>::create(m_image_ctx, on_finish);
   req->send();
 }
 
-void ObjectMap::rollback(uint64_t snap_id, Context *on_finish) {
+template <typename I>
+void ObjectMap<I>::rollback(uint64_t snap_id, Context *on_finish) {
   assert(m_image_ctx.snap_lock.is_locked());
   assert(m_image_ctx.object_map_lock.is_wlocked());
 
@@ -129,7 +139,8 @@ void ObjectMap::rollback(uint64_t snap_id, Context *on_finish) {
   req->send();
 }
 
-void ObjectMap::snapshot_add(uint64_t snap_id, Context *on_finish) {
+template <typename I>
+void ObjectMap<I>::snapshot_add(uint64_t snap_id, Context *on_finish) {
   assert(m_image_ctx.snap_lock.is_locked());
   assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0);
   assert(snap_id != CEPH_NOSNAP);
@@ -140,7 +151,8 @@ void ObjectMap::snapshot_add(uint64_t snap_id, Context *on_finish) {
   req->send();
 }
 
-void ObjectMap::snapshot_remove(uint64_t snap_id, Context *on_finish) {
+template <typename I>
+void ObjectMap<I>::snapshot_remove(uint64_t snap_id, Context *on_finish) {
   assert(m_image_ctx.snap_lock.is_wlocked());
   assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0);
   assert(snap_id != CEPH_NOSNAP);
@@ -151,7 +163,8 @@ void ObjectMap::snapshot_remove(uint64_t snap_id, Context *on_finish) {
   req->send();
 }
 
-void ObjectMap::aio_save(Context *on_finish) {
+template <typename I>
+void ObjectMap<I>::aio_save(Context *on_finish) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.snap_lock.is_locked());
   assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP,
@@ -172,8 +185,9 @@ void ObjectMap::aio_save(Context *on_finish) {
   comp->release();
 }
 
-void ObjectMap::aio_resize(uint64_t new_size, uint8_t default_object_state,
-			   Context *on_finish) {
+template <typename I>
+void ObjectMap<I>::aio_resize(uint64_t new_size, uint8_t default_object_state,
+			      Context *on_finish) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_image_ctx.snap_lock.is_locked());
   assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP,
@@ -188,10 +202,11 @@ void ObjectMap::aio_resize(uint64_t new_size, uint8_t default_object_state,
   req->send();
 }
 
-void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no,
-                           uint64_t end_object_no, uint8_t new_state,
-                           const boost::optional<uint8_t> &current_state,
-                           Context *on_finish) {
+template <typename I>
+void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
+                              uint64_t end_object_no, uint8_t new_state,
+                              const boost::optional<uint8_t> &current_state,
+                              Context *on_finish) {
   assert(m_image_ctx.snap_lock.is_locked());
   assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0);
   assert(snap_id != CEPH_NOSNAP || m_image_ctx.owner_lock.is_locked());
@@ -227,10 +242,13 @@ void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no,
     }
   }
 
-  object_map::UpdateRequest *req = new object_map::UpdateRequest(
+  auto req = object_map::UpdateRequest<I>::create(
     m_image_ctx, &m_object_map, snap_id, start_object_no, end_object_no,
     new_state, current_state, on_finish);
   req->send();
 }
 
 } // namespace librbd
+
+template class librbd::ObjectMap<librbd::ImageCtx>;
+
diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h
index 44ddaf0548a8b..3ac857322f17e 100644
--- a/src/librbd/ObjectMap.h
+++ b/src/librbd/ObjectMap.h
@@ -21,9 +21,14 @@ namespace librbd {
 
 class ImageCtx;
 
+template <typename ImageCtxT = ImageCtx>
 class ObjectMap {
 public:
-  ObjectMap(ImageCtx &image_ctx, uint64_t snap_id);
+  static ObjectMap *create(ImageCtxT &image_ctx, uint64_t snap_id) {
+    return new ObjectMap(image_ctx, snap_id);
+  }
+
+  ObjectMap(ImageCtxT &image_ctx, uint64_t snap_id);
 
   static int remove(librados::IoCtx &io_ctx, const std::string &image_id);
   static std::string object_map_name(const std::string &image_id,
@@ -85,7 +90,7 @@ class ObjectMap {
   void snapshot_remove(uint64_t snap_id, Context *on_finish);
 
 private:
-  ImageCtx &m_image_ctx;
+  ImageCtxT &m_image_ctx;
   ceph::BitVector<2> m_object_map;
   uint64_t m_snap_id;
 
@@ -99,4 +104,6 @@ class ObjectMap {
 
 } // namespace librbd
 
+extern template class librbd::ObjectMap<librbd::ImageCtx>;
+
 #endif // CEPH_LIBRBD_OBJECT_MAP_H
diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc
index 31a503f4d4f16..030ed6d70b2b6 100644
--- a/src/librbd/Operations.cc
+++ b/src/librbd/Operations.cc
@@ -544,7 +544,7 @@ int Operations<I>::resize(uint64_t size, ProgressContext& prog_ctx) {
   }
 
   if (m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP) &&
-      !ObjectMap::is_compatible(m_image_ctx.layout, size)) {
+      !ObjectMap<>::is_compatible(m_image_ctx.layout, size)) {
     lderr(cct) << "New size not compatible with object map" << dendl;
     return -EINVAL;
   }
@@ -582,7 +582,7 @@ void Operations<I>::execute_resize(uint64_t size, ProgressContext &prog_ctx,
     return;
   } else if (m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP,
                                        m_image_ctx.snap_lock) &&
-             !ObjectMap::is_compatible(m_image_ctx.layout, size)) {
+             !ObjectMap<>::is_compatible(m_image_ctx.layout, size)) {
     m_image_ctx.snap_lock.put_read();
     on_finish->complete(-EINVAL);
     return;
diff --git a/src/librbd/image/SetSnapRequest.cc b/src/librbd/image/SetSnapRequest.cc
index 50d485ce54e72..def86f99a5d8e 100644
--- a/src/librbd/image/SetSnapRequest.cc
+++ b/src/librbd/image/SetSnapRequest.cc
@@ -262,7 +262,7 @@ Context *SetSnapRequest<I>::send_open_object_map(int *result) {
   using klass = SetSnapRequest<I>;
   Context *ctx = create_context_callback<
     klass, &klass::handle_open_object_map>(this);
-  m_object_map = new ObjectMap(m_image_ctx, m_snap_id);
+  m_object_map = ObjectMap<I>::create(m_image_ctx, m_snap_id);
   m_object_map->open(ctx);
   return nullptr;
 }
diff --git a/src/librbd/image/SetSnapRequest.h b/src/librbd/image/SetSnapRequest.h
index 1e2df49bcf14b..b64fa68afa0c1 100644
--- a/src/librbd/image/SetSnapRequest.h
+++ b/src/librbd/image/SetSnapRequest.h
@@ -12,7 +12,7 @@ namespace librbd {
 
 template <typename> class ExclusiveLock;
 class ImageCtx;
-class ObjectMap;
+template <typename> class ObjectMap;
 
 namespace image {
 
@@ -84,7 +84,7 @@ class SetSnapRequest {
 
   uint64_t m_snap_id;
   ExclusiveLock<ImageCtxT> *m_exclusive_lock;
-  ObjectMap *m_object_map;
+  ObjectMap<ImageCtxT> *m_object_map;
   RefreshParentRequest<ImageCtxT> *m_refresh_parent;
 
   bool m_writes_blocked;
diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 68d195be5e2f7..f02c186b19c70 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -76,7 +76,7 @@ int remove_object_map(ImageCtx *ictx) {
   int r;
   for (std::map<snap_t, SnapInfo>::iterator it = ictx->snap_info.begin();
        it != ictx->snap_info.end(); ++it) {
-    std::string oid(ObjectMap::object_map_name(ictx->id, it->first));
+    std::string oid(ObjectMap<>::object_map_name(ictx->id, it->first));
     r = ictx->md_ctx.remove(oid);
     if (r < 0 && r != -ENOENT) {
       lderr(cct) << "failed to remove object map " << oid << ": "
@@ -85,7 +85,7 @@ int remove_object_map(ImageCtx *ictx) {
     }
   }
 
-  r = ictx->md_ctx.remove(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP));
+  r = ictx->md_ctx.remove(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP));
   if (r < 0 && r != -ENOENT) {
     lderr(cct) << "failed to remove object map: " << cpp_strerror(r) << dendl;
     return r;
@@ -107,7 +107,7 @@ int create_object_map(ImageCtx *ictx) {
     snap_ids.push_back(it->first);
   }
 
-  if (!ObjectMap::is_compatible(ictx->layout, max_size)) {
+  if (!ObjectMap<>::is_compatible(ictx->layout, max_size)) {
     lderr(cct) << "image size not compatible with object map" << dendl;
     return -EINVAL;
   }
@@ -115,7 +115,7 @@ int create_object_map(ImageCtx *ictx) {
   for (std::vector<uint64_t>::iterator it = snap_ids.begin();
     it != snap_ids.end(); ++it) {
     librados::ObjectWriteOperation op;
-    std::string oid(ObjectMap::object_map_name(ictx->id, *it));
+    std::string oid(ObjectMap<>::object_map_name(ictx->id, *it));
     uint64_t snap_size = ictx->get_image_size(*it);
     cls_client::object_map_resize(&op, Striper::get_num_objects(ictx->layout, snap_size),
                                   OBJECT_NONEXISTENT);
@@ -1139,7 +1139,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
         layout.stripe_count = stripe_count;
       }
 
-      if (!ObjectMap::is_compatible(layout, size)) {
+      if (!ObjectMap<>::is_compatible(layout, size)) {
         lderr(cct) << "image size not compatible with object map" << dendl;
         goto err_remove_header;
       }
@@ -1147,7 +1147,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       librados::ObjectWriteOperation op;
       cls_client::object_map_resize(&op, Striper::get_num_objects(layout, size),
                                     OBJECT_NONEXISTENT);
-      r = io_ctx.operate(ObjectMap::object_map_name(id, CEPH_NOSNAP), &op);
+      r = io_ctx.operate(ObjectMap<>::object_map_name(id, CEPH_NOSNAP), &op);
       if (r < 0) {
         lderr(cct) << "error creating initial object map: "
                    << cpp_strerror(r) << dendl;
@@ -1204,7 +1204,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
 
   err_remove_object_map:
     if ((features & RBD_FEATURE_OBJECT_MAP) != 0) {
-      remove_r = ObjectMap::remove(io_ctx, id);
+      remove_r = ObjectMap<>::remove(io_ctx, id);
       if (remove_r < 0) {
         lderr(cct) << "error cleaning up object map after creation failed: "
                    << cpp_strerror(remove_r) << dendl;
@@ -2247,7 +2247,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
         }
 
         ldout(cct, 10) << "removing object map..." << dendl;
-        r = ObjectMap::remove(io_ctx, id);
+        r = ObjectMap<>::remove(io_ctx, id);
         if (r < 0 && r != -ENOENT) {
           lderr(cct) << "error removing image object map" << dendl;
           return r;
diff --git a/src/librbd/object_map/LockRequest.cc b/src/librbd/object_map/LockRequest.cc
index 3af50735e431b..213799aabc5ca 100644
--- a/src/librbd/object_map/LockRequest.cc
+++ b/src/librbd/object_map/LockRequest.cc
@@ -32,7 +32,7 @@ void LockRequest<I>::send() {
 template <typename I>
 void LockRequest<I>::send_lock() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
   ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl;
 
   librados::ObjectWriteOperation op;
@@ -68,7 +68,7 @@ Context *LockRequest<I>::handle_lock(int *ret_val) {
 template <typename I>
 void LockRequest<I>::send_get_lock_info() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
   ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl;
 
   librados::ObjectReadOperation op;
@@ -113,7 +113,7 @@ Context *LockRequest<I>::handle_get_lock_info(int *ret_val) {
 template <typename I>
 void LockRequest<I>::send_break_locks() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
   ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << ", "
                  << "num_lockers=" << m_lockers.size() << dendl;
 
diff --git a/src/librbd/object_map/RefreshRequest.cc b/src/librbd/object_map/RefreshRequest.cc
index 9421c1216c4b1..c7b4834d2f1a9 100644
--- a/src/librbd/object_map/RefreshRequest.cc
+++ b/src/librbd/object_map/RefreshRequest.cc
@@ -72,7 +72,7 @@ void RefreshRequest<I>::send_lock() {
     return;
   }
 
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl;
 
   using klass = RefreshRequest<I>;
@@ -96,7 +96,7 @@ Context *RefreshRequest<I>::handle_lock(int *ret_val) {
 template <typename I>
 void RefreshRequest<I>::send_load() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl;
 
   librados::ObjectReadOperation op;
@@ -122,7 +122,7 @@ Context *RefreshRequest<I>::handle_load(int *ret_val) {
                                                   &m_on_disk_object_map);
   }
 
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   if (*ret_val == -EINVAL) {
      // object map is corrupt on-disk -- clear it and properly size it
      // so future IO can keep the object map in sync
@@ -220,7 +220,7 @@ Context *RefreshRequest<I>::handle_resize_invalidate(int *ret_val) {
 template <typename I>
 void RefreshRequest<I>::send_resize() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl;
 
   librados::ObjectWriteOperation op;
diff --git a/src/librbd/object_map/RefreshRequest.h b/src/librbd/object_map/RefreshRequest.h
index 4c2b059fa268f..24a7998ae1bc7 100644
--- a/src/librbd/object_map/RefreshRequest.h
+++ b/src/librbd/object_map/RefreshRequest.h
@@ -19,6 +19,12 @@ namespace object_map {
 template <typename ImageCtxT = ImageCtx>
 class RefreshRequest {
 public:
+  static RefreshRequest *create(ImageCtxT &image_ctx,
+                                ceph::BitVector<2> *object_map,
+                                uint64_t snap_id, Context *on_finish) {
+    return new RefreshRequest(image_ctx, object_map, snap_id, on_finish);
+  }
+
   RefreshRequest(ImageCtxT &image_ctx, ceph::BitVector<2> *object_map,
                  uint64_t snap_id, Context *on_finish);
 
diff --git a/src/librbd/object_map/ResizeRequest.cc b/src/librbd/object_map/ResizeRequest.cc
index 5698fc80f7bf2..3182c8aef91ca 100644
--- a/src/librbd/object_map/ResizeRequest.cc
+++ b/src/librbd/object_map/ResizeRequest.cc
@@ -30,7 +30,7 @@ void ResizeRequest::send() {
   RWLock::WLocker l(m_image_ctx.object_map_lock);
   m_num_objs = Striper::get_num_objects(m_image_ctx.layout, m_new_size);
 
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 5) << this << " resizing on-disk object map: "
                 << "ictx=" << &m_image_ctx << ", "
                 << "oid=" << oid << ", num_objs=" << m_num_objs << dendl;
diff --git a/src/librbd/object_map/SnapshotCreateRequest.cc b/src/librbd/object_map/SnapshotCreateRequest.cc
index 6408973416fa0..5d77d2630d385 100644
--- a/src/librbd/object_map/SnapshotCreateRequest.cc
+++ b/src/librbd/object_map/SnapshotCreateRequest.cc
@@ -79,7 +79,7 @@ void SnapshotCreateRequest::send_read_map() {
   assert(m_image_ctx.get_snap_info(m_snap_id) != NULL);
 
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
   ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl;
   m_state = STATE_READ_MAP;
 
@@ -96,7 +96,7 @@ void SnapshotCreateRequest::send_read_map() {
 
 void SnapshotCreateRequest::send_write_map() {
   CephContext *cct = m_image_ctx.cct;
-  std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid
                 << dendl;
   m_state = STATE_WRITE_MAP;
@@ -117,7 +117,7 @@ bool SnapshotCreateRequest::send_add_snapshot() {
   }
 
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
   ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl;
   m_state = STATE_ADD_SNAPSHOT;
 
diff --git a/src/librbd/object_map/SnapshotRemoveRequest.cc b/src/librbd/object_map/SnapshotRemoveRequest.cc
index 94c0952504a14..821dd5b8781bb 100644
--- a/src/librbd/object_map/SnapshotRemoveRequest.cc
+++ b/src/librbd/object_map/SnapshotRemoveRequest.cc
@@ -112,7 +112,7 @@ bool SnapshotRemoveRequest::should_complete(int r) {
 
 void SnapshotRemoveRequest::send_load_map() {
   CephContext *cct = m_image_ctx.cct;
-  std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid
                 << dendl;
   m_state = STATE_LOAD_MAP;
@@ -129,7 +129,7 @@ void SnapshotRemoveRequest::send_load_map() {
 
 void SnapshotRemoveRequest::send_remove_snapshot() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_next_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_next_snap_id));
   ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl;
   m_state = STATE_REMOVE_SNAPSHOT;
 
@@ -161,7 +161,7 @@ void SnapshotRemoveRequest::send_invalidate_next_map() {
 
 void SnapshotRemoveRequest::send_remove_map() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 5) << this << " " << __func__ << ": oid=" << oid << dendl;
   m_state = STATE_REMOVE_MAP;
 
diff --git a/src/librbd/object_map/SnapshotRollbackRequest.cc b/src/librbd/object_map/SnapshotRollbackRequest.cc
index 10eb59136d381..0da4fd4ae8e3b 100644
--- a/src/librbd/object_map/SnapshotRollbackRequest.cc
+++ b/src/librbd/object_map/SnapshotRollbackRequest.cc
@@ -76,7 +76,7 @@ bool SnapshotRollbackRequest::should_complete(int r) {
 }
 
 void SnapshotRollbackRequest::send_read_map() {
-  std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
 
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid
@@ -97,7 +97,8 @@ void SnapshotRollbackRequest::send_write_map() {
   RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
 
   CephContext *cct = m_image_ctx.cct;
-  std::string snap_oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
+  std::string snap_oid(ObjectMap<>::object_map_name(m_image_ctx.id,
+                                                    CEPH_NOSNAP));
   ldout(cct, 5) << this << " " << __func__ << ": snap_oid=" << snap_oid
                 << dendl;
   m_state = STATE_WRITE_MAP;
diff --git a/src/librbd/object_map/UnlockRequest.cc b/src/librbd/object_map/UnlockRequest.cc
index c7ae9801d3ce0..3458bbed17f23 100644
--- a/src/librbd/object_map/UnlockRequest.cc
+++ b/src/librbd/object_map/UnlockRequest.cc
@@ -31,7 +31,7 @@ void UnlockRequest<I>::send() {
 template <typename I>
 void UnlockRequest<I>::send_unlock() {
   CephContext *cct = m_image_ctx.cct;
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
   ldout(cct, 10) << this << " " << __func__ << ": oid=" << oid << dendl;
 
   librados::ObjectWriteOperation op;
diff --git a/src/librbd/object_map/UnlockRequest.h b/src/librbd/object_map/UnlockRequest.h
index b52a3d0f89e97..ae1d9e93425de 100644
--- a/src/librbd/object_map/UnlockRequest.h
+++ b/src/librbd/object_map/UnlockRequest.h
@@ -15,6 +15,10 @@ namespace object_map {
 template <typename ImageCtxT = ImageCtx>
 class UnlockRequest {
 public:
+  static UnlockRequest *create(ImageCtxT &image_ctx, Context *on_finish) {
+    return new UnlockRequest(image_ctx, on_finish);
+  }
+
   UnlockRequest(ImageCtxT &image_ctx, Context *on_finish);
 
   void send();
diff --git a/src/librbd/object_map/UpdateRequest.cc b/src/librbd/object_map/UpdateRequest.cc
index 3edf32aa67fa7..e88085add4115 100644
--- a/src/librbd/object_map/UpdateRequest.cc
+++ b/src/librbd/object_map/UpdateRequest.cc
@@ -17,14 +17,15 @@
 namespace librbd {
 namespace object_map {
 
-void UpdateRequest::send() {
+template <typename I>
+void UpdateRequest<I>::send() {
   assert(m_image_ctx.snap_lock.is_locked());
   assert(m_image_ctx.object_map_lock.is_locked());
   CephContext *cct = m_image_ctx.cct;
 
   // safe to update in-memory state first without handling rollback since any
   // failures will invalidate the object map
-  std::string oid(ObjectMap::object_map_name(m_image_ctx.id, m_snap_id));
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, m_snap_id));
   ldout(cct, 20) << this << " updating object map"
                  << ": ictx=" << &m_image_ctx << ", oid=" << oid << ", ["
 		 << m_start_object_no << "," << m_end_object_no << ") = "
@@ -46,7 +47,8 @@ void UpdateRequest::send() {
   rados_completion->release();
 }
 
-void UpdateRequest::finish_request() {
+template <typename I>
+void UpdateRequest<I>::finish_request() {
   RWLock::RLocker snap_locker(m_image_ctx.snap_lock);
   RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
   ldout(m_image_ctx.cct, 20) << this << " on-disk object map updated"
@@ -68,3 +70,5 @@ void UpdateRequest::finish_request() {
 
 } // namespace object_map
 } // namespace librbd
+
+template class librbd::object_map::UpdateRequest<librbd::ImageCtx>;
diff --git a/src/librbd/object_map/UpdateRequest.h b/src/librbd/object_map/UpdateRequest.h
index d1ce4075dd0d4..5f3ae5cd53f51 100644
--- a/src/librbd/object_map/UpdateRequest.h
+++ b/src/librbd/object_map/UpdateRequest.h
@@ -17,13 +17,25 @@ class ImageCtx;
 
 namespace object_map {
 
+template <typename ImageCtxT = librbd::ImageCtx>
 class UpdateRequest : public Request {
 public:
+  static UpdateRequest *create(ImageCtx &image_ctx,
+                               ceph::BitVector<2> *object_map,
+                               uint64_t snap_id, uint64_t start_object_no,
+                               uint64_t end_object_no, uint8_t new_state,
+                               const boost::optional<uint8_t> &current_state,
+                               Context *on_finish) {
+    return new UpdateRequest(image_ctx, object_map, snap_id, start_object_no,
+                             end_object_no, new_state, current_state,
+                             on_finish);
+  }
+
   UpdateRequest(ImageCtx &image_ctx, ceph::BitVector<2> *object_map,
                 uint64_t snap_id, uint64_t start_object_no,
                 uint64_t end_object_no, uint8_t new_state,
                 const boost::optional<uint8_t> &current_state,
-      	  Context *on_finish)
+      	        Context *on_finish)
     : Request(image_ctx, snap_id, on_finish), m_object_map(*object_map),
       m_start_object_no(start_object_no), m_end_object_no(end_object_no),
       m_new_state(new_state), m_current_state(current_state)
@@ -46,4 +58,6 @@ class UpdateRequest : public Request {
 } // namespace object_map
 } // namespace librbd
 
+extern template class librbd::object_map::UpdateRequest<librbd::ImageCtx>;
+
 #endif // CEPH_LIBRBD_OBJECT_MAP_UPDATE_REQUEST_H
diff --git a/src/test/librbd/object_map/test_mock_LockRequest.cc b/src/test/librbd/object_map/test_mock_LockRequest.cc
index 0ee782bce1337..819b0125c897f 100644
--- a/src/test/librbd/object_map/test_mock_LockRequest.cc
+++ b/src/test/librbd/object_map/test_mock_LockRequest.cc
@@ -27,14 +27,16 @@ class TestMockObjectMapLockRequest : public TestMockFixture {
   typedef LockRequest<MockImageCtx> MockLockRequest;
 
   void expect_lock(MockImageCtx &mock_image_ctx, int r) {
-    std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP));
+    std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id,
+                                                 CEPH_NOSNAP));
     EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                 exec(oid, _, StrEq("lock"), StrEq("lock"), _, _, _))
                   .WillOnce(Return(r));
   }
 
   void expect_get_lock_info(MockImageCtx &mock_image_ctx, int r) {
-    std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP));
+    std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id,
+                                                 CEPH_NOSNAP));
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(oid, _, StrEq("lock"), StrEq("get_info"), _, _, _));
     if (r < 0) {
@@ -59,7 +61,8 @@ class TestMockObjectMapLockRequest : public TestMockFixture {
   }
 
   void expect_break_lock(MockImageCtx &mock_image_ctx, int r) {
-    std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP));
+    std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id,
+                                                 CEPH_NOSNAP));
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(oid, _, StrEq("lock"), StrEq("break_lock"), _, _, _));
     if (r < 0) {
diff --git a/src/test/librbd/object_map/test_mock_RefreshRequest.cc b/src/test/librbd/object_map/test_mock_RefreshRequest.cc
index be982ce684e89..8d67a1a4f5d4a 100644
--- a/src/test/librbd/object_map/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/object_map/test_mock_RefreshRequest.cc
@@ -86,7 +86,7 @@ class TestMockObjectMapRefreshRequest : public TestMockFixture {
   void expect_object_map_load(MockObjectMapImageCtx &mock_image_ctx,
                               ceph::BitVector<2> *object_map, uint64_t snap_id,
                               int r) {
-    std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, snap_id));
+    std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id, snap_id));
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(oid, _, StrEq("rbd"), StrEq("object_map_load"), _, _, _));
     if (r < 0) {
@@ -117,14 +117,16 @@ class TestMockObjectMapRefreshRequest : public TestMockFixture {
   }
 
   void expect_truncate_request(MockObjectMapImageCtx &mock_image_ctx) {
-    std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, TEST_SNAP_ID));
+    std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id,
+                                                 TEST_SNAP_ID));
     EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), truncate(oid, 0, _))
                   .WillOnce(Return(0));
   }
 
   void expect_object_map_resize(MockObjectMapImageCtx &mock_image_ctx,
                                 uint64_t num_objects, int r) {
-    std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, TEST_SNAP_ID));
+    std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id,
+                                                 TEST_SNAP_ID));
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(oid, _, StrEq("rbd"), StrEq("object_map_resize"), _, _, _));
     expect.WillOnce(Return(r));
diff --git a/src/test/librbd/object_map/test_mock_ResizeRequest.cc b/src/test/librbd/object_map/test_mock_ResizeRequest.cc
index 42007d34487a4..44fd714353ba1 100644
--- a/src/test/librbd/object_map/test_mock_ResizeRequest.cc
+++ b/src/test/librbd/object_map/test_mock_ResizeRequest.cc
@@ -22,7 +22,7 @@ using ::testing::StrEq;
 class TestMockObjectMapResizeRequest : public TestMockFixture {
 public:
   void expect_resize(librbd::ImageCtx *ictx, uint64_t snap_id, int r) {
-    std::string oid(ObjectMap::object_map_name(ictx->id, snap_id));
+    std::string oid(ObjectMap<>::object_map_name(ictx->id, snap_id));
     if (snap_id == CEPH_NOSNAP) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
diff --git a/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc
index 3dd62989f2970..823a32de9fe16 100644
--- a/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotCreateRequest.cc
@@ -31,11 +31,11 @@ class TestMockObjectMapSnapshotCreateRequest : public TestMockFixture {
   void expect_read_map(librbd::ImageCtx *ictx, int r) {
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                  read(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP),
+                  read(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP),
                        0, 0, _)).WillOnce(Return(r));
     } else {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                  read(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP),
+                  read(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP),
                        0, 0, _)).WillOnce(DoDefault());
     }
   }
@@ -44,18 +44,18 @@ class TestMockObjectMapSnapshotCreateRequest : public TestMockFixture {
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   write_full(
-                    ObjectMap::object_map_name(ictx->id, snap_id), _, _))
+                    ObjectMap<>::object_map_name(ictx->id, snap_id), _, _))
                   .WillOnce(Return(r));
     } else {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   write_full(
-                    ObjectMap::object_map_name(ictx->id, snap_id), _, _))
+                    ObjectMap<>::object_map_name(ictx->id, snap_id), _, _))
                   .WillOnce(DoDefault());
     }
   }
 
   void expect_add_snapshot(librbd::ImageCtx *ictx, int r) {
-    std::string oid(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP));
+    std::string oid(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP));
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
diff --git a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
index 215c214eb8b10..270b267c53fed 100644
--- a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
@@ -23,7 +23,7 @@ using ::testing::StrEq;
 class TestMockObjectMapSnapshotRemoveRequest : public TestMockFixture {
 public:
   void expect_load_map(librbd::ImageCtx *ictx, uint64_t snap_id, int r) {
-    std::string snap_oid(ObjectMap::object_map_name(ictx->id, snap_id));
+    std::string snap_oid(ObjectMap<>::object_map_name(ictx->id, snap_id));
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   exec(snap_oid, _, StrEq("rbd"), StrEq("object_map_load"), _, _, _))
@@ -36,7 +36,7 @@ class TestMockObjectMapSnapshotRemoveRequest : public TestMockFixture {
   }
 
   void expect_remove_snapshot(librbd::ImageCtx *ictx, int r) {
-    std::string oid(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP));
+    std::string oid(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP));
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
@@ -52,7 +52,7 @@ class TestMockObjectMapSnapshotRemoveRequest : public TestMockFixture {
   }
 
   void expect_remove_map(librbd::ImageCtx *ictx, uint64_t snap_id, int r) {
-    std::string snap_oid(ObjectMap::object_map_name(ictx->id, snap_id));
+    std::string snap_oid(ObjectMap<>::object_map_name(ictx->id, snap_id));
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx), remove(snap_oid, _))
                     .WillOnce(Return(r));
diff --git a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc
index 51fc932c0b07a..ff3e06a8d70c5 100644
--- a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc
@@ -24,29 +24,29 @@ class TestMockObjectMapSnapshotRollbackRequest : public TestMockFixture {
   void expect_read_map(librbd::ImageCtx *ictx, uint64_t snap_id, int r) {
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                  read(ObjectMap::object_map_name(ictx->id, snap_id),
+                  read(ObjectMap<>::object_map_name(ictx->id, snap_id),
                        0, 0, _)).WillOnce(Return(r));
     } else {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                  read(ObjectMap::object_map_name(ictx->id, snap_id),
+                  read(ObjectMap<>::object_map_name(ictx->id, snap_id),
                        0, 0, _)).WillOnce(DoDefault());
     }
   }
 
   void expect_write_map(librbd::ImageCtx *ictx, int r) {
     EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                exec(ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), _,
+                exec(ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), _,
 		     StrEq("lock"), StrEq("assert_locked"), _, _, _))
                   .WillOnce(DoDefault());
     if (r < 0) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   write_full(
-                    ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), _, _))
+                    ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), _, _))
                   .WillOnce(Return(r));
     } else {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   write_full(
-                    ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP), _, _))
+                    ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP), _, _))
                   .WillOnce(DoDefault());
     }
   }
diff --git a/src/test/librbd/object_map/test_mock_UnlockRequest.cc b/src/test/librbd/object_map/test_mock_UnlockRequest.cc
index 7f834ca8dba18..a834a5004fa31 100644
--- a/src/test/librbd/object_map/test_mock_UnlockRequest.cc
+++ b/src/test/librbd/object_map/test_mock_UnlockRequest.cc
@@ -25,7 +25,8 @@ class TestMockObjectMapUnlockRequest : public TestMockFixture {
   typedef UnlockRequest<MockImageCtx> MockUnlockRequest;
 
   void expect_unlock(MockImageCtx &mock_image_ctx, int r) {
-    std::string oid(ObjectMap::object_map_name(mock_image_ctx.id, CEPH_NOSNAP));
+    std::string oid(ObjectMap<>::object_map_name(mock_image_ctx.id,
+                                                 CEPH_NOSNAP));
     EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                 exec(oid, _, StrEq("lock"), StrEq("unlock"), _, _, _))
                   .WillOnce(Return(r));
diff --git a/src/test/librbd/object_map/test_mock_UpdateRequest.cc b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
index 67ace6249116f..f82ffb82ac21d 100644
--- a/src/test/librbd/object_map/test_mock_UpdateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
@@ -24,7 +24,7 @@ using ::testing::StrEq;
 class TestMockObjectMapUpdateRequest : public TestMockFixture {
 public:
   void expect_update(librbd::ImageCtx *ictx, uint64_t snap_id, int r) {
-    std::string oid(ObjectMap::object_map_name(ictx->id, snap_id));
+    std::string oid(ObjectMap<>::object_map_name(ictx->id, snap_id));
     if (snap_id == CEPH_NOSNAP) {
       EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                   exec(oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
@@ -69,7 +69,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateInMemory) {
   }
 
   C_SaferCond cond_ctx;
-  AsyncRequest<> *req = new UpdateRequest(
+  AsyncRequest<> *req = new UpdateRequest<>(
     *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
     OBJECT_EXISTS, &cond_ctx);
   {
@@ -101,7 +101,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateHeadOnDisk) {
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
-  AsyncRequest<> *req = new UpdateRequest(
+  AsyncRequest<> *req = new UpdateRequest<>(
     *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
     OBJECT_EXISTS, &cond_ctx);
   {
@@ -129,7 +129,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateSnapOnDisk) {
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
-  AsyncRequest<> *req = new UpdateRequest(
+  AsyncRequest<> *req = new UpdateRequest<>(
     *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_NONEXISTENT,
     OBJECT_EXISTS, &cond_ctx);
   {
@@ -156,7 +156,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateOnDiskError) {
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
-  AsyncRequest<> *req = new UpdateRequest(
+  AsyncRequest<> *req = new UpdateRequest<>(
     *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT,
     OBJECT_EXISTS, &cond_ctx);
   {
@@ -186,7 +186,7 @@ TEST_F(TestMockObjectMapUpdateRequest, RebuildSnapOnDisk) {
   object_map.resize(1);
 
   C_SaferCond cond_ctx;
-  AsyncRequest<> *req = new UpdateRequest(
+  AsyncRequest<> *req = new UpdateRequest<>(
     *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_EXISTS_CLEAN,
     boost::optional<uint8_t>(), &cond_ctx);
   {
diff --git a/src/test/librbd/test_ObjectMap.cc b/src/test/librbd/test_ObjectMap.cc
index b3b19e45ccd30..1e8566e9c5fe5 100644
--- a/src/test/librbd/test_ObjectMap.cc
+++ b/src/test/librbd/test_ObjectMap.cc
@@ -18,7 +18,7 @@ class TestObjectMap : public TestFixture {
 
   int when_open_object_map(librbd::ImageCtx *ictx) {
     C_SaferCond ctx;
-    librbd::ObjectMap object_map(*ictx, ictx->snap_id);
+    librbd::ObjectMap<> object_map(*ictx, ictx->snap_id);
     object_map.open(&ctx);
     return ctx.wait();
   }
@@ -38,7 +38,7 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenCorrupt) {
   }
   ASSERT_EQ(0, lock_ctx.wait());
 
-  std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP);
+  std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP);
   bufferlist bl;
   bl.append("corrupt");
   ASSERT_EQ(0, ictx->data_ctx.write_full(oid, bl));
@@ -64,8 +64,8 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenTooSmall) {
   librados::ObjectWriteOperation op;
   librbd::cls_client::object_map_resize(&op, 0, OBJECT_NONEXISTENT);
 
-  std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP);
-  ASSERT_EQ(0, ictx->data_ctx.operate(oid, &op));
+  std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP);
+  ASSERT_EQ(0, ictx->md_ctx.operate(oid, &op));
 
   ASSERT_EQ(0, when_open_object_map(ictx));
   ASSERT_TRUE(ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID));
@@ -85,7 +85,7 @@ TEST_F(TestObjectMap, InvalidateFlagOnDisk) {
   }
   ASSERT_EQ(0, lock_ctx.wait());
 
-  std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP);
+  std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP);
   bufferlist bl;
   bl.append("corrupt");
   ASSERT_EQ(0, ictx->data_ctx.write_full(oid, bl));
@@ -104,7 +104,7 @@ TEST_F(TestObjectMap, InvalidateFlagInMemoryOnly) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_FALSE(ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID));
 
-  std::string oid = librbd::ObjectMap::object_map_name(ictx->id, CEPH_NOSNAP);
+  std::string oid = librbd::ObjectMap<>::object_map_name(ictx->id, CEPH_NOSNAP);
   bufferlist valid_bl;
   ASSERT_LT(0, ictx->data_ctx.read(oid, valid_bl, 0, 0));
 
diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index c6ac977d73250..f79dbc12270ef 100644
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -558,7 +558,7 @@ TEST_F(TestInternal, SnapshotCopyup)
         state = OBJECT_EXISTS_CLEAN;
       }
 
-      librbd::ObjectMap object_map(*ictx2, ictx2->snap_id);
+      librbd::ObjectMap<> object_map(*ictx2, ictx2->snap_id);
       C_SaferCond ctx;
       object_map.open(&ctx);
       ASSERT_EQ(0, ctx.wait());
diff --git a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc
index c0105aeaa210b..c54aa2c223b53 100644
--- a/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc
+++ b/src/test/rbd_mirror/image_sync/test_mock_SnapshotCreateRequest.cc
@@ -91,8 +91,8 @@ class TestMockImageSyncSnapshotCreateRequest : public TestMockFixture {
 
   void expect_object_map_resize(librbd::MockTestImageCtx &mock_image_ctx,
                                 librados::snap_t snap_id, int r) {
-    std::string oid(librbd::ObjectMap::object_map_name(mock_image_ctx.id,
-                                                       snap_id));
+    std::string oid(librbd::ObjectMap<>::object_map_name(mock_image_ctx.id,
+                                                         snap_id));
     EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                 exec(oid, _, StrEq("rbd"), StrEq("object_map_resize"), _, _, _))
                   .WillOnce(Return(r));
diff --git a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc
index ee0c80fbad24b..ede82db4d0dd2 100644
--- a/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc
+++ b/src/tools/rbd_mirror/image_sync/SnapshotCreateRequest.cc
@@ -222,7 +222,7 @@ void SnapshotCreateRequest<I>::send_create_object_map() {
   librados::snap_t local_snap_id = snap_it->second;
   m_local_image_ctx->snap_lock.put_read();
 
-  std::string object_map_oid(librbd::ObjectMap::object_map_name(
+  std::string object_map_oid(librbd::ObjectMap<>::object_map_name(
     m_local_image_ctx->id, local_snap_id));
   uint64_t object_count = Striper::get_num_objects(m_local_image_ctx->layout,
                                                    m_size);

From 87557754f591607a1667fecfc8dabc7e52bbbca0 Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Wed, 7 Dec 2016 22:38:47 -0500
Subject: [PATCH 4/5] librbd: new block guard helper to prevent concurrent IO
 to blocks

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit b1d624b43ec159d4a07616e86557ea48f089b7a1)

Conflicts:
	src/librbd/BlockGuard.h: fixed compile issue
	src/librbd/Makefile.am: added BlockGuard
	src/test/Makefile-client.am: added BlockGuard test
	src/test/librbd/CMakeLists.txt: trivial resolution
---
 src/librbd/BlockGuard.h            | 172 +++++++++++++++++++++++++++++
 src/librbd/Makefile.am             |   1 +
 src/test/Makefile-client.am        |   1 +
 src/test/librbd/CMakeLists.txt     |   1 +
 src/test/librbd/test_BlockGuard.cc |  98 ++++++++++++++++
 5 files changed, 273 insertions(+)
 create mode 100644 src/librbd/BlockGuard.h
 create mode 100644 src/test/librbd/test_BlockGuard.cc

diff --git a/src/librbd/BlockGuard.h b/src/librbd/BlockGuard.h
new file mode 100644
index 0000000000000..36eb8be2eb18f
--- /dev/null
+++ b/src/librbd/BlockGuard.h
@@ -0,0 +1,172 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#ifndef CEPH_LIBRBD_IO_BLOCK_GUARD_H
+#define CEPH_LIBRBD_IO_BLOCK_GUARD_H
+
+#include "include/int_types.h"
+#include "common/dout.h"
+#include "common/Mutex.h"
+#include <boost/intrusive/list.hpp>
+#include <boost/intrusive/set.hpp>
+#include <deque>
+#include <list>
+#include "include/assert.h"
+
+#define dout_subsys ceph_subsys_rbd
+#undef dout_prefix
+#define dout_prefix *_dout << "librbd::BlockGuard: " << this << " " \
+                           <<  __func__ << ": "
+
+namespace librbd {
+
+struct BlockExtent {
+  uint64_t block_start = 0;
+  uint64_t block_end = 0;
+
+  BlockExtent() {
+  }
+  BlockExtent(uint64_t block_start, uint64_t block_end)
+    : block_start(block_start), block_end(block_end) {
+  }
+};
+
+struct BlockGuardCell {
+};
+
+/**
+ * Helper class to restrict and order concurrent IO to the same block. The
+ * definition of a block is dependent upon the user of this class. It might
+ * represent a backing object, 512 byte sectors, etc.
+ */
+template <typename BlockOperation>
+class BlockGuard {
+private:
+  struct DetainedBlockExtent;
+
+public:
+  typedef std::list<BlockOperation> BlockOperations;
+
+  BlockGuard(CephContext *cct)
+    : m_cct(cct), m_lock("librbd::BlockGuard::m_lock") {
+  }
+
+  BlockGuard(const BlockGuard&) = delete;
+  BlockGuard &operator=(const BlockGuard&) = delete;
+
+  /**
+   * Detain future IO for a range of blocks. the guard will assume
+   * ownership of the provided operation if the operation is blocked.
+   * @return 0 upon success and IO can be issued
+   *         >0 if the IO is blocked,
+   *         <0 upon error
+   */
+  int detain(const BlockExtent &block_extent, BlockOperation *block_operation,
+             BlockGuardCell **cell) {
+    Mutex::Locker locker(m_lock);
+    ldout(m_cct, 20) << "block_start=" << block_extent.block_start << ", "
+                     << "block_end=" << block_extent.block_end << ", "
+                     << "free_slots=" << m_free_detained_block_extents.size()
+                     << dendl;
+
+    DetainedBlockExtent *detained_block_extent;
+    auto it = m_detained_block_extents.find(block_extent);
+    if (it != m_detained_block_extents.end()) {
+      // request against an already detained block
+      detained_block_extent = &(*it);
+      if (block_operation != nullptr) {
+        detained_block_extent->block_operations.emplace_back(
+          std::move(*block_operation));
+      }
+
+      // alert the caller that the IO was detained
+      *cell = nullptr;
+      return detained_block_extent->block_operations.size();
+    } else {
+      if (!m_free_detained_block_extents.empty()) {
+        detained_block_extent = &m_free_detained_block_extents.front();
+        detained_block_extent->block_operations.clear();
+        m_free_detained_block_extents.pop_front();
+      } else {
+        ldout(m_cct, 20) << "no free detained block cells" << dendl;
+        m_detained_block_extent_pool.emplace_back();
+        detained_block_extent = &m_detained_block_extent_pool.back();
+      }
+
+      detained_block_extent->block_extent = block_extent;
+      m_detained_block_extents.insert(*detained_block_extent);
+      *cell = reinterpret_cast<BlockGuardCell*>(detained_block_extent);
+      return 0;
+    }
+  }
+
+  /**
+   * Release any detained IO operations from the provided cell.
+   */
+  void release(BlockGuardCell *cell, BlockOperations *block_operations) {
+    Mutex::Locker locker(m_lock);
+
+    assert(cell != nullptr);
+    auto &detained_block_extent = reinterpret_cast<DetainedBlockExtent &>(
+      *cell);
+    ldout(m_cct, 20) << "block_start="
+                     << detained_block_extent.block_extent.block_start << ", "
+                     << "block_end="
+                     << detained_block_extent.block_extent.block_end << ", "
+                     << "pending_ops="
+                     << (detained_block_extent.block_operations.empty() ?
+                          0 : detained_block_extent.block_operations.size() - 1)
+                     << dendl;
+
+    *block_operations = std::move(detained_block_extent.block_operations);
+    m_detained_block_extents.erase(detained_block_extent.block_extent);
+    m_free_detained_block_extents.push_back(detained_block_extent);
+  }
+
+private:
+  struct DetainedBlockExtent : public boost::intrusive::list_base_hook<>,
+                               public boost::intrusive::set_base_hook<> {
+    DetainedBlockExtent() {
+    }
+    DetainedBlockExtent(const BlockExtent &block_extent)
+      : block_extent(block_extent) {
+    }
+
+    BlockExtent block_extent;
+    BlockOperations block_operations;
+  };
+
+  struct DetainedBlockExtentCompare {
+    bool operator()(const DetainedBlockExtent &lhs,
+                    const DetainedBlockExtent &rhs) const {
+      // check for range overlap (lhs < rhs)
+      if (lhs.block_extent.block_end <= rhs.block_extent.block_start) {
+        return true;
+      }
+      return false;
+    }
+  };
+
+  typedef std::deque<DetainedBlockExtent> DetainedBlockExtentsPool;
+  typedef boost::intrusive::list<DetainedBlockExtent> DetainedBlockExtents;
+  typedef boost::intrusive::set<
+    DetainedBlockExtent,
+    boost::intrusive::compare<DetainedBlockExtentCompare> >
+      BlockExtentToDetainedBlockExtents;
+
+  CephContext *m_cct;
+
+  Mutex m_lock;
+  DetainedBlockExtentsPool m_detained_block_extent_pool;
+  DetainedBlockExtents m_free_detained_block_extents;
+  BlockExtentToDetainedBlockExtents m_detained_block_extents;
+
+};
+
+} // namespace librbd
+
+#undef dout_subsys
+#undef dout_prefix
+#define dout_prefix *_dout
+
+#endif // CEPH_LIBRBD_IO_BLOCK_GUARD_H
diff --git a/src/librbd/Makefile.am b/src/librbd/Makefile.am
index d6bc3581e3f55..8cc8fe47dfc88 100644
--- a/src/librbd/Makefile.am
+++ b/src/librbd/Makefile.am
@@ -97,6 +97,7 @@ noinst_HEADERS += \
 	librbd/AsyncObjectThrottle.h \
 	librbd/AsyncOperation.h \
 	librbd/AsyncRequest.h \
+        librbd/BlockGuard.h \
 	librbd/CopyupRequest.h \
 	librbd/DiffIterate.h \
 	librbd/ExclusiveLock.h \
diff --git a/src/test/Makefile-client.am b/src/test/Makefile-client.am
index 6eade231ff8d9..bf20a35b5f5f8 100644
--- a/src/test/Makefile-client.am
+++ b/src/test/Makefile-client.am
@@ -383,6 +383,7 @@ librbd_test_mock_la_CXXFLAGS = $(UNITTEST_CXXFLAGS)
 noinst_LTLIBRARIES += librbd_test_mock.la
 
 unittest_librbd_SOURCES = \
+        test/librbd/test_BlockGuard.cc \
         test/librbd/test_main.cc \
 	test/librbd/test_mock_fixture.cc \
 	test/librbd/test_mock_ExclusiveLock.cc \
diff --git a/src/test/librbd/CMakeLists.txt b/src/test/librbd/CMakeLists.txt
index aff27a628ca57..b2781979393fd 100644
--- a/src/test/librbd/CMakeLists.txt
+++ b/src/test/librbd/CMakeLists.txt
@@ -15,6 +15,7 @@ set_target_properties(rbd_test PROPERTIES COMPILE_FLAGS ${UNITTEST_CXX_FLAGS})
 # unittest_librbd
 # doesn't use add_ceph_test because it is called by run-rbd-unit-tests.sh
 set(unittest_librbd_srcs
+  test_BlockGuard.cc
   test_main.cc
   test_mock_fixture.cc
   test_mock_AioImageRequest.cc
diff --git a/src/test/librbd/test_BlockGuard.cc b/src/test/librbd/test_BlockGuard.cc
new file mode 100644
index 0000000000000..b16188b04a474
--- /dev/null
+++ b/src/test/librbd/test_BlockGuard.cc
@@ -0,0 +1,98 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#include "test/librbd/test_fixture.h"
+#include "test/librbd/test_support.h"
+#include "librbd/BlockGuard.h"
+
+namespace librbd {
+
+class TestIOBlockGuard : public TestFixture {
+public:
+  static uint32_t s_index;
+
+  struct Operation {
+    uint32_t index;
+    Operation() : index(++s_index) {
+    }
+    Operation(Operation &&rhs) : index(rhs.index) {
+    }
+    Operation(const Operation &) = delete;
+
+    Operation& operator=(Operation &&rhs) {
+      index = rhs.index;
+      return *this;
+    }
+
+    bool operator==(const Operation &rhs) const {
+      return index == rhs.index;
+    }
+  };
+
+  typedef std::list<Operation> Operations;
+
+  typedef BlockGuard<Operation> OpBlockGuard;
+
+  virtual void SetUp() override {
+    TestFixture::SetUp();
+    m_cct = reinterpret_cast<CephContext*>(m_ioctx.cct());
+  }
+
+  CephContext *m_cct;
+};
+
+TEST_F(TestIOBlockGuard, NonDetainedOps) {
+  OpBlockGuard op_block_guard(m_cct);
+
+  Operation op1;
+  BlockGuardCell *cell1;
+  ASSERT_EQ(0, op_block_guard.detain({1, 3}, &op1, &cell1));
+
+  Operation op2;
+  BlockGuardCell *cell2;
+  ASSERT_EQ(0, op_block_guard.detain({0, 1}, &op2, &cell2));
+
+  Operation op3;
+  BlockGuardCell *cell3;
+  ASSERT_EQ(0, op_block_guard.detain({3, 6}, &op3, &cell3));
+
+  Operations released_ops;
+  op_block_guard.release(cell1, &released_ops);
+  ASSERT_TRUE(released_ops.empty());
+
+  op_block_guard.release(cell2, &released_ops);
+  ASSERT_TRUE(released_ops.empty());
+
+  op_block_guard.release(cell3, &released_ops);
+  ASSERT_TRUE(released_ops.empty());
+}
+
+TEST_F(TestIOBlockGuard, DetainedOps) {
+  OpBlockGuard op_block_guard(m_cct);
+
+  Operation op1;
+  BlockGuardCell *cell1;
+  ASSERT_EQ(0, op_block_guard.detain({1, 3}, &op1, &cell1));
+
+  Operation op2;
+  BlockGuardCell *cell2;
+  ASSERT_EQ(1, op_block_guard.detain({2, 6}, &op2, &cell2));
+  ASSERT_EQ(nullptr, cell2);
+
+  Operation op3;
+  BlockGuardCell *cell3;
+  ASSERT_EQ(2, op_block_guard.detain({0, 2}, &op3, &cell3));
+  ASSERT_EQ(nullptr, cell3);
+
+  Operations expected_ops;
+  expected_ops.push_back(std::move(op2));
+  expected_ops.push_back(std::move(op3));
+  Operations released_ops;
+  op_block_guard.release(cell1, &released_ops);
+  ASSERT_EQ(expected_ops, released_ops);
+}
+
+uint32_t TestIOBlockGuard::s_index = 0;
+
+} // namespace librbd
+

From cdd6cbfdfe5a77008ba298667bb7add8c236027a Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Fri, 9 Dec 2016 09:39:39 -0500
Subject: [PATCH 5/5] librbd: block concurrent in-flight object map updates for
 the same object

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 7d743bfce61c6ede0a34fc0982e52be1d367d772)
---
 src/librbd/ObjectMap.cc                |  77 ++++++-
 src/librbd/ObjectMap.h                 |  42 +++-
 src/test/librbd/CMakeLists.txt         |   1 +
 src/test/librbd/test_mock_ObjectMap.cc | 272 +++++++++++++++++++++++++
 4 files changed, 381 insertions(+), 11 deletions(-)
 create mode 100644 src/test/librbd/test_mock_ObjectMap.cc

diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index 9d57ea8b5a124..c59366eceb0b4 100644
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -2,6 +2,7 @@
 // vim: ts=8 sw=2 smarttab
 
 #include "librbd/ObjectMap.h"
+#include "librbd/BlockGuard.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/object_map/RefreshRequest.h"
@@ -26,14 +27,20 @@
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
-#define dout_prefix *_dout << "librbd::ObjectMap: "
+#define dout_prefix *_dout << "librbd::ObjectMap: " << this << " " << __func__ \
+                           << ": "
 
 namespace librbd {
 
 template <typename I>
 ObjectMap<I>::ObjectMap(I &image_ctx, uint64_t snap_id)
-  : m_image_ctx(image_ctx), m_snap_id(snap_id)
-{
+  : m_image_ctx(image_ctx), m_snap_id(snap_id),
+    m_update_guard(new UpdateGuard(m_image_ctx.cct)) {
+}
+
+template <typename I>
+ObjectMap<I>::~ObjectMap() {
+  delete m_update_guard;
 }
 
 template <typename I>
@@ -92,8 +99,7 @@ bool ObjectMap<I>::object_may_exist(uint64_t object_no) const
   RWLock::RLocker l(m_image_ctx.object_map_lock);
   uint8_t state = (*this)[object_no];
   bool exists = (state != OBJECT_NONEXISTENT);
-  ldout(m_image_ctx.cct, 20) << &m_image_ctx << " object_may_exist: "
-			     << "object_no=" << object_no << " r=" << exists
+  ldout(m_image_ctx.cct, 20) << "object_no=" << object_no << " r=" << exists
 			     << dendl;
   return exists;
 }
@@ -202,6 +208,63 @@ void ObjectMap<I>::aio_resize(uint64_t new_size, uint8_t default_object_state,
   req->send();
 }
 
+template <typename I>
+void ObjectMap<I>::detained_aio_update(UpdateOperation &&op) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 20) << dendl;
+
+  assert(m_image_ctx.snap_lock.is_locked());
+  assert(m_image_ctx.object_map_lock.is_wlocked());
+
+  BlockGuardCell *cell;
+  int r = m_update_guard->detain({op.start_object_no, op.end_object_no},
+                                &op, &cell);
+  if (r < 0) {
+    lderr(cct) << "failed to detain object map update: " << cpp_strerror(r)
+               << dendl;
+    m_image_ctx.op_work_queue->queue(op.on_finish, r);
+    return;
+  } else if (r > 0) {
+    ldout(cct, 20) << "detaining object map update due to in-flight update: "
+                   << "start=" << op.start_object_no << ", "
+		   << "end=" << op.end_object_no << ", "
+                   << (op.current_state ?
+                         stringify(static_cast<uint32_t>(*op.current_state)) :
+                         "")
+		   << "->" << static_cast<uint32_t>(op.new_state) << dendl;
+    return;
+  }
+
+  ldout(cct, 20) << "in-flight update cell: " << cell << dendl;
+  Context *on_finish = op.on_finish;
+  Context *ctx = new FunctionContext([this, cell, on_finish](int r) {
+      handle_detained_aio_update(cell, r, on_finish);
+    });
+  aio_update(CEPH_NOSNAP, op.start_object_no, op.end_object_no, op.new_state,
+             op.current_state, ctx);
+}
+
+template <typename I>
+void ObjectMap<I>::handle_detained_aio_update(BlockGuardCell *cell, int r,
+                                              Context *on_finish) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 20) << "cell=" << cell << ", r=" << r << dendl;
+
+  typename UpdateGuard::BlockOperations block_ops;
+  m_update_guard->release(cell, &block_ops);
+
+  {
+    RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
+    RWLock::RLocker snap_locker(m_image_ctx.snap_lock);
+    RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
+    for (auto &op : block_ops) {
+      detained_aio_update(std::move(op));
+    }
+  }
+
+  on_finish->complete(r);
+}
+
 template <typename I>
 void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
                               uint64_t end_object_no, uint8_t new_state,
@@ -217,8 +280,8 @@ void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
   assert(start_object_no < end_object_no);
 
   CephContext *cct = m_image_ctx.cct;
-  ldout(cct, 20) << &m_image_ctx << " aio_update: start=" << start_object_no
-		 << ", end=" << end_object_no << ", "
+  ldout(cct, 20) << "start=" << start_object_no << ", "
+		 << "end=" << end_object_no << ", "
                  << (current_state ?
                        stringify(static_cast<uint32_t>(*current_state)) : "")
 		 << "->" << static_cast<uint32_t>(new_state) << dendl;
diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h
index 3ac857322f17e..a3d5ea74e5396 100644
--- a/src/librbd/ObjectMap.h
+++ b/src/librbd/ObjectMap.h
@@ -19,6 +19,8 @@ namespace librados {
 
 namespace librbd {
 
+template <typename Op> class BlockGuard;
+struct BlockGuardCell;
 class ImageCtx;
 
 template <typename ImageCtxT = ImageCtx>
@@ -29,6 +31,7 @@ class ObjectMap {
   }
 
   ObjectMap(ImageCtxT &image_ctx, uint64_t snap_id);
+  ~ObjectMap();
 
   static int remove(librados::IoCtx &io_ctx, const std::string &image_id);
   static std::string object_map_name(const std::string &image_id,
@@ -77,11 +80,17 @@ class ObjectMap {
       if (object_no == end_object_no) {
         return false;
       }
-    }
 
-    aio_update(snap_id, start_object_no, end_object_no, new_state,
-               current_state,
-               util::create_context_callback<T, MF>(callback_object));
+      UpdateOperation update_operation(start_object_no, end_object_no,
+                                       new_state, current_state,
+                                       util::create_context_callback<T, MF>(
+                                         callback_object));
+      detained_aio_update(std::move(update_operation));
+    } else {
+      aio_update(snap_id, start_object_no, end_object_no, new_state,
+                 current_state,
+                 util::create_context_callback<T, MF>(callback_object));
+    }
     return true;
   }
 
@@ -90,10 +99,35 @@ class ObjectMap {
   void snapshot_remove(uint64_t snap_id, Context *on_finish);
 
 private:
+  struct UpdateOperation {
+    uint64_t start_object_no;
+    uint64_t end_object_no;
+    uint8_t new_state;
+    boost::optional<uint8_t> current_state;
+    Context *on_finish;
+
+    UpdateOperation(uint64_t start_object_no, uint64_t end_object_no,
+                    uint8_t new_state,
+                    const boost::optional<uint8_t> &current_state,
+                    Context *on_finish)
+      : start_object_no(start_object_no), end_object_no(end_object_no),
+        new_state(new_state), current_state(current_state),
+        on_finish(on_finish) {
+    }
+  };
+
+  typedef BlockGuard<UpdateOperation> UpdateGuard;
+
   ImageCtxT &m_image_ctx;
   ceph::BitVector<2> m_object_map;
   uint64_t m_snap_id;
 
+  UpdateGuard *m_update_guard = nullptr;
+
+  void detained_aio_update(UpdateOperation &&update_operation);
+  void handle_detained_aio_update(BlockGuardCell *cell, int r,
+                                  Context *on_finish);
+
   void aio_update(uint64_t snap_id, uint64_t start_object_no,
                   uint64_t end_object_no, uint8_t new_state,
                   const boost::optional<uint8_t> &current_state,
diff --git a/src/test/librbd/CMakeLists.txt b/src/test/librbd/CMakeLists.txt
index b2781979393fd..06d091092b888 100644
--- a/src/test/librbd/CMakeLists.txt
+++ b/src/test/librbd/CMakeLists.txt
@@ -21,6 +21,7 @@ set(unittest_librbd_srcs
   test_mock_AioImageRequest.cc
   test_mock_ExclusiveLock.cc
   test_mock_Journal.cc
+  test_mock_ObjectMap.cc
   test_mock_ObjectWatcher.cc
   exclusive_lock/test_mock_AcquireRequest.cc
   exclusive_lock/test_mock_ReleaseRequest.cc
diff --git a/src/test/librbd/test_mock_ObjectMap.cc b/src/test/librbd/test_mock_ObjectMap.cc
new file mode 100644
index 0000000000000..f14c6b9a5b274
--- /dev/null
+++ b/src/test/librbd/test_mock_ObjectMap.cc
@@ -0,0 +1,272 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#include "test/librbd/test_mock_fixture.h"
+#include "test/librbd/test_support.h"
+#include "test/librbd/mock/MockImageCtx.h"
+#include "librbd/ObjectMap.h"
+#include "librbd/object_map/RefreshRequest.h"
+#include "librbd/object_map/UnlockRequest.h"
+#include "librbd/object_map/UpdateRequest.h"
+
+namespace librbd {
+
+namespace {
+
+struct MockTestImageCtx : public MockImageCtx {
+  MockTestImageCtx(ImageCtx &image_ctx) : MockImageCtx(image_ctx) {
+  }
+};
+
+} // anonymous namespace
+
+namespace object_map {
+
+template <>
+struct RefreshRequest<MockTestImageCtx> {
+  Context *on_finish = nullptr;
+  ceph::BitVector<2u> *object_map = nullptr;
+  static RefreshRequest *s_instance;
+  static RefreshRequest *create(MockTestImageCtx &image_ctx,
+                                ceph::BitVector<2u> *object_map,
+                                uint64_t snap_id, Context *on_finish) {
+    assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    s_instance->object_map = object_map;
+    return s_instance;
+  }
+
+  MOCK_METHOD0(send, void());
+
+  RefreshRequest() {
+    s_instance = this;
+  }
+};
+
+template <>
+struct UnlockRequest<MockTestImageCtx> {
+  Context *on_finish = nullptr;
+  static UnlockRequest *s_instance;
+  static UnlockRequest *create(MockTestImageCtx &image_ctx,
+                               Context *on_finish) {
+    assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  MOCK_METHOD0(send, void());
+  UnlockRequest() {
+    s_instance = this;
+  }
+};
+
+template <>
+struct UpdateRequest<MockTestImageCtx> {
+  Context *on_finish = nullptr;
+  static UpdateRequest *s_instance;
+  static UpdateRequest *create(MockTestImageCtx &image_ctx,
+                               ceph::BitVector<2u> *object_map,
+                               uint64_t snap_id,
+                               uint64_t start_object_no, uint64_t end_object_no,
+                               uint8_t new_state,
+                               const boost::optional<uint8_t> &current_state,
+                               Context *on_finish) {
+    assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    s_instance->construct(snap_id, start_object_no, end_object_no, new_state,
+                          current_state);
+    return s_instance;
+  }
+
+  MOCK_METHOD5(construct, void(uint64_t snap_id, uint64_t start_object_no,
+                               uint64_t end_object_no, uint8_t new_state,
+                               const boost::optional<uint8_t> &current_state));
+  MOCK_METHOD0(send, void());
+  UpdateRequest() {
+    s_instance = this;
+  }
+};
+
+RefreshRequest<MockTestImageCtx> *RefreshRequest<MockTestImageCtx>::s_instance = nullptr;
+UnlockRequest<MockTestImageCtx> *UnlockRequest<MockTestImageCtx>::s_instance = nullptr;
+UpdateRequest<MockTestImageCtx> *UpdateRequest<MockTestImageCtx>::s_instance = nullptr;
+
+} // namespace object_map
+} // namespace librbd
+
+#include "librbd/ObjectMap.cc"
+
+namespace librbd {
+
+using testing::_;
+using testing::InSequence;
+using testing::Invoke;
+
+class TestMockObjectMap : public TestMockFixture {
+public:
+  typedef ObjectMap<MockTestImageCtx> MockObjectMap;
+  typedef object_map::RefreshRequest<MockTestImageCtx> MockRefreshRequest;
+  typedef object_map::UnlockRequest<MockTestImageCtx> MockUnlockRequest;
+  typedef object_map::UpdateRequest<MockTestImageCtx> MockUpdateRequest;
+
+  void expect_refresh(MockTestImageCtx &mock_image_ctx,
+                      MockRefreshRequest &mock_refresh_request,
+                      const ceph::BitVector<2u> &object_map, int r) {
+    EXPECT_CALL(mock_refresh_request, send())
+      .WillOnce(Invoke([&mock_image_ctx, &mock_refresh_request, &object_map, r]() {
+          *mock_refresh_request.object_map = object_map;
+          mock_image_ctx.image_ctx->op_work_queue->queue(mock_refresh_request.on_finish, r);
+        }));
+  }
+
+  void expect_unlock(MockTestImageCtx &mock_image_ctx,
+                     MockUnlockRequest &mock_unlock_request, int r) {
+    EXPECT_CALL(mock_unlock_request, send())
+      .WillOnce(Invoke([&mock_image_ctx, &mock_unlock_request, r]() {
+          mock_image_ctx.image_ctx->op_work_queue->queue(mock_unlock_request.on_finish, r);
+        }));
+  }
+
+  void expect_update(MockTestImageCtx &mock_image_ctx,
+                     MockUpdateRequest &mock_update_request,
+                     uint64_t snap_id, uint64_t start_object_no,
+                     uint64_t end_object_no, uint8_t new_state,
+                     const boost::optional<uint8_t> &current_state,
+                     Context **on_finish) {
+    EXPECT_CALL(mock_update_request, construct(snap_id, start_object_no,
+                                               end_object_no, new_state,
+                                               current_state))
+      .Times(1);
+    EXPECT_CALL(mock_update_request, send())
+      .WillOnce(Invoke([&mock_image_ctx, &mock_update_request, on_finish]() {
+          *on_finish = mock_update_request.on_finish;
+        }));
+  }
+
+};
+
+TEST_F(TestMockObjectMap, NonDetainedUpdate) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+
+  InSequence seq;
+  ceph::BitVector<2u> object_map;
+  object_map.resize(4);
+  MockRefreshRequest mock_refresh_request;
+  expect_refresh(mock_image_ctx, mock_refresh_request, object_map, 0);
+
+  MockUpdateRequest mock_update_request;
+  Context *finish_update_1;
+  expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP,
+                0, 1, 1, {}, &finish_update_1);
+  Context *finish_update_2;
+  expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP,
+                1, 2, 1, {}, &finish_update_2);
+
+  MockUnlockRequest mock_unlock_request;
+  expect_unlock(mock_image_ctx, mock_unlock_request, 0);
+
+  MockObjectMap mock_object_map(mock_image_ctx, CEPH_NOSNAP);
+  C_SaferCond open_ctx;
+  mock_object_map.open(&open_ctx);
+  ASSERT_EQ(0, open_ctx.wait());
+
+  C_SaferCond update_ctx1;
+  C_SaferCond update_ctx2;
+  {
+    RWLock::RLocker snap_locker(mock_image_ctx.snap_lock);
+    RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock);
+    mock_object_map.aio_update(CEPH_NOSNAP, 0, 1, {}, &update_ctx1);
+    mock_object_map.aio_update(CEPH_NOSNAP, 1, 1, {}, &update_ctx2);
+  }
+
+  finish_update_2->complete(0);
+  ASSERT_EQ(0, update_ctx2.wait());
+
+  finish_update_1->complete(0);
+  ASSERT_EQ(0, update_ctx1.wait());
+
+  C_SaferCond close_ctx;
+  mock_object_map.close(&close_ctx);
+  ASSERT_EQ(0, close_ctx.wait());
+}
+
+TEST_F(TestMockObjectMap, DetainedUpdate) {
+  REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+
+  InSequence seq;
+  ceph::BitVector<2u> object_map;
+  object_map.resize(4);
+  MockRefreshRequest mock_refresh_request;
+  expect_refresh(mock_image_ctx, mock_refresh_request, object_map, 0);
+
+  MockUpdateRequest mock_update_request;
+  Context *finish_update_1;
+  expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP,
+                1, 4, 1, {}, &finish_update_1);
+  Context *finish_update_2 = nullptr;
+  expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP,
+                1, 3, 1, {}, &finish_update_2);
+  Context *finish_update_3 = nullptr;
+  expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP,
+                2, 3, 1, {}, &finish_update_3);
+  Context *finish_update_4 = nullptr;
+  expect_update(mock_image_ctx, mock_update_request, CEPH_NOSNAP,
+                0, 2, 1, {}, &finish_update_4);
+
+  MockUnlockRequest mock_unlock_request;
+  expect_unlock(mock_image_ctx, mock_unlock_request, 0);
+
+  MockObjectMap mock_object_map(mock_image_ctx, CEPH_NOSNAP);
+  C_SaferCond open_ctx;
+  mock_object_map.open(&open_ctx);
+  ASSERT_EQ(0, open_ctx.wait());
+
+  C_SaferCond update_ctx1;
+  C_SaferCond update_ctx2;
+  C_SaferCond update_ctx3;
+  C_SaferCond update_ctx4;
+  {
+    RWLock::RLocker snap_locker(mock_image_ctx.snap_lock);
+    RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock);
+    mock_object_map.aio_update(CEPH_NOSNAP, 1, 4, 1, {}, &update_ctx1);
+    mock_object_map.aio_update(CEPH_NOSNAP, 1, 3, 1, {}, &update_ctx2);
+    mock_object_map.aio_update(CEPH_NOSNAP, 2, 3, 1, {}, &update_ctx3);
+    mock_object_map.aio_update(CEPH_NOSNAP, 0, 2, 1, {}, &update_ctx4);
+  }
+
+  // updates 2, 3, and 4 are blocked on update 1
+  ASSERT_EQ(nullptr, finish_update_2);
+  finish_update_1->complete(0);
+  ASSERT_EQ(0, update_ctx1.wait());
+
+  // updates 3 and 4 are blocked on update 2
+  ASSERT_NE(nullptr, finish_update_2);
+  ASSERT_EQ(nullptr, finish_update_3);
+  ASSERT_EQ(nullptr, finish_update_4);
+  finish_update_2->complete(0);
+  ASSERT_EQ(0, update_ctx2.wait());
+
+  ASSERT_NE(nullptr, finish_update_3);
+  ASSERT_NE(nullptr, finish_update_4);
+  finish_update_3->complete(0);
+  finish_update_4->complete(0);
+  ASSERT_EQ(0, update_ctx3.wait());
+  ASSERT_EQ(0, update_ctx4.wait());
+
+  C_SaferCond close_ctx;
+  mock_object_map.close(&close_ctx);
+  ASSERT_EQ(0, close_ctx.wait());
+}
+
+} // namespace librbd
+