From: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Date: Fri, 4 Jun 2021 16:29:37 +0000 (+0200)
Subject: rbd-mirror: fix mirror image removal
X-Git-Tag: v16.2.7~65^2~10
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2adf2ee5cc4d9772868846ca203a32c7459497fb;p=ceph.git

rbd-mirror: fix mirror image removal

Invoke ImageRemoveRequest instead of calling directly
mirror_image_remove so that the MirrroringWatcher can pick up local
image deletion.

Fixes: https://tracker.ceph.com/issues/51031
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
(cherry picked from commit 34082b7ee48a33e566348395395858e1e0db3013)
---

diff --git a/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc b/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc
index c3702a9cc33b..e1a52c87602c 100644
--- a/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc
+++ b/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc
@@ -9,6 +9,7 @@
 #include "librbd/TrashWatcher.h"
 #include "librbd/journal/ResetRequest.h"
 #include "librbd/mirror/GetInfoRequest.h"
+#include "librbd/mirror/ImageRemoveRequest.h"
 #include "librbd/trash/MoveRequest.h"
 #include "tools/rbd_mirror/Threads.h"
 #include "tools/rbd_mirror/image_deleter/TrashMoveRequest.h"
@@ -131,6 +132,37 @@ struct GetInfoRequest<librbd::MockTestImageCtx> {
 
 GetInfoRequest<librbd::MockTestImageCtx>* GetInfoRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
 
+template<>
+struct ImageRemoveRequest<librbd::MockTestImageCtx> {
+  static ImageRemoveRequest* s_instance;
+  std::string global_image_id;
+  std::string image_id;
+  Context* on_finish;
+
+  static ImageRemoveRequest *create(librados::IoCtx& io_ctx,
+                                    const std::string& global_image_id,
+                                    const std::string& image_id,
+                                    Context* on_finish) {
+    ceph_assert(s_instance != nullptr);
+    s_instance->global_image_id = global_image_id;
+    s_instance->image_id = image_id;
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  ImageRemoveRequest() {
+    ceph_assert(s_instance == nullptr);
+    s_instance = this;
+  }
+  ~ImageRemoveRequest() {
+    s_instance = nullptr;
+  }
+
+  MOCK_METHOD0(send, void());
+};
+
+ImageRemoveRequest<librbd::MockTestImageCtx>* ImageRemoveRequest<librbd::MockTestImageCtx>::s_instance = nullptr;
+
 } // namespace mirror
 namespace trash {
 
@@ -185,6 +217,7 @@ public:
   typedef TrashMoveRequest<librbd::MockTestImageCtx> MockTrashMoveRequest;
   typedef librbd::journal::ResetRequest<librbd::MockTestImageCtx> MockJournalResetRequest;
   typedef librbd::mirror::GetInfoRequest<librbd::MockTestImageCtx> MockGetMirrorInfoRequest;
+  typedef librbd::mirror::ImageRemoveRequest<librbd::MockTestImageCtx> MockImageRemoveRequest;
   typedef librbd::trash::MoveRequest<librbd::MockTestImageCtx> MockLibrbdTrashMoveRequest;
   typedef librbd::TrashWatcher<librbd::MockTestImageCtx> MockTrashWatcher;
 
@@ -273,11 +306,12 @@ public:
       .WillOnce(Return(r));
   }
 
-  void expect_mirror_image_remove(librados::IoCtx &ioctx, int r) {
-    EXPECT_CALL(get_mock_io_ctx(ioctx),
-                exec(StrEq("rbd_mirroring"), _, StrEq("rbd"),
-                     StrEq("mirror_image_remove"), _, _, _, _))
-      .WillOnce(Return(r));
+  void expect_mirror_image_remove_request(
+      MockImageRemoveRequest& mock_image_remove_request, int r) {
+    EXPECT_CALL(mock_image_remove_request, send())
+      .WillOnce(Invoke([this, &mock_image_remove_request, r]() {
+                  m_threads->work_queue->queue(mock_image_remove_request.on_finish, r);
+                }));
   }
 
   void expect_journal_reset(MockJournalResetRequest& mock_journal_reset_request,
@@ -355,7 +389,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, SuccessJournal) {
   MockLibrbdTrashMoveRequest mock_librbd_trash_move_request;
   expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id",
                     {}, 0);
-  expect_mirror_image_remove(m_local_io_ctx, 0);
+  MockImageRemoveRequest mock_image_remove_request;
+  expect_mirror_image_remove_request(mock_image_remove_request, 0);
 
   expect_close(mock_image_ctx, 0);
 
@@ -396,7 +431,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, SuccessSnapshot) {
   MockLibrbdTrashMoveRequest mock_librbd_trash_move_request;
   expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id",
                     {}, 0);
-  expect_mirror_image_remove(m_local_io_ctx, 0);
+  MockImageRemoveRequest mock_image_remove_request;
+  expect_mirror_image_remove_request(mock_image_remove_request, 0);
 
   expect_close(mock_image_ctx, 0);
 
@@ -742,7 +778,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, RemoveMirrorImageError) {
   MockLibrbdTrashMoveRequest mock_librbd_trash_move_request;
   expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id",
                     {}, 0);
-  expect_mirror_image_remove(m_local_io_ctx, -EINVAL);
+  MockImageRemoveRequest mock_image_remove_request;
+  expect_mirror_image_remove_request(mock_image_remove_request, -EINVAL);
 
   expect_close(mock_image_ctx, 0);
 
@@ -791,7 +828,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, CloseImageError) {
   MockLibrbdTrashMoveRequest mock_librbd_trash_move_request;
   expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id",
                     {}, 0);
-  expect_mirror_image_remove(m_local_io_ctx, 0);
+  MockImageRemoveRequest mock_image_remove_request;
+  expect_mirror_image_remove_request(mock_image_remove_request, 0);
 
   expect_close(mock_image_ctx, -EINVAL);
 
@@ -842,7 +880,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, DelayedDelation) {
   expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id",
                     600, 0);
 
-  expect_mirror_image_remove(m_local_io_ctx, 0);
+  MockImageRemoveRequest mock_image_remove_request;
+  expect_mirror_image_remove_request(mock_image_remove_request, 0);
   expect_close(mock_image_ctx, 0);
 
   MockTrashWatcher mock_trash_watcher;
diff --git a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
index 7f718cb9c66e..234bb6912966 100644
--- a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
+++ b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
@@ -15,6 +15,7 @@
 #include "librbd/Utils.h"
 #include "librbd/asio/ContextWQ.h"
 #include "librbd/journal/ResetRequest.h"
+#include "librbd/mirror/ImageRemoveRequest.h"
 #include "librbd/mirror/GetInfoRequest.h"
 #include "librbd/trash/MoveRequest.h"
 #include "tools/rbd_mirror/image_deleter/Types.h"
@@ -315,15 +316,12 @@ template <typename I>
 void TrashMoveRequest<I>::remove_mirror_image() {
   dout(10) << dendl;
 
-  librados::ObjectWriteOperation op;
-  librbd::cls_client::mirror_image_remove(&op, m_image_id);
-
-  auto aio_comp = create_rados_callback<
+  auto ctx = create_context_callback<
     TrashMoveRequest<I>,
     &TrashMoveRequest<I>::handle_remove_mirror_image>(this);
-  int r = m_io_ctx.aio_operate(RBD_MIRRORING, aio_comp, &op);
-  ceph_assert(r == 0);
-  aio_comp->release();
+  auto req = librbd::mirror::ImageRemoveRequest<I>::create(
+    m_io_ctx, m_global_image_id, m_image_id, ctx);
+  req->send();
 }
 
 template <typename I>