librbd: delay marking primary mirror snapshot as completed

Until the image state object(s) are written, the snapshot should
be considered incomplete and not ready for replay.

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

diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
index 9cc5063827de89e47f4a3cc9c066c90ac6dd8488..ae31256419d86539560f060292c728142a3324c5 100644 (file)
--- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
+++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
@@ -115,10 +115,6 @@ void CreatePrimaryRequest<I>::create_snapshot() {
       cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY_DEMOTED :
       cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY),
     m_mirror_peer_uuids, "", CEPH_NOSNAP};
-
-  // TODO delay until after image state written
-  ns.complete = true;
-
   auto ctx = create_context_callback<
     CreatePrimaryRequest<I>,
     &CreatePrimaryRequest<I>::handle_create_snapshot>(this);

diff --git a/src/librbd/mirror/snapshot/SetImageStateRequest.cc b/src/librbd/mirror/snapshot/SetImageStateRequest.cc
index 8e2c804a2911ff7c02af33b8cd71a9b44a34a2e3..6c90a1614c5cd598239cca671ad52bc4a0089e19 100644 (file)
--- a/src/librbd/mirror/snapshot/SetImageStateRequest.cc
+++ b/src/librbd/mirror/snapshot/SetImageStateRequest.cc
@@ -165,6 +165,39 @@ void SetImageStateRequest<I>::handle_write_image_state(int r) {
     return;
   }
 
+  update_primary_snapshot();
+}
+
+template <typename I>
+void SetImageStateRequest<I>::update_primary_snapshot() {
+  CephContext *cct = m_image_ctx->cct;
+  ldout(cct, 20) << dendl;
+
+  librados::ObjectWriteOperation op;
+  librbd::cls_client::mirror_image_snapshot_set_copy_progress(
+    &op, m_snap_id, true, 0);
+
+  auto aio_comp = create_rados_callback<
+    SetImageStateRequest<I>,
+    &SetImageStateRequest<I>::handle_update_primary_snapshot>(this);
+  int r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, aio_comp,
+                                          &op);
+  ceph_assert(r == 0);
+  aio_comp->release();
+}
+
+template <typename I>
+void SetImageStateRequest<I>::handle_update_primary_snapshot(int r) {
+  CephContext *cct = m_image_ctx->cct;
+  ldout(cct, 20) << "r=" << r << dendl;
+
+  if (r < 0) {
+    lderr(cct) << "failed to update primary snapshot: " << cpp_strerror(r)
+               << dendl;
+    finish(r);
+    return;
+  }
+
   finish(0);
 }
 

diff --git a/src/librbd/mirror/snapshot/SetImageStateRequest.h b/src/librbd/mirror/snapshot/SetImageStateRequest.h
index a16ba74c49f53d4c94d816464dcbf96374523798..663c281340452eb40a72357851c424f9ed21bb6f 100644 (file)
--- a/src/librbd/mirror/snapshot/SetImageStateRequest.h
+++ b/src/librbd/mirror/snapshot/SetImageStateRequest.h
@@ -49,6 +49,9 @@ private:
    * WRITE_IMAGE_STATE
    *    |
    *    v
+   * UPDATE_PRIMARY_SNAPSHOT
+   *    |
+   *    v
    * <finish>
    *
    * @endverbatim
@@ -73,6 +76,9 @@ private:
   void write_image_state();
   void handle_write_image_state(int r);
 
+  void update_primary_snapshot();
+  void handle_update_primary_snapshot(int r);
+
   void finish(int r);
 };
 

diff --git a/src/test/librbd/test_mirroring.cc b/src/test/librbd/test_mirroring.cc
index cb69dcfbc1d317809b27381ac34bbdfc075106ff..199b5e23b8771de1491eafdb76f78c31ab7992aa 100644 (file)
--- a/src/test/librbd/test_mirroring.cc
+++ b/src/test/librbd/test_mirroring.cc
@@ -1331,6 +1331,8 @@ TEST_F(TestMirroring, SnapshotImageState)
 {
   REQUIRE_FORMAT_V2();
 
+  ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_IMAGE));
+
   uint64_t features;
   ASSERT_TRUE(get_features(&features));
   int order = 20;
@@ -1340,6 +1342,11 @@ TEST_F(TestMirroring, SnapshotImageState)
   librbd::Image image;
   ASSERT_EQ(0, m_rbd.open(m_ioctx, image, image_name.c_str()));
   ASSERT_EQ(0, image.snap_create("snap"));
+  ASSERT_EQ(0, image.mirror_image_enable2(RBD_MIRROR_IMAGE_MODE_SNAPSHOT));
+  std::vector<librbd::snap_info_t> snaps;
+  ASSERT_EQ(0, image.snap_list(snaps));
+  ASSERT_EQ(2U, snaps.size());
+  auto snap_id = snaps[1].id;
 
   auto ictx = new librbd::ImageCtx(image_name, "", nullptr, m_ioctx, false);
   ASSERT_EQ(0, ictx->state->open(0));
@@ -1352,7 +1359,7 @@ TEST_F(TestMirroring, SnapshotImageState)
   {
     C_SaferCond cond;
     auto req = librbd::mirror::snapshot::SetImageStateRequest<>::create(
-      ictx, 123, &cond);
+      ictx, snap_id, &cond);
     req->send();
     ASSERT_EQ(0, cond.wait());
   }
@@ -1361,7 +1368,7 @@ TEST_F(TestMirroring, SnapshotImageState)
   {
     C_SaferCond cond;
     auto req = librbd::mirror::snapshot::GetImageStateRequest<>::create(
-      ictx, 123, &image_state, &cond);
+      ictx, snap_id, &image_state, &cond);
     req->send();
     ASSERT_EQ(0, cond.wait());
   }
@@ -1376,7 +1383,7 @@ TEST_F(TestMirroring, SnapshotImageState)
   {
     C_SaferCond cond;
     auto req = librbd::mirror::snapshot::RemoveImageStateRequest<>::create(
-      ictx, 123, &cond);
+      ictx, snap_id, &cond);
     req->send();
     ASSERT_EQ(0, cond.wait());
   }
@@ -1392,7 +1399,7 @@ TEST_F(TestMirroring, SnapshotImageState)
   {
     C_SaferCond cond;
     auto req = librbd::mirror::snapshot::SetImageStateRequest<>::create(
-      ictx, 123, &cond);
+      ictx, snap_id, &cond);
     req->send();
     ASSERT_EQ(0, cond.wait());
   }
@@ -1400,7 +1407,7 @@ TEST_F(TestMirroring, SnapshotImageState)
   {
     C_SaferCond cond;
     auto req = librbd::mirror::snapshot::GetImageStateRequest<>::create(
-      ictx, 123, &image_state, &cond);
+      ictx, snap_id, &image_state, &cond);
     req->send();
     ASSERT_EQ(0, cond.wait());
   }
@@ -1417,7 +1424,7 @@ TEST_F(TestMirroring, SnapshotImageState)
   {
     C_SaferCond cond;
     auto req = librbd::mirror::snapshot::RemoveImageStateRequest<>::create(
-      ictx, 123, &cond);
+      ictx, snap_id, &cond);
     req->send();
     ASSERT_EQ(0, cond.wait());
   }