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>

diff --git a/src/librbd/object_map/SnapshotCreateRequest.cc b/src/librbd/object_map/SnapshotCreateRequest.cc
index eec5a642950d15c92d28526803459800b78e8998..2949dec8dd7bb074e2bece35e4a6fa07ba08fbd2 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() {
   ceph_assert(m_image_ctx.snap_lock.is_locked());
-  ceph_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 ab53d8ed0e9d49c836eda11b1cb5c166310a8d04..31553177306f0183335f3abaea910d5b68d42ef8 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,6 +237,7 @@ Context *SnapshotCreateRequest<I>::handle_create_object_map(int *result) {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl;
 
+  update_snap_context();
   image_ctx.io_work_queue->unblock_writes();
   if (*result < 0) {
     lderr(cct) << this << " " << __func__ << ": failed to snapshot object map: "

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);