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)

 Conflicts:
	src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc
- Trivial conflict resolution

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 2630915115d0366fe9a9bdce59c0595516b44a98..49bcbc844ba925875e998c95be8bfabd95448785 100644 (file)
--- 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"
@@ -130,6 +131,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 {
 
@@ -184,6 +216,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;
 
@@ -275,11 +308,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,
@@ -357,7 +391,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);
   expect_destroy(mock_image_ctx);
@@ -399,7 +434,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);
   expect_destroy(mock_image_ctx);
@@ -750,7 +786,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);
   expect_destroy(mock_image_ctx);
@@ -800,7 +837,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);
   expect_destroy(mock_image_ctx);
@@ -852,7 +890,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);
   expect_destroy(mock_image_ctx);
 

diff --git a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
index 0eadca7bf2a86eda3f5a03971a3fac1a2ba113a2..5a2dc9c7243a90d9e46526dafafe3739ea55f6a3 100644 (file)
--- a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
+++ b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
@@ -13,6 +13,7 @@
 #include "librbd/TrashWatcher.h"
 #include "librbd/Utils.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"
@@ -309,15 +310,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>