Merge pull request #16113 from smithfarm/wip-19807-kraken

kraken: tests: remove hard-coded image name from TestLibRBD.Mirror

Reviewed-by: Jason Dillaman <dillaman@redhat.com>
Reviewed-by: Mykola Golub <mgolub@mirantis.com>

src/librbd/internal.cc

@@ -2478,7 +2478,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     return cls_client::metadata_list(&ictx->md_ctx, ictx->header_oid, start, max, pairs);
   }
 
-  int mirror_image_enable(ImageCtx *ictx) {
+  int mirror_image_enable(ImageCtx *ictx, bool relax_same_pool_parent_check) {
     CephContext *cct = ictx->cct;
     ldout(cct, 20) << "mirror_image_enable " << ictx << dendl;
 
@@ -2501,16 +2501,25 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       return -EINVAL;
     }
 
-    // is mirroring not enabled for the parent? 
+    // is mirroring not enabled for the parent?
     {
       RWLock::RLocker l(ictx->parent_lock);
       ImageCtx *parent = ictx->parent;
-      if(parent) {
-        cls::rbd::MirrorImage mirror_image_internal;
-        r = cls_client::mirror_image_get(&(parent->md_ctx), parent->id, &mirror_image_internal);
-        if (r == -ENOENT) {
-          lderr(cct) << "mirroring is not enabled for the parent" << dendl;
-          return -EINVAL;
+      if (parent) {
+        if (relax_same_pool_parent_check &&
+            parent->md_ctx.get_id() == ictx->md_ctx.get_id()) {
+          if (!parent->test_features(RBD_FEATURE_JOURNALING)) {
+            lderr(cct) << "journaling is not enabled for the parent" << dendl;
+            return -EINVAL;
+          }
+        } else {
+          cls::rbd::MirrorImage mirror_image_internal;
+          r = cls_client::mirror_image_get(&(parent->md_ctx), parent->id,
+                                           &mirror_image_internal);
+          if (r == -ENOENT) {
+            lderr(cct) << "mirroring is not enabled for the parent" << dendl;
+            return -EINVAL;
+          }
         }
       }
     }
@@ -3025,7 +3034,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
             return r;
           }
 
-          r = mirror_image_enable(img_ctx);
+          r = mirror_image_enable(img_ctx, true);
           if (r < 0) {
             lderr(cct) << "error enabling mirroring for image "
                        << img_pair.first << ": " << cpp_strerror(r) << dendl;

src/librbd/internal.h

@@ -214,7 +214,7 @@ namespace librbd {
   int mirror_image_status_summary(IoCtx& io_ctx,
       std::map<mirror_image_status_state_t, int> *states);
 
-  int mirror_image_enable(ImageCtx *ictx);
+  int mirror_image_enable(ImageCtx *ictx, bool relax_same_pool_parent_check);
   int mirror_image_disable(ImageCtx *ictx, bool force);
   int mirror_image_promote(ImageCtx *ictx, bool force);
   int mirror_image_demote(ImageCtx *ictx);

src/librbd/librbd.cc

@@ -1380,7 +1380,7 @@ namespace librbd {
 
   int Image::mirror_image_enable() {
     ImageCtx *ictx = (ImageCtx *)ctx;
-    return librbd::mirror_image_enable(ictx);
+    return librbd::mirror_image_enable(ictx, false);
   }
 
   int Image::mirror_image_disable(bool force) {
@@ -2967,7 +2967,7 @@ extern "C" int rbd_metadata_list(rbd_image_t image, const char *start, uint64_t
 extern "C" int rbd_mirror_image_enable(rbd_image_t image)
 {
   librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
-  return librbd::mirror_image_enable(ictx);
+  return librbd::mirror_image_enable(ictx, false);
 }
 
 extern "C" int rbd_mirror_image_disable(rbd_image_t image, bool force)

src/test/librbd/test_librbd.cc

@@ -4979,6 +4979,28 @@ TEST_F(TestLibRBD, Mirror) {
 
   ASSERT_EQ(0, rbd.mirror_mode_set(ioctx, RBD_MIRROR_MODE_IMAGE));
   ASSERT_EQ(0, rbd.mirror_mode_get(ioctx, &mirror_mode));
+
+  // Add some images to the pool
+  int order = 0;
+  std::string parent_name = get_temp_image_name();
+  std::string child_name = get_temp_image_name();
+  ASSERT_EQ(0, create_image_pp(rbd, ioctx, parent_name.c_str(), 2 << 20,
+                               &order));
+  bool old_format;
+  uint64_t features;
+  ASSERT_EQ(0, get_features(&old_format, &features));
+  if ((features & RBD_FEATURE_LAYERING) != 0) {
+    librbd::Image parent;
+    ASSERT_EQ(0, rbd.open(ioctx, parent, parent_name.c_str(), NULL));
+    ASSERT_EQ(0, parent.snap_create("parent_snap"));
+    ASSERT_EQ(0, parent.close());
+    ASSERT_EQ(0, rbd.open(ioctx, parent, parent_name.c_str(), "parent_snap"));
+    ASSERT_EQ(0, parent.snap_protect("parent_snap"));
+    ASSERT_EQ(0, parent.close());
+    ASSERT_EQ(0, rbd.clone(ioctx, parent_name.c_str(), "parent_snap", ioctx,
+                           child_name.c_str(), features, &order));
+  }
+
   ASSERT_EQ(RBD_MIRROR_MODE_IMAGE, mirror_mode);
 
   ASSERT_EQ(0, rbd.mirror_mode_set(ioctx, RBD_MIRROR_MODE_POOL));

src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc

@@ -9,6 +9,7 @@
 #include "tools/rbd_mirror/image_replayer/BootstrapRequest.h"
 #include "tools/rbd_mirror/image_replayer/CloseImageRequest.h"
 #include "tools/rbd_mirror/image_replayer/CreateImageRequest.h"
+#include "tools/rbd_mirror/image_replayer/IsPrimaryRequest.h"
 #include "tools/rbd_mirror/image_replayer/OpenImageRequest.h"
 #include "tools/rbd_mirror/image_replayer/OpenLocalImageRequest.h"
 #include "test/journal/mock/MockJournaler.h"
@@ -149,6 +150,31 @@ struct CreateImageRequest<librbd::MockTestImageCtx> {
   MOCK_METHOD0(send, void());
 };
 
+template<>
+struct IsPrimaryRequest<librbd::MockTestImageCtx> {
+  static IsPrimaryRequest* s_instance;
+  bool *primary = nullptr;
+  Context *on_finish = nullptr;
+
+  static IsPrimaryRequest* create(librbd::MockTestImageCtx *image_ctx,
+                                  bool *primary, Context *on_finish) {
+    assert(s_instance != nullptr);
+    s_instance->primary = primary;
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  IsPrimaryRequest() {
+    assert(s_instance == nullptr);
+    s_instance = this;
+  }
+  ~IsPrimaryRequest() {
+    s_instance = nullptr;
+  }
+
+  MOCK_METHOD0(send, void());
+};
+
 template<>
 struct OpenImageRequest<librbd::MockTestImageCtx> {
   static OpenImageRequest* s_instance;
@@ -216,6 +242,8 @@ CloseImageRequest<librbd::MockTestImageCtx>*
   CloseImageRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
 CreateImageRequest<librbd::MockTestImageCtx>*
   CreateImageRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
+IsPrimaryRequest<librbd::MockTestImageCtx>*
+  IsPrimaryRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
 OpenImageRequest<librbd::MockTestImageCtx>*
   OpenImageRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
 OpenLocalImageRequest<librbd::MockTestImageCtx>*
@@ -251,6 +279,7 @@ class TestMockImageReplayerBootstrapRequest : public TestMockFixture {
   typedef ImageSyncThrottlerRef<librbd::MockTestImageCtx> MockImageSyncThrottler;
   typedef BootstrapRequest<librbd::MockTestImageCtx> MockBootstrapRequest;
   typedef CloseImageRequest<librbd::MockTestImageCtx> MockCloseImageRequest;
+  typedef IsPrimaryRequest<librbd::MockTestImageCtx> MockIsPrimaryRequest;
   typedef OpenImageRequest<librbd::MockTestImageCtx> MockOpenImageRequest;
   typedef OpenLocalImageRequest<librbd::MockTestImageCtx> MockOpenLocalImageRequest;
   typedef std::list<cls::journal::Tag> Tags;
@@ -358,11 +387,13 @@ class TestMockImageReplayerBootstrapRequest : public TestMockFixture {
         }));
   }
 
-  void expect_journal_is_tag_owner(librbd::MockJournal &mock_journal,
-                                   bool is_owner, int r) {
-    EXPECT_CALL(mock_journal, is_tag_owner(_))
-      .WillOnce(DoAll(SetArgPointee<0>(is_owner),
-                      Return(r)));
+  void expect_is_primary(MockIsPrimaryRequest &mock_is_primary_request,
+			 bool primary, int r) {
+    EXPECT_CALL(mock_is_primary_request, send())
+      .WillOnce(Invoke([this, &mock_is_primary_request, primary, r]() {
+          *mock_is_primary_request.primary = primary;
+          m_threads->work_queue->queue(mock_is_primary_request.on_finish, r);
+        }));
   }
 
   void expect_journal_get_tag_tid(librbd::MockJournal &mock_journal,
@@ -455,7 +486,8 @@ TEST_F(TestMockImageReplayerBootstrapRequest, NonPrimaryRemoteSyncingState) {
   MockOpenImageRequest mock_open_image_request;
   expect_open_image(mock_open_image_request, m_remote_io_ctx,
                     mock_remote_image_ctx.id, mock_remote_image_ctx, 0);
-  expect_journal_is_tag_owner(mock_journal, false, 0);
+  MockIsPrimaryRequest mock_is_primary_request;
+  expect_is_primary(mock_is_primary_request, false, 0);
 
   // switch the state to replaying
   mirror_peer_client_meta.state = librbd::journal::MIRROR_PEER_STATE_REPLAYING;
@@ -512,7 +544,8 @@ TEST_F(TestMockImageReplayerBootstrapRequest, RemoteDemotePromote) {
   MockOpenImageRequest mock_open_image_request;
   expect_open_image(mock_open_image_request, m_remote_io_ctx,
                     mock_remote_image_ctx.id, mock_remote_image_ctx, 0);
-  expect_journal_is_tag_owner(mock_journal, true, 0);
+  MockIsPrimaryRequest mock_is_primary_request;
+  expect_is_primary(mock_is_primary_request, true, 0);
 
   // open the local image
   mock_local_image_ctx.journal = &mock_journal;
@@ -590,7 +623,8 @@ TEST_F(TestMockImageReplayerBootstrapRequest, MultipleRemoteDemotePromotes) {
   MockOpenImageRequest mock_open_image_request;
   expect_open_image(mock_open_image_request, m_remote_io_ctx,
                     mock_remote_image_ctx.id, mock_remote_image_ctx, 0);
-  expect_journal_is_tag_owner(mock_journal, true, 0);
+  MockIsPrimaryRequest mock_is_primary_request;
+  expect_is_primary(mock_is_primary_request, true, 0);
 
   // open the local image
   mock_local_image_ctx.journal = &mock_journal;
@@ -678,7 +712,8 @@ TEST_F(TestMockImageReplayerBootstrapRequest, LocalDemoteRemotePromote) {
   MockOpenImageRequest mock_open_image_request;
   expect_open_image(mock_open_image_request, m_remote_io_ctx,
                     mock_remote_image_ctx.id, mock_remote_image_ctx, 0);
-  expect_journal_is_tag_owner(mock_journal, true, 0);
+  MockIsPrimaryRequest mock_is_primary_request;
+  expect_is_primary(mock_is_primary_request, true, 0);
 
   // open the local image
   mock_local_image_ctx.journal = &mock_journal;
@@ -754,7 +789,8 @@ TEST_F(TestMockImageReplayerBootstrapRequest, SplitBrainForcePromote) {
   MockOpenImageRequest mock_open_image_request;
   expect_open_image(mock_open_image_request, m_remote_io_ctx,
                     mock_remote_image_ctx.id, mock_remote_image_ctx, 0);
-  expect_journal_is_tag_owner(mock_journal, true, 0);
+  MockIsPrimaryRequest mock_is_primary_request;
+  expect_is_primary(mock_is_primary_request, true, 0);
 
   // open the local image
   mock_local_image_ctx.journal = &mock_journal;

src/test/rbd_mirror/test_ImageReplayer.cc

@@ -99,6 +99,7 @@ class TestImageReplayer : public ::rbd::mirror::TestFixture {
 
     EXPECT_EQ(0, m_remote_cluster.ioctx_create(m_remote_pool_name.c_str(),
 					       m_remote_ioctx));
+    EXPECT_EQ(0, librbd::mirror_mode_set(m_remote_ioctx, RBD_MIRROR_MODE_POOL));
 
     m_image_name = get_temp_image_name();
     uint64_t features = librbd::util::get_rbd_default_features(g_ceph_context);
@@ -392,7 +393,7 @@ TEST_F(TestImageReplayer, BootstrapErrorLocalImageExists)
 
 TEST_F(TestImageReplayer, BootstrapErrorNoJournal)
 {
-  // disable remote journal journaling
+  // disable remote image journaling
   librbd::ImageCtx *ictx;
   open_remote_image(&ictx);
   uint64_t features;
@@ -407,6 +408,58 @@ TEST_F(TestImageReplayer, BootstrapErrorNoJournal)
   ASSERT_EQ(-ENOENT, cond.wait());
 }
 
+TEST_F(TestImageReplayer, BootstrapErrorMirrorDisabled)
+{
+  // disable remote image mirroring
+  ASSERT_EQ(0, librbd::mirror_mode_set(m_remote_ioctx, RBD_MIRROR_MODE_IMAGE));
+  librbd::ImageCtx *ictx;
+  open_remote_image(&ictx);
+  ASSERT_EQ(0, librbd::mirror_image_disable(ictx, true));
+  close_image(ictx);
+
+  create_replayer<>();
+  C_SaferCond cond;
+  m_replayer->start(&cond);
+  ASSERT_EQ(-ENOENT, cond.wait());
+}
+
+TEST_F(TestImageReplayer, BootstrapMirrorDisabling)
+{
+  // set remote image mirroring state to DISABLING
+  ASSERT_EQ(0, librbd::mirror_mode_set(m_remote_ioctx, RBD_MIRROR_MODE_IMAGE));
+  librbd::ImageCtx *ictx;
+  open_remote_image(&ictx);
+  ASSERT_EQ(0, librbd::mirror_image_enable(ictx, false));
+  cls::rbd::MirrorImage mirror_image;
+  ASSERT_EQ(0, librbd::cls_client::mirror_image_get(&m_remote_ioctx, ictx->id,
+                                                    &mirror_image));
+  mirror_image.state = cls::rbd::MirrorImageState::MIRROR_IMAGE_STATE_DISABLING;
+  ASSERT_EQ(0, librbd::cls_client::mirror_image_set(&m_remote_ioctx, ictx->id,
+                                                    mirror_image));
+  close_image(ictx);
+
+  create_replayer<>();
+  C_SaferCond cond;
+  m_replayer->start(&cond);
+  ASSERT_EQ(0, cond.wait());
+  ASSERT_TRUE(m_replayer->is_stopped());
+}
+
+TEST_F(TestImageReplayer, BootstrapDemoted)
+{
+  // demote remote image
+  librbd::ImageCtx *ictx;
+  open_remote_image(&ictx);
+  ASSERT_EQ(0, librbd::mirror_image_demote(ictx));
+  close_image(ictx);
+
+  create_replayer<>();
+  C_SaferCond cond;
+  m_replayer->start(&cond);
+  ASSERT_EQ(0, cond.wait());
+  ASSERT_TRUE(m_replayer->is_stopped());
+}
+
 TEST_F(TestImageReplayer, StartInterrupted)
 {
   create_replayer<>();

src/tools/rbd_mirror/CMakeLists.txt

@@ -13,6 +13,7 @@ set(rbd_mirror_internal
   image_replayer/CloseImageRequest.cc
   image_replayer/CreateImageRequest.cc
   image_replayer/EventPreprocessor.cc
+  image_replayer/IsPrimaryRequest.cc
   image_replayer/OpenImageRequest.cc
   image_replayer/OpenLocalImageRequest.cc
   image_replayer/ReplayStatusFormatter.cc

src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc

@@ -5,6 +5,7 @@
 #include "BootstrapRequest.h"
 #include "CloseImageRequest.h"
 #include "CreateImageRequest.h"
+#include "IsPrimaryRequest.h"
 #include "OpenImageRequest.h"
 #include "OpenLocalImageRequest.h"
 #include "common/debug.h"
@@ -257,11 +258,6 @@ void BootstrapRequest<I>::open_remote_image() {
 
 template <typename I>
 void BootstrapRequest<I>::handle_open_remote_image(int r) {
-  // deduce the class type for the journal to support unit tests
-  using Journal = typename std::decay<
-    typename std::remove_pointer<decltype(std::declval<I>().journal)>
-    ::type>::type;
-
   dout(20) << ": r=" << r << dendl;
 
   if (r < 0) {
@@ -271,18 +267,36 @@ void BootstrapRequest<I>::handle_open_remote_image(int r) {
     return;
   }
 
-  // TODO: make async
-  bool tag_owner;
-  r = Journal::is_tag_owner(m_remote_image_ctx, &tag_owner);
+  is_primary();
+}
+
+template <typename I>
+void BootstrapRequest<I>::is_primary() {
+  dout(20) << dendl;
+
+  update_progress("OPEN_REMOTE_IMAGE");
+
+  Context *ctx = create_context_callback<
+    BootstrapRequest<I>, &BootstrapRequest<I>::handle_is_primary>(
+      this);
+  IsPrimaryRequest<I> *request = IsPrimaryRequest<I>::create(m_remote_image_ctx,
+                                                             &m_primary, ctx);
+  request->send();
+}
+
+template <typename I>
+void BootstrapRequest<I>::handle_is_primary(int r) {
+  dout(20) << ": r=" << r << dendl;
+
   if (r < 0) {
-    derr << ": failed to query remote image primary status: " << cpp_strerror(r)
+    derr << ": error querying remote image primary status: " << cpp_strerror(r)
          << dendl;
     m_ret_val = r;
     close_remote_image();
     return;
   }
 
-  if (!tag_owner) {
+  if (!m_primary) {
     dout(5) << ": remote image is not primary -- skipping image replay"
             << dendl;
     m_ret_val = -EREMOTEIO;