rbd-mirror: image create should lookup local parent snapshot id

A recent code change associated with a librbd cleanup incorrectly started
using the remote parent image's snapshot id.

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

diff --git a/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc
index 62c0e43e3856297238e9086feb87f77953f7731f..b78c1d3b03bcbc25f2dbfe0df71d72e428b7e2f6 100644 (file)
--- a/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc
+++ b/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc
@@ -206,21 +206,17 @@ public:
     ASSERT_EQ(0, open_image(m_remote_io_ctx, m_image_name, &m_remote_image_ctx));
   }
 
+  void snap_create(librbd::ImageCtx *image_ctx, const std::string &snap_name) {
+    ASSERT_EQ(0, image_ctx->operations->snap_create(cls::rbd::UserSnapshotNamespace(),
+                                                   snap_name.c_str()));
+    ASSERT_EQ(0, image_ctx->operations->snap_protect(cls::rbd::UserSnapshotNamespace(),
+                                                    snap_name.c_str()));
+    ASSERT_EQ(0, image_ctx->state->refresh());
+  }
+
   int clone_image(librbd::ImageCtx *parent_image_ctx,
                   const std::string &snap_name, const std::string &clone_name) {
-    {
-      librbd::ImageCtx *ictx = new librbd::ImageCtx(parent_image_ctx->name,
-                                                   "", "", m_remote_io_ctx,
-                                                    false);
-      ictx->state->open(false);
-      EXPECT_EQ(0, ictx->operations->snap_create(cls::rbd::UserSnapshotNamespace(),
-                                                snap_name.c_str()));
-      EXPECT_EQ(0, ictx->operations->snap_protect(cls::rbd::UserSnapshotNamespace(),
-                                                 snap_name.c_str()));
-      ictx->state->close();
-    }
-
-    EXPECT_EQ(0, parent_image_ctx->state->refresh());
+    snap_create(parent_image_ctx, snap_name);
 
     int order = 0;
     return librbd::clone(m_remote_io_ctx, parent_image_ctx->name.c_str(),
@@ -292,7 +288,7 @@ public:
           RWLock::RLocker snap_locker(mock_image_ctx.snap_lock);
           auto id = mock_image_ctx.image_ctx->get_snap_id(
             cls::rbd::UserSnapshotNamespace{}, snap_name);
-          ASSERT_NE(snap_id, id);
+          ASSERT_EQ(snap_id, id);
           m_threads->work_queue->queue(on_finish, r);
         }));
   }
@@ -366,6 +362,7 @@ TEST_F(TestMockImageReplayerCreateImageRequest, Clone) {
   librbd::ImageCtx *local_image_ctx;
   ASSERT_EQ(0, create_image(rbd, m_local_io_ctx, m_image_name, m_image_size));
   ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &local_image_ctx));
+  snap_create(local_image_ctx, "snap");
 
   std::string clone_image_name = get_temp_image_name();
   ASSERT_EQ(0, clone_image(m_remote_image_ctx, "snap", clone_image_name));
@@ -533,6 +530,7 @@ TEST_F(TestMockImageReplayerCreateImageRequest, CloneSnapSetError) {
   librbd::ImageCtx *local_image_ctx;
   ASSERT_EQ(0, create_image(rbd, m_local_io_ctx, m_image_name, m_image_size));
   ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &local_image_ctx));
+  snap_create(local_image_ctx, "snap");
 
   std::string clone_image_name = get_temp_image_name();
   ASSERT_EQ(0, clone_image(m_remote_image_ctx, "snap", clone_image_name));
@@ -576,6 +574,7 @@ TEST_F(TestMockImageReplayerCreateImageRequest, CloneError) {
   librbd::ImageCtx *local_image_ctx;
   ASSERT_EQ(0, create_image(rbd, m_local_io_ctx, m_image_name, m_image_size));
   ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &local_image_ctx));
+  snap_create(local_image_ctx, "snap");
 
   std::string clone_image_name = get_temp_image_name();
   ASSERT_EQ(0, clone_image(m_remote_image_ctx, "snap", clone_image_name));
