librbd: keep IO blocked until after snapshot object map created

The IO was being unblocked before object map was created, allowing
a potential copyup request to fail to update a still-to-be-created
object map.

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

Conflicts:
	src/librbd/object_map/SnapshotCreateRequest.cc: trivial resolution
	src/librbd/operation/SnapshotCreateRequest.cc: trivial resolution

diff --git a/src/librbd/object_map/SnapshotCreateRequest.cc b/src/librbd/object_map/SnapshotCreateRequest.cc
index acee5ff1d306e2d12fd9b2a07b7538a17843aee2..d79dea98b846be5478c9d26952f269964ac83b41 100644 (file)
--- a/src/librbd/object_map/SnapshotCreateRequest.cc
+++ b/src/librbd/object_map/SnapshotCreateRequest.cc
@@ -76,7 +76,6 @@ bool SnapshotCreateRequest::should_complete(int r) {
 
 void SnapshotCreateRequest::send_read_map() {
   assert(m_image_ctx.snap_lock.is_locked());
-  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));
@@ -135,7 +134,7 @@ bool SnapshotCreateRequest::send_add_snapshot() {
 void SnapshotCreateRequest::update_object_map() {
   RWLock::WLocker snap_locker(m_image_ctx.snap_lock);
   RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
-  
+
   auto it = m_object_map.begin();
   auto end_it = m_object_map.end();
   for (; it != end_it; ++it) {

diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc
index 63ddd8848c4d38ec785745fe045a2a20a223fe00..2208dedbf6733400d3e4d32f2dfeee2885c32325 100644 (file)
--- a/src/librbd/operation/SnapshotCreateRequest.cc
+++ b/src/librbd/operation/SnapshotCreateRequest.cc
@@ -208,12 +208,11 @@ template <typename I>
 Context *SnapshotCreateRequest<I>::send_create_object_map() {
   I &image_ctx = this->m_image_ctx;
 
-  update_snap_context();
-
   image_ctx.snap_lock.get_read();
   if (image_ctx.object_map == nullptr || m_skip_object_map) {
     image_ctx.snap_lock.put_read();
 
+    update_snap_context();
     image_ctx.io_work_queue->unblock_writes();
     return this->create_context_finisher(0);
   }
@@ -238,9 +237,14 @@ Context *SnapshotCreateRequest<I>::handle_create_object_map(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  assert(*result == 0);
-
+  update_snap_context();
   image_ctx.io_work_queue->unblock_writes();
+  if (*result < 0) {
+    lderr(cct) << this << " " << __func__ << ": failed to snapshot object map: "
+               << cpp_strerror(*result) << dendl;
+    return this->create_context_finisher(*result);
+  }
+
   return this->create_context_finisher(0);
 }
 

diff --git a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
index 7124df5b367bf5c9d00ef751e01bead8e790c9f0..64d995d43fcf6a04f2dca13cebd9e7bb22bf7e99 100644 (file)
--- a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
+++ b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
@@ -125,8 +125,8 @@ TEST_F(TestMockOperationSnapshotCreateRequest, Success) {
   expect_allocate_snap_id(mock_image_ctx, 0);
   expect_snap_create(mock_image_ctx, 0);
   if (!mock_image_ctx.old_format) {
-    expect_update_snap_context(mock_image_ctx);
     expect_object_map_snap_create(mock_image_ctx);
+    expect_update_snap_context(mock_image_ctx);
   }
   expect_unblock_writes(mock_image_ctx);
 
@@ -194,8 +194,8 @@ TEST_F(TestMockOperationSnapshotCreateRequest, CreateSnapStale) {
   expect_allocate_snap_id(mock_image_ctx, -ESTALE);
   expect_snap_create(mock_image_ctx, -ESTALE);
   if (!mock_image_ctx.old_format) {
-    expect_update_snap_context(mock_image_ctx);
     expect_object_map_snap_create(mock_image_ctx);
+    expect_update_snap_context(mock_image_ctx);
   }
   expect_unblock_writes(mock_image_ctx);