From 44a0d7c839b31d9cccc6b28443830c3ff6d9701b Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 14 Jul 2020 18:38:17 -0400
Subject: [PATCH] librbd: utilize neorados to issue async blacklist request

The librados API does not currently offer an async 'mon_command'
API method. Instead of adding one just to support this effort,
re-use the neorados API to issue an asynchronous 'mon_command'
for blacklisting a client.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
 src/librbd/managed_lock/BreakRequest.cc       | 78 ++++++++++++-------
 src/librbd/managed_lock/BreakRequest.h        |  6 ++
 .../managed_lock/test_mock_BreakRequest.cc    | 23 +++++-
 3 files changed, 74 insertions(+), 33 deletions(-)

diff --git a/src/librbd/managed_lock/BreakRequest.cc b/src/librbd/managed_lock/BreakRequest.cc
index 62605704890..2c8e3c54f01 100644
--- a/src/librbd/managed_lock/BreakRequest.cc
+++ b/src/librbd/managed_lock/BreakRequest.cc
@@ -12,6 +12,7 @@
 #include "librbd/ImageCtx.h"
 #include "librbd/Utils.h"
 #include "librbd/asio/ContextWQ.h"
+#include "librbd/asio/Utils.h"
 #include "librbd/managed_lock/GetLockerRequest.h"
 
 #define dout_subsys ceph_subsys_rbd
@@ -25,29 +26,6 @@ namespace managed_lock {
 using util::create_context_callback;
 using util::create_rados_callback;
 
-namespace {
-
-struct C_BlacklistClient : public Context {
-  librados::IoCtx &ioctx;
-  std::string locker_address;
-  uint32_t expire_seconds;
-  Context *on_finish;
-
-  C_BlacklistClient(librados::IoCtx &ioctx, const std::string &locker_address,
-                    uint32_t expire_seconds, Context *on_finish)
-    : ioctx(ioctx), locker_address(locker_address),
-      expire_seconds(expire_seconds), on_finish(on_finish) {
-  }
-
-  void finish(int r) override {
-    librados::Rados rados(ioctx);
-    r = rados.blacklist_add(locker_address, expire_seconds);
-    on_finish->complete(r);
-  }
-};
-
-} // anonymous namespace
-
 template <typename I>
 BreakRequest<I>::BreakRequest(librados::IoCtx& ioctx,
                               AsioEngine& asio_engine,
@@ -173,13 +151,29 @@ void BreakRequest<I>::send_blacklist() {
     return;
   }
 
-  // TODO: need async version of RadosClient::blacklist_add
-  using klass = BreakRequest<I>;
-  Context *ctx = create_context_callback<klass, &klass::handle_blacklist>(
-    this);
-  m_asio_engine.get_work_queue()->queue(
-    new C_BlacklistClient(m_ioctx, m_locker.address,
-                          m_blacklist_expire_seconds, ctx), 0);
+  entity_addr_t locker_addr;
+  if (!locker_addr.parse(m_locker.address.c_str(), 0)) {
+    lderr(m_cct) << "unable to parse locker address: " << m_locker.address
+                 << dendl;
+    finish(-EINVAL);
+    return;
+  }
+
+  std::stringstream cmd;
+  cmd << "{"
+      << "\"prefix\": \"osd blacklist\", "
+      << "\"blacklistop\": \"add\", "
+      << "\"addr\": \"" << locker_addr << "\"";
+  if (m_blacklist_expire_seconds != 0) {
+    cmd << ", \"expire\": " << m_blacklist_expire_seconds << ".0";
+  }
+  cmd << "}";
+
+  bufferlist in_bl;
+  m_asio_engine.get_rados_api().mon_command(
+    {cmd.str()}, in_bl, nullptr, nullptr,
+    librbd::asio::util::get_callback_adapter(
+      [this](int r) { handle_blacklist(r); }));
 }
 
 template <typename I>
@@ -192,6 +186,30 @@ void BreakRequest<I>::handle_blacklist(int r) {
     finish(r);
     return;
   }
+
+  wait_for_osd_map();
+}
+
+template <typename I>
+void BreakRequest<I>::wait_for_osd_map() {
+  ldout(m_cct, 10) << dendl;
+
+  m_asio_engine.get_rados_api().wait_for_latest_osd_map(
+    librbd::asio::util::get_callback_adapter(
+      [this](int r) { handle_wait_for_osd_map(r); }));
+}
+
+template <typename I>
+void BreakRequest<I>::handle_wait_for_osd_map(int r) {
+  ldout(m_cct, 10) << "r=" << r << dendl;
+
+  if (r < 0) {
+    lderr(m_cct) << "failed to wait for updated OSD map: " << cpp_strerror(r)
+                 << dendl;
+    finish(r);
+    return;
+  }
+
   send_break_lock();
 }
 