@@ -620,6 +619,7 @@ TEST_F(TestMockImageReplayerCreateImageRequest, CloneLocalParentCloseError) {
   librbd::ImageCtx *local_image_ctx;
   ASSERT_EQ(0, create_image(rbd, m_local_io_ctx, m_image_name, m_image_size));
   ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &local_image_ctx));
+  snap_create(local_image_ctx, "snap");
 
   std::string clone_image_name = get_temp_image_name();
   ASSERT_EQ(0, clone_image(m_remote_image_ctx, "snap", clone_image_name));
@@ -664,6 +664,7 @@ TEST_F(TestMockImageReplayerCreateImageRequest, CloneRemoteParentCloseError) {
   librbd::ImageCtx *local_image_ctx;
   ASSERT_EQ(0, create_image(rbd, m_local_io_ctx, m_image_name, m_image_size));
   ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &local_image_ctx));
+  snap_create(local_image_ctx, "snap");
 
   std::string clone_image_name = get_temp_image_name();
   ASSERT_EQ(0, clone_image(m_remote_image_ctx, "snap", clone_image_name));

diff --git a/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc b/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc
index 351bf1e50ce6fffec94638a84a37cc59d7ba44f4..7a2bf9031649670231a77f53b5ab5fab09fb2417 100644 (file)
--- a/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc
+++ b/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc
@@ -262,20 +262,55 @@ void CreateImageRequest<I>::handle_open_local_parent_image(int r) {
 
 template <typename I>
 void CreateImageRequest<I>::set_local_parent_snap() {
-  dout(20) << ": parent_snap_id=" << m_remote_parent_spec.snap_id << dendl;
+  std::string snap_name;
+  cls::rbd::SnapshotNamespace snap_namespace;
+  {
+    RWLock::RLocker remote_snap_locker(m_remote_parent_image_ctx->snap_lock);
+    auto it = m_remote_parent_image_ctx->snap_info.find(
+      m_remote_parent_spec.snap_id);
+    if (it != m_remote_parent_image_ctx->snap_info.end()) {
+      snap_name = it->second.name;
+      snap_namespace = it->second.snap_namespace;
+    }
+  }
+
+  if (snap_name.empty()) {
+    dout(15) << ": failed to locate remote parent snapshot" << dendl;
+    m_ret_val = -ENOENT;
+    close_local_parent_image();
+    return;
+  }
+
+  m_local_parent_spec.snap_id = CEPH_NOSNAP;
+  {
+    RWLock::RLocker local_snap_locker(m_local_parent_image_ctx->snap_lock);
+    auto it = m_local_parent_image_ctx->snap_ids.find(
+      {snap_namespace, snap_name});
+    if (it != m_local_parent_image_ctx->snap_ids.end()) {
+      m_local_parent_spec.snap_id = it->second;
+    }
+  }
+
+  if (m_local_parent_spec.snap_id == CEPH_NOSNAP) {
+    dout(15) << ": failed to locate local parent snapshot" << dendl;
+    m_ret_val = -ENOENT;
+    close_local_parent_image();
+    return;
+  }
+
+  dout(15) << ": local_snap_id=" << m_local_parent_spec.snap_id << dendl;
 
   Context *ctx = create_context_callback<
     CreateImageRequest<I>,
     &CreateImageRequest<I>::handle_set_local_parent_snap>(this);
-  m_local_parent_image_ctx->state->snap_set(m_remote_parent_spec.snap_id,
-                                           ctx);
+  m_local_parent_image_ctx->state->snap_set(m_local_parent_spec.snap_id, ctx);
 }
 
 template <typename I>
 void CreateImageRequest<I>::handle_set_local_parent_snap(int r) {
   dout(20) << ": r=" << r << dendl;
   if (r < 0) {
-    derr << ": failed to set parent snapshot " << m_remote_parent_spec.snap_id
+    derr << ": failed to set parent snapshot " << m_local_parent_spec.snap_id
          << ": " << cpp_strerror(r) << dendl;
     m_ret_val = r;
     close_local_parent_image();