rbd-mirror: fix issue with snapshot-based mirroring deletion propagation

The mirror uuid is only used for journal-based mirroring to determine if
the images are linked. Snapshot-based mirroring needs to ensure the
mirror peer uuid is still initialized even if the image isn't available.

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

10 files changed, 57 insertions, 20 deletions

diff --git a/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc
index 1888607a622..d2cb1afb677 100644
--- a/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc
+++ b/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc
@@ -101,7 +101,6 @@ struct StateBuilder<librbd::MockTestImageCtx> {
 
   std::string local_image_id;
   librbd::mirror::PromotionState local_promotion_state;
-  std::string local_primary_mirror_uuid;
 };
 
 GetMirrorImageIdRequest<librbd::MockTestImageCtx>* GetMirrorImageIdRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
@@ -116,6 +115,8 @@ struct StateBuilder<librbd::MockTestImageCtx>
   cls::rbd::MirrorImageMode mirror_image_mode =
     cls::rbd::MIRROR_IMAGE_MODE_JOURNAL;
 
+  std::string local_primary_mirror_uuid;
+
   static StateBuilder* create(const std::string&) {
     ceph_assert(s_instance != nullptr);
     return s_instance;
@@ -298,8 +299,6 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, SuccessSnapshot) {
             mock_journal_state_builder.mirror_image_mode);
   ASSERT_EQ(librbd::mirror::PROMOTION_STATE_NON_PRIMARY,
             mock_journal_state_builder.local_promotion_state);