diff --git a/src/librbd/managed_lock/BreakRequest.h b/src/librbd/managed_lock/BreakRequest.h
index 640c0146e07..3c812999810 100644
--- a/src/librbd/managed_lock/BreakRequest.h
+++ b/src/librbd/managed_lock/BreakRequest.h
@@ -58,6 +58,9 @@ private:
    * BLACKLIST (skip if disabled)
    *    |
    *    v
+   * WAIT_FOR_OSD_MAP
+   *    |
+   *    v
    * BREAK_LOCK
    *    |
    *    v
@@ -99,6 +102,9 @@ private:
   void send_blacklist();
   void handle_blacklist(int r);
 
+  void wait_for_osd_map();
+  void handle_wait_for_osd_map(int r);
+
   void send_break_lock();
   void handle_break_lock(int r);
 
diff --git a/src/test/librbd/managed_lock/test_mock_BreakRequest.cc b/src/test/librbd/managed_lock/test_mock_BreakRequest.cc
index e9ac8be7e9e..4c810fab05e 100644
--- a/src/test/librbd/managed_lock/test_mock_BreakRequest.cc
+++ b/src/test/librbd/managed_lock/test_mock_BreakRequest.cc
@@ -56,6 +56,11 @@ GetLockerRequest<librbd::MockTestImageCtx> *GetLockerRequest<librbd::MockTestIma
 // template definitions
 #include "librbd/managed_lock/BreakRequest.cc"
 
+MATCHER(IsBlacklistCommand, "") {
+  return (arg.size() == 1 &&
+          arg[0].find("\"blacklistop\": \"add\"") != std::string::npos);
+}
+
 namespace librbd {
 namespace managed_lock {
 
@@ -106,9 +111,17 @@ public:
 
 
   void expect_blacklist_add(MockTestImageCtx &mock_image_ctx, int r) {
-    EXPECT_CALL(*get_mock_io_ctx(mock_image_ctx.md_ctx).get_mock_rados_client(),
-                blacklist_add(_, _))
-                  .WillOnce(Return(r));
+    auto& mock_rados_client = librados::get_mock_rados_client(
+      mock_image_ctx.rados_api);
+    EXPECT_CALL(mock_rados_client, mon_command(IsBlacklistCommand(), _, _, _))
+      .WillOnce(Return(r));
+  }
+
+  void expect_wait_for_latest_osd_map(MockTestImageCtx &mock_image_ctx, int r) {
+    auto& mock_rados_client = librados::get_mock_rados_client(
+      mock_image_ctx.rados_api);
+    EXPECT_CALL(mock_rados_client, wait_for_latest_osd_map())
+      .WillOnce(Return(r));
   }
 
   void expect_break_lock(MockTestImageCtx &mock_image_ctx, int r) {
@@ -142,6 +155,7 @@ TEST_F(TestMockManagedLockBreakRequest, DeadLockOwner) {
                     0);
 
   expect_blacklist_add(mock_image_ctx, 0);
+  expect_wait_for_latest_osd_map(mock_image_ctx, 0);
   expect_break_lock(mock_image_ctx, 0);
 
   C_SaferCond ctx;
@@ -171,6 +185,7 @@ TEST_F(TestMockManagedLockBreakRequest, ForceBreak) {
                     0);
 
   expect_blacklist_add(mock_image_ctx, 0);
+  expect_wait_for_latest_osd_map(mock_image_ctx, 0);
   expect_break_lock(mock_image_ctx, 0);
 
   C_SaferCond ctx;
@@ -428,6 +443,7 @@ TEST_F(TestMockManagedLockBreakRequest, BreakLockMissing) {
                     0);
 
   expect_blacklist_add(mock_image_ctx, 0);
+  expect_wait_for_latest_osd_map(mock_image_ctx, 0);
   expect_break_lock(mock_image_ctx, -ENOENT);
 
   C_SaferCond ctx;
@@ -457,6 +473,7 @@ TEST_F(TestMockManagedLockBreakRequest, BreakLockError) {
                     0);
 
   expect_blacklist_add(mock_image_ctx, 0);
+  expect_wait_for_latest_osd_map(mock_image_ctx, 0);
   expect_break_lock(mock_image_ctx, -EINVAL);
 
   C_SaferCond ctx;