librbd: use newly added method to list watchers in remove request

Signed-off-by: Mykola Golub <mgolub@suse.com>

diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc
index 09564cca81a641589c65808ba36bf43a628101a7..76fbf8d0df57755cc59e38c7f96396b474f92aca 100644 (file)
--- a/src/librbd/image/RemoveRequest.cc
+++ b/src/librbd/image/RemoveRequest.cc
@@ -11,6 +11,7 @@
 #include "librbd/ExclusiveLock.h"
 #include "librbd/MirroringWatcher.h"
 #include "librbd/image/DetachChildRequest.h"
+#include "librbd/image/ListWatchersRequest.h"
 #include "librbd/journal/DisabledPolicy.h"
 #include "librbd/journal/RemoveRequest.h"
 #include "librbd/mirror/DisableRequest.h"
@@ -229,26 +230,21 @@ template<typename I>
 void RemoveRequest<I>::list_image_watchers() {
   ldout(m_cct, 20) << dendl;
 
-  librados::ObjectReadOperation op;
-  op.list_watchers(&m_watchers, &m_ret_val);
-
+  int flags = LIST_WATCHERS_FILTER_OUT_MY_INSTANCE |
+              LIST_WATCHERS_FILTER_OUT_MIRROR_INSTANCES;
   using klass = RemoveRequest<I>;
-  librados::AioCompletion *rados_completion =
-    create_rados_callback<klass, &klass::handle_list_image_watchers>(this);
+  Context *ctx = create_context_callback<
+    klass, &klass::handle_list_image_watchers>(this);
 
-  int r = m_image_ctx->md_ctx.aio_operate(m_header_oid, rados_completion,
-                                          &op, &m_out_bl);
-  assert(r == 0);
-  rados_completion->release();
+  auto req = ListWatchersRequest<I>::create(*m_image_ctx, flags, &m_watchers,
+                                            ctx);
+  req->send();
 }
 
 template<typename I>
 void RemoveRequest<I>::handle_list_image_watchers(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (r == 0 && m_ret_val < 0) {
-    r = m_ret_val;
-  }
   if (r < 0) {
     lderr(m_cct) << "error listing image watchers: " << cpp_strerror(r)
                  << dendl;
@@ -256,86 +252,12 @@ void RemoveRequest<I>::handle_list_image_watchers(int r) {
     return;
   }
 
-  get_mirror_image();
-}
-
-template<typename I>
-void RemoveRequest<I>::get_mirror_image() {
-  ldout(m_cct, 20) << dendl;
-  if ((m_watchers.empty()) ||
-      ((m_image_ctx->features & RBD_FEATURE_JOURNALING) == 0)) {
-    check_image_watchers();
-    return;
-  }
-
-  librados::ObjectReadOperation op;
-  cls_client::mirror_image_get_start(&op, m_image_id);
-
-  using klass = RemoveRequest<I>;
-  librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_get_mirror_image>(this);
-  m_out_bl.clear();
-  int r = m_image_ctx->md_ctx.aio_operate(RBD_MIRRORING, comp, &op, &m_out_bl);
-  assert(r == 0);
-  comp->release();
-}
-
-template<typename I>
-void RemoveRequest<I>::handle_get_mirror_image(int r) {
-  ldout(m_cct, 20) << "r=" << r << dendl;
-
-  if (r == -ENOENT || r == -EOPNOTSUPP) {
-    check_image_watchers();
-    return;
-  } else if (r < 0) {
-    ldout(m_cct, 5) << "error retrieving mirror image: " << cpp_strerror(r)
-                    << dendl;
-  }
-
-  list_mirror_watchers();
-}
-
-template<typename I>
-void RemoveRequest<I>::list_mirror_watchers() {
-  ldout(m_cct, 20) << dendl;
-
-  librados::ObjectReadOperation op;
-  op.list_watchers(&m_mirror_watchers, &m_ret_val);
-
-  using klass = RemoveRequest<I>;
-  librados::AioCompletion *rados_completion =
-    create_rados_callback<klass, &klass::handle_list_mirror_watchers>(this);
-  m_out_bl.clear();
-  int r = m_image_ctx->md_ctx.aio_operate(RBD_MIRRORING, rados_completion,
-                                          &op, &m_out_bl);
-  assert(r == 0);
-  rados_completion->release();
-}
-
-template<typename I>
-void RemoveRequest<I>::handle_list_mirror_watchers(int r) {
-  ldout(m_cct, 20) << "r=" << r << dendl;
-
-  if (r == 0 && m_ret_val < 0) {
-    r = m_ret_val;
-  }
-  if (r < 0 && r != -ENOENT) {
-    ldout(m_cct, 5) << "error listing mirror watchers: " << cpp_strerror(r)
-                    << dendl;
-  }
-
-  for (auto &watcher : m_mirror_watchers) {
-    m_watchers.remove_if([watcher] (obj_watch_t &w) {
-        return (strncmp(w.addr, watcher.addr, sizeof(w.addr)) == 0);
-      });
-  }
-
   check_image_watchers();
 }
 
 template<typename I>
 void RemoveRequest<I>::check_image_watchers() {
-  if (m_watchers.size() > 1) {
+  if (!m_watchers.empty()) {
     lderr(m_cct) << "image has watchers - not removing" << dendl;
     send_close_image(-EBUSY);
     return;

diff --git a/src/librbd/image/RemoveRequest.h b/src/librbd/image/RemoveRequest.h
index 803bd440c2d24c983e9d9ede4b1f1bf2cfb0cb09..0bb95aa09999c12a71c77fa996e335925d716a73 100644 (file)
--- a/src/librbd/image/RemoveRequest.h
+++ b/src/librbd/image/RemoveRequest.h
@@ -8,6 +8,8 @@
 #include "librbd/ImageCtx.h"
 #include "librbd/image/TypeTraits.h"
 
+#include <list>
+
 class Context;
 class ContextWQ;
 class SafeTimer;
@@ -142,7 +144,6 @@ private:
   int m_ret_val = 0;
   bufferlist m_out_bl;
   std::list<obj_watch_t> m_watchers;
-  std::list<obj_watch_t> m_mirror_watchers;
 
   std::map<uint64_t, SnapInfo> m_snap_infos;
 
@@ -170,12 +171,6 @@ private:
   void list_image_watchers();
   void handle_list_image_watchers(int r);
 
-  void get_mirror_image();
-  void handle_get_mirror_image(int r);
-
-  void list_mirror_watchers();
-  void handle_list_mirror_watchers(int r);
-
   void check_image_watchers();
 
   void check_group();

diff --git a/src/test/librbd/image/test_mock_RemoveRequest.cc b/src/test/librbd/image/test_mock_RemoveRequest.cc
index 41695a801979d360ff657b2fd6c2119549b96e0b..47a637740339124321c0c8eb518545f3247717aa 100644 (file)
--- a/src/test/librbd/image/test_mock_RemoveRequest.cc
+++ b/src/test/librbd/image/test_mock_RemoveRequest.cc
@@ -12,6 +12,7 @@
 #include "librbd/Operations.h"
 #include "librbd/image/TypeTraits.h"
 #include "librbd/image/DetachChildRequest.h"
+#include "librbd/image/ListWatchersRequest.h"
 #include "librbd/image/RemoveRequest.h"
 #include "librbd/image/RefreshParentRequest.h"
 #include "librbd/journal/RemoveRequest.h"
@@ -182,6 +183,33 @@ public:
 DisableRequest<MockTestImageCtx> *DisableRequest<MockTestImageCtx>::s_instance;
 
 } // namespace mirror
+
+namespace image {
+
+template<>
+class ListWatchersRequest<MockTestImageCtx> {
+public:
+  static ListWatchersRequest *s_instance;
+  Context *on_finish = nullptr;
+
+  static ListWatchersRequest *create(MockTestImageCtx &image_ctx, int flags,
+                                     std::list<obj_watch_t> *watchers,
+                                     Context *on_finish) {
+    assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  ListWatchersRequest() {
+    s_instance = this;
+  }
+
+  MOCK_METHOD0(send, void());
+};
+
+ListWatchersRequest<MockTestImageCtx> *ListWatchersRequest<MockTestImageCtx>::s_instance;
+
+} // namespace image
 } // namespace librbd
 
 // template definitions
@@ -217,6 +245,7 @@ public:
   typedef typename TypeTraits::ContextWQ ContextWQ;
   typedef RemoveRequest<MockTestImageCtx> MockRemoveRequest;
   typedef DetachChildRequest<MockTestImageCtx> MockDetachChildRequest;
+  typedef ListWatchersRequest<MockTestImageCtx> MockListWatchersRequest;
   typedef librbd::operation::SnapshotRemoveRequest<MockTestImageCtx> MockSnapshotRemoveRequest;
   typedef librbd::operation::TrimRequest<MockTestImageCtx> MockTrimRequest;
   typedef librbd::journal::RemoveRequest<MockTestImageCtx> MockJournalRemoveRequest;
@@ -264,6 +293,13 @@ public:
                 }));
   }
 