-  ASSERT_EQ(std::string("remote mirror uuid"),
-            mock_journal_state_builder.local_primary_mirror_uuid);
 }
 
 TEST_F(TestMockImageReplayerPrepareLocalImageRequest, MirrorImageIdError) {
diff --git a/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc
index 04ee88205db..ae03f708c08 100644
--- a/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc
+++ b/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc
@@ -120,6 +120,8 @@ struct GetMirrorImageIdRequest<librbd::MockTestImageCtx> {
 template<>
 struct StateBuilder<librbd::MockTestImageCtx> {
   std::string local_image_id;
+  librbd::mirror::PromotionState local_promotion_state =
+    librbd::mirror::PROMOTION_STATE_NON_PRIMARY;
   std::string remote_image_id;
   std::string remote_mirror_uuid;
   librbd::mirror::PromotionState remote_promotion_state;
diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
index b4b237d044f..0ec96c58f8f 100644
--- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
+++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
@@ -178,7 +178,14 @@ void BootstrapRequest<I>::handle_prepare_remote_image(int r) {
     finish(r);
     return;
   } else if (r == -ENOENT || state_builder == nullptr) {
-    dout(10) << "remote image does not exist" << dendl;
+    dout(10) << "remote image does not exist";
+    if (state_builder != nullptr) {
+      *_dout << ": "
+             << "local_image_id=" << state_builder->local_image_id  << ", "
+             << "remote_image_id=" << state_builder->remote_image_id << ", "
+             << "is_linked=" << state_builder->is_linked();
+    }
+    *_dout << dendl;
 
     // TODO need to support multiple remote images
     if (state_builder != nullptr &&
diff --git a/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc b/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc
index 4cbf2bd923b..a64117425d9 100644
--- a/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc
+++ b/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc
@@ -131,7 +131,11 @@ void PrepareLocalImageRequest<I>::handle_get_mirror_info(int r) {
   switch (m_mirror_image.mode) {
   case cls::rbd::MIRROR_IMAGE_MODE_JOURNAL:
     // journal-based local image exists
-    *m_state_builder = journal::StateBuilder<I>::create(m_global_image_id);
+    {
+      auto state_builder = journal::StateBuilder<I>::create(m_global_image_id);
+      state_builder->local_primary_mirror_uuid = m_primary_mirror_uuid;
+      *m_state_builder = state_builder;
+    }
     break;
   case cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT:
     // snapshot-based local image exists
@@ -144,9 +148,11 @@ void PrepareLocalImageRequest<I>::handle_get_mirror_info(int r) {
     break;
   }
 
+  dout(10) << "local_image_id=" << m_local_image_id << ", "
+           << "local_promotion_state=" << m_promotion_state << ", "
+           << "local_primary_mirror_uuid=" << m_primary_mirror_uuid << dendl;
   (*m_state_builder)->local_image_id = m_local_image_id;
   (*m_state_builder)->local_promotion_state = m_promotion_state;
-  (*m_state_builder)->local_primary_mirror_uuid = m_primary_mirror_uuid;
   finish(0);
 }
 
diff --git a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc
index b3d4ec4e18a..3231d5fa023 100644
--- a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc
+++ b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc
@@ -60,7 +60,11 @@ void PrepareRemoteImageRequest<I>::handle_get_remote_image_id(int r) {
   dout(10) << "r=" << r << ", "
            << "remote_image_id=" << m_remote_image_id << dendl;
 
-  if (r < 0) {
+  if (r == -ENOENT) {
+    finalize_snapshot_state_builder(r);
+    finish(r);
+    return;
+  } else if (r < 0) {
     finish(r);
     return;
   }
@@ -88,6 +92,7 @@ void PrepareRemoteImageRequest<I>::handle_get_mirror_info(int r) {
 
   if (r == -ENOENT) {
     dout(10) << "image " << m_global_image_id << " not mirrored" << dendl;
+    finalize_snapshot_state_builder(r);
     finish(r);
     return;
   } else if (r < 0) {
@@ -110,7 +115,9 @@ void PrepareRemoteImageRequest<I>::handle_get_mirror_info(int r) {
     return;
   } else if (m_promotion_state != librbd::mirror::PROMOTION_STATE_PRIMARY &&
              (state_builder == nullptr ||
-              state_builder->local_image_id.empty())) {
+              state_builder->local_image_id.empty() ||
+              state_builder->local_promotion_state ==
+                librbd::mirror::PROMOTION_STATE_UNKNOWN)) {
     // no local image and remote isn't primary -- don't sync it
     dout(5) << "remote image is not primary -- not syncing" << dendl;
     finish(-EREMOTEIO);
@@ -122,7 +129,7 @@ void PrepareRemoteImageRequest<I>::handle_get_mirror_info(int r) {
     get_client();
     break;
   case cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT:
-    finalize_snapshot_state_builder();
+    finalize_snapshot_state_builder(0);
     finish(0);
     break;
   default:
@@ -240,18 +247,26 @@ void PrepareRemoteImageRequest<I>::finalize_journal_state_builder(
 }
 
 template <typename I>
-void PrepareRemoteImageRequest<I>::finalize_snapshot_state_builder() {
+void PrepareRemoteImageRequest<I>::finalize_snapshot_state_builder(int r) {
   snapshot::StateBuilder<I>* state_builder = nullptr;
   if (*m_state_builder != nullptr) {
-    // already verified that it's a matching builder in
-    // 'handle_get_mirror_info'
     state_builder = dynamic_cast<snapshot::StateBuilder<I>*>(*m_state_builder);
-    ceph_assert(state_builder != nullptr);
-  } else {
+  } else if (r >= 0) {
     state_builder = snapshot::StateBuilder<I>::create(m_global_image_id);
     *m_state_builder = state_builder;
   }
 
+  if (state_builder == nullptr) {
+    // local image prepare failed or is using journal-based mirroring
+    return;
+  }
+
+  dout(10) << "remote_mirror_uuid=" << m_remote_pool_meta.mirror_uuid << ", "
+           << "remote_mirror_peer_uuid="
+           << m_remote_pool_meta.mirror_peer_uuid << ", "
+           << "remote_image_id=" << m_remote_image_id << ", "
+           << "remote_promotion_state=" << m_promotion_state << dendl;
+  ceph_assert(state_builder != nullptr);
   state_builder->remote_mirror_uuid = m_remote_pool_meta.mirror_uuid;
   state_builder->remote_mirror_peer_uuid = m_remote_pool_meta.mirror_peer_uuid;
   state_builder->remote_image_id = m_remote_image_id;
diff --git a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h
index b546cdc80ff..8006c50666a 100644
--- a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h
+++ b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h
@@ -118,7 +118,8 @@ private:
   bufferlist m_out_bl;
   std::string m_remote_image_id;
   cls::rbd::MirrorImage m_mirror_image;
-  librbd::mirror::PromotionState m_promotion_state;
+  librbd::mirror::PromotionState m_promotion_state =
+    librbd::mirror::PROMOTION_STATE_UNKNOWN;
   std::string m_primary_mirror_uuid;
 
   // journal-based mirroring
@@ -139,7 +140,7 @@ private:
 
   void finalize_journal_state_builder(cls::journal::ClientState client_state,
                                       const MirrorPeerClientMeta& client_meta);
-  void finalize_snapshot_state_builder();
+  void finalize_snapshot_state_builder(int r);
 
   void finish(int r);
 };
diff --git a/src/tools/rbd_mirror/image_replayer/StateBuilder.cc b/src/tools/rbd_mirror/image_replayer/StateBuilder.cc
index 20a5378d47a..3cd03e51e5c 100644
--- a/src/tools/rbd_mirror/image_replayer/StateBuilder.cc
+++ b/src/tools/rbd_mirror/image_replayer/StateBuilder.cc
@@ -43,10 +43,8 @@ bool StateBuilder<I>::is_local_primary() const  {
 
 template <typename I>
 bool StateBuilder<I>::is_linked() const {
-  ceph_assert(!remote_mirror_uuid.empty());
   return (local_promotion_state ==
-            librbd::mirror::PROMOTION_STATE_NON_PRIMARY &&
-          local_primary_mirror_uuid == remote_mirror_uuid);
+            librbd::mirror::PROMOTION_STATE_NON_PRIMARY);
 }
 
 template <typename I>
diff --git a/src/tools/rbd_mirror/image_replayer/StateBuilder.h b/src/tools/rbd_mirror/image_replayer/StateBuilder.h
index cb70fdf7972..99f5fce4487 100644
--- a/src/tools/rbd_mirror/image_replayer/StateBuilder.h
+++ b/src/tools/rbd_mirror/image_replayer/StateBuilder.h
@@ -82,7 +82,6 @@ public:
   std::string local_image_id;
   librbd::mirror::PromotionState local_promotion_state =
     librbd::mirror::PROMOTION_STATE_PRIMARY;
-  std::string local_primary_mirror_uuid;
   ImageCtxT* local_image_ctx = nullptr;
 
   std::string remote_mirror_uuid;
diff --git a/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc b/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc
index 8cb032b25c0..c28e0694823 100644
--- a/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc
+++ b/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc
@@ -58,6 +58,13 @@ bool StateBuilder<I>::is_disconnected() const {
 }
 
 template <typename I>
+bool StateBuilder<I>::is_linked() const {
+  ceph_assert(!this->remote_mirror_uuid.empty());
+  return (image_replayer::StateBuilder<I>::is_linked() &&
+          local_primary_mirror_uuid == this->remote_mirror_uuid);
+}
+
+template <typename I>
 cls::rbd::MirrorImageMode StateBuilder<I>::get_mirror_image_mode() const {
   return cls::rbd::MIRROR_IMAGE_MODE_JOURNAL;
 }
diff --git a/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.h b/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.h
index f35ff9fe624..3f3621727ce 100644
--- a/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.h
+++ b/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.h
@@ -37,6 +37,7 @@ public:
   void close(Context* on_finish) override;
 
   bool is_disconnected() const override;
+  bool is_linked() const override;
 
   cls::rbd::MirrorImageMode get_mirror_image_mode() const override;
 
@@ -63,6 +64,8 @@ public:
       PoolMetaCache* pool_meta_cache,
       ReplayerListener* replayer_listener) override;
 
+  std::string local_primary_mirror_uuid;
+
   Journaler* remote_journaler = nullptr;
   cls::journal::ClientState remote_client_state =
     cls::journal::CLIENT_STATE_CONNECTED;