From efcc8d14612487b5426fa5ab6f8f502760c4e697 Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Mon, 9 Mar 2020 22:26:13 -0400
Subject: [PATCH] librbd: enable mirroring for non-primary images w/o
 journaling

The state machine was incorrectly skipping over the mirror enable
step for non-primary images when the journaling feature bit was not
enabled.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
 src/librbd/image/CloneRequest.cc              | 18 ++++----
 src/librbd/image/CreateRequest.cc             |  2 +-
 .../librbd/image/test_mock_CloneRequest.cc    | 43 +++++++++++++++++++
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/src/librbd/image/CloneRequest.cc b/src/librbd/image/CloneRequest.cc
index 5da8769a3ef..0de9f78afa4 100644
--- a/src/librbd/image/CloneRequest.cc
+++ b/src/librbd/image/CloneRequest.cc
@@ -424,7 +424,10 @@ template <typename I>
 void CloneRequest<I>::get_mirror_mode() {
   ldout(m_cct, 15) << dendl;
 
-  if (!m_imctx->test_features(RBD_FEATURE_JOURNALING)) {
+  if (!m_non_primary_global_image_id.empty()) {
+    enable_mirror();
+    return;
+  } else if (!m_imctx->test_features(RBD_FEATURE_JOURNALING)) {
     close_child();
     return;
   }
@@ -455,15 +458,12 @@ void CloneRequest<I>::handle_get_mirror_mode(int r) {
                  << dendl;
 
     m_r_saved = r;
-    close_child();
-  } else {
-    if (m_mirror_mode == cls::rbd::MIRROR_MODE_POOL ||
-	!m_non_primary_global_image_id.empty()) {
-      enable_mirror();
-    } else {
-      close_child();
-    }
+  } else if (m_mirror_mode == cls::rbd::MIRROR_MODE_POOL) {
+    enable_mirror();
+    return;
   }
+
+  close_child();
 }
 
 template <typename I>
diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc
index ca4f141652e..175565223c7 100644
--- a/src/librbd/image/CreateRequest.cc
+++ b/src/librbd/image/CreateRequest.cc
@@ -544,7 +544,7 @@ void CreateRequest<I>::handle_object_map_resize(int r) {
 template<typename I>
 void CreateRequest<I>::fetch_mirror_mode() {
   if ((m_features & RBD_FEATURE_JOURNALING) == 0) {
-    complete(0);
+    mirror_image_enable();
     return;
   }
 
diff --git a/src/test/librbd/image/test_mock_CloneRequest.cc b/src/test/librbd/image/test_mock_CloneRequest.cc
index d21a83a40e3..41b362ab382 100644
--- a/src/test/librbd/image/test_mock_CloneRequest.cc
+++ b/src/test/librbd/image/test_mock_CloneRequest.cc
@@ -917,5 +917,48 @@ TEST_F(TestMockImageCloneRequest, CloseParentError) {
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
 
+TEST_F(TestMockImageCloneRequest, SnapshotMirrorEnableNonPrimary) {
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
+  MockTestImageCtx mock_image_ctx(*image_ctx);
+  expect_op_work_queue(mock_image_ctx);
+
+  InSequence seq;
+  expect_open(mock_image_ctx, 0);
+
+  expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123);
+  expect_is_snap_protected(mock_image_ctx, true, 0);
+
+  MockCreateRequest mock_create_request;
+  expect_create(mock_create_request, 0);
+
+  expect_open(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
+
+  MockAttachChildRequest mock_attach_child_request;
+  expect_attach_child(mock_attach_child_request, 2, 0);
+
+  MockMetadataCopyRequest mock_request;
+  expect_metadata_copy(mock_request, 0);
+
+  MockMirrorEnableRequest mock_mirror_enable_request;
+  expect_mirror_enable(mock_mirror_enable_request, 0);
+
+  expect_close(mock_image_ctx, 0);
+  expect_close(mock_image_ctx, 0);
+
+  C_SaferCond ctx;
+  ImageOptions clone_opts;
+  auto req = new MockCloneRequest(m_cct->_conf, m_ioctx, "parent id", "", {}, 123,
+                                  m_ioctx, "clone name", "clone id", clone_opts,
+                                  cls::rbd::MIRROR_IMAGE_MODE_JOURNAL,
+                                  "global image id", "primary mirror uuid",
+                                  image_ctx->op_work_queue, &ctx);
+  req->send();
+  ASSERT_EQ(0, ctx.wait());
+}
+
 } // namespace image
 } // namespace librbd
-- 
2.39.5