+  void expect_list_image_watchers(
+    MockTestImageCtx &mock_image_ctx,
+    MockListWatchersRequest &mock_list_watchers_request, int r) {
+    EXPECT_CALL(mock_list_watchers_request, send())
+      .WillOnce(FinishRequest(&mock_list_watchers_request, r, &mock_image_ctx));
+  }
+
   void expect_get_group(MockTestImageCtx &mock_image_ctx, int r) {
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(mock_image_ctx.header_oid, _, StrEq("rbd"),
@@ -307,15 +343,6 @@ public:
       .WillOnce(Return(r));
   }
 
-  void expect_mirror_image_get(MockTestImageCtx &mock_image_ctx, int r) {
-    if ((mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0ULL) {
-      EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
-                  exec(RBD_MIRRORING, _, StrEq("rbd"),
-                       StrEq("mirror_image_get"), _, _, _))
-        .WillOnce(Return(r));
-    }
-  }
-
   void expect_dir_remove_image(librados::IoCtx &ioctx, int r) {
     EXPECT_CALL(get_mock_io_ctx(ioctx),
                 exec(RBD_DIRECTORY, _, StrEq("rbd"), StrEq("dir_remove_image"),
@@ -365,6 +392,9 @@ TEST_F(TestMockImageRemoveRequest, SuccessV1) {
   InSequence seq;
   expect_state_open(*m_mock_imctx, 0);
 
+  MockListWatchersRequest mock_list_watchers_request;
+  expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0);
+
   MockTrimRequest mock_trim_request;
   expect_trim(*m_mock_imctx, mock_trim_request, 0);
 
@@ -421,7 +451,9 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV1) {
   expect_set_journal_policy(*m_mock_imctx);
   expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0);
 
-  expect_mirror_image_get(*m_mock_imctx, 0);
+  MockListWatchersRequest mock_list_watchers_request;
+  expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0);
+
   expect_get_group(*m_mock_imctx, 0);
 
   MockTrimRequest mock_trim_request;
@@ -472,7 +504,9 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV2) {
   expect_set_journal_policy(*m_mock_imctx);
   expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0);
 
-  expect_mirror_image_get(*m_mock_imctx, 0);
+  MockListWatchersRequest mock_list_watchers_request;
+  expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0);
+
   expect_get_group(*m_mock_imctx, 0);
 
   MockTrimRequest mock_trim_request;
@@ -523,7 +557,9 @@ TEST_F(TestMockImageRemoveRequest, NotExistsV2) {
   expect_set_journal_policy(*m_mock_imctx);
   expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0);
 
-  expect_mirror_image_get(*m_mock_imctx, 0);
+  MockListWatchersRequest mock_list_watchers_request;
+  expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0);
+
   expect_get_group(*m_mock_imctx, 0);
 
   MockTrimRequest mock_trim_request;
@@ -612,7 +648,9 @@ TEST_F(TestMockImageRemoveRequest, AutoDeleteSnapshots) {
   expect_set_journal_policy(*m_mock_imctx);
   expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0);
 
-  expect_mirror_image_get(*m_mock_imctx, 0);
+  MockListWatchersRequest mock_list_watchers_request;
+  expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0);
+
   expect_get_group(*m_mock_imctx, 0);
 
   MockSnapshotRemoveRequest mock_snap_remove_request;