]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
librbd: make quiesce_complete to pass handle
authorMykola Golub <mgolub@suse.com>
Tue, 1 Sep 2020 08:55:42 +0000 (09:55 +0100)
committerMykola Golub <mgolub@suse.com>
Wed, 2 Sep 2020 10:15:12 +0000 (11:15 +0100)
Use it to distinguish failed watchers to skip calling unquiesce
callback for those.

Signed-off-by: Mykola Golub <mgolub@suse.com>
src/include/rbd/librbd.h
src/include/rbd/librbd.hpp
src/librbd/ImageState.cc
src/librbd/ImageState.h
src/librbd/ImageWatcher.cc
src/librbd/librbd.cc
src/test/librbd/test_librbd.cc
src/tools/rbd_nbd/rbd-nbd.cc

index 30abbd0a0d4f74523827b801c8e34e205b4012ec..810b03693365885972094c955fe0d8d66dfc53e3 100644 (file)
@@ -1411,9 +1411,11 @@ CEPH_RBD_API int rbd_quiesce_watch(rbd_image_t image,
  * Notify quiesce is complete
  *
  * @param image the image to notify
+ * @param handle which watch is complete
  * @param r the return code
  */
-CEPH_RADOS_API void rbd_quiesce_complete(rbd_image_t image, int r);
+CEPH_RADOS_API void rbd_quiesce_complete(rbd_image_t image, uint64_t handle,
+                                         int r);
 
 /**
  * Unregister a quiesce/unquiesce watcher.
index 76acfa9b393eca724134b08c40bef4151478dbaa..ec96f72b8fab83b2de07ac0b359b357d410bc838 100644 (file)
@@ -794,7 +794,7 @@ public:
 
   int quiesce_watch(QuiesceWatchCtx *ctx, uint64_t *handle);
   int quiesce_unwatch(uint64_t handle);
-  void quiesce_complete(int r);
+  void quiesce_complete(uint64_t handle, int r);
 
 private:
   friend class RBD;
index 4de2dc4ba405caec762b8cfbfa4f7a0cda654b42..ab63b69bddaeb995e267dd17c5436db532b6c6fa 100644 (file)
@@ -299,27 +299,28 @@ public:
     notify(UNQUIESCE, on_finish);
   }
 
-  void quiesce_complete(int r) {
+  void quiesce_complete(uint64_t handle, int r) {
     Context *on_notify = nullptr;
     {
       std::lock_guard locker{m_lock};
       ceph_assert(m_on_notify != nullptr);
       ceph_assert(m_handle_quiesce_cnt > 0);
-      ceph_assert(m_blocked);
 
       m_handle_quiesce_cnt--;
 
-      if (m_handle_quiesce_cnt > 0) {
-        // TOFO: r should be saved
-        return;
+      if (r < 0) {
+        ldout(m_cct, 10) << "QuiesceWatchers::" << __func__ << ": watcher "
+                         << handle << " failed" << dendl;
+        m_failed_watchers.insert(handle);
+        m_ret_val = r;
       }
 
-      if (r < 0) {
-        // TODO: fix for multiple watchers case
-        m_blocked = false;
+      if (m_handle_quiesce_cnt > 0) {
+        return;
       }
 
       std::swap(on_notify, m_on_notify);
+      r = m_ret_val;
     }
 
     on_notify->complete(r);
@@ -338,7 +339,9 @@ private:
   std::list<Context *> m_pending_notify;
   std::map<uint64_t, Context*> m_pending_unregister;
   uint64_t m_handle_quiesce_cnt = 0;
+  std::set<uint64_t> m_failed_watchers;
   bool m_blocked = false;
+  int m_ret_val = 0;
 
   void notify(EventType event_type, Context *on_finish) {
     ceph_assert(ceph_mutex_is_locked(m_lock));
@@ -354,7 +357,12 @@ private:
     Context *ctx = nullptr;
     if (event_type == QUIESCE) {
       ceph_assert(!m_blocked);
+      ceph_assert(m_handle_quiesce_cnt == 0);
+
       m_blocked = true;
+      m_handle_quiesce_cnt = m_watchers.size();
+      m_failed_watchers.clear();
+      m_ret_val = 0;
     } else {
       ceph_assert(event_type == UNQUIESCE);
       ceph_assert(m_blocked);
@@ -366,12 +374,11 @@ private:
     auto gather_ctx = new C_Gather(m_cct, ctx);
 
     ceph_assert(m_on_notify == nullptr);
-    ceph_assert(m_handle_quiesce_cnt == 0);
 
     m_on_notify = on_finish;
 
-    for (auto it : m_watchers) {
-      send_notify(it.first, it.second, event_type, gather_ctx->new_sub());
+    for (auto &[handle, watcher] : m_watchers) {
+      send_notify(handle, watcher, event_type, gather_ctx->new_sub());
     }
 
     gather_ctx->activate();
@@ -385,10 +392,18 @@ private:
                          << handle << ", event_type=" << event_type << dendl;
         switch (event_type) {
         case QUIESCE:
-          m_handle_quiesce_cnt++;
           watcher->handle_quiesce();
           break;
         case UNQUIESCE:
+          {
+            std::lock_guard locker{m_lock};
+
+            if (m_failed_watchers.count(handle)) {
+              ldout(m_cct, 20) << "QuiesceWatchers::" << __func__
+                               << ": skip for failed watcher" << dendl;
+              break;
+            }
+          }
           watcher->handle_unquiesce();
           break;
         default:
@@ -1010,10 +1025,10 @@ void ImageState<I>::notify_unquiesce(Context *on_finish) {
 }
 
 template <typename I>
-void ImageState<I>::quiesce_complete(int r) {
+void ImageState<I>::quiesce_complete(uint64_t handle, int r) {
   CephContext *cct = m_image_ctx->cct;
-  ldout(cct, 20) << __func__ << ": r=" << r << dendl;
-  m_quiesce_watchers->quiesce_complete(r);
+  ldout(cct, 20) << __func__ << ": handle=" << handle << " r=" << r << dendl;
+  m_quiesce_watchers->quiesce_complete(handle, r);
 }
 
 } // namespace librbd
index 424e81882adccdfe853bc938eceb83de1277b0b0..5107c1a17a9ae014961b97db8ac13b527e5f30d5 100644 (file)
@@ -57,7 +57,7 @@ public:
   int unregister_quiesce_watcher(uint64_t handle);
   void notify_quiesce(Context *on_finish);
   void notify_unquiesce(Context *on_finish);
-  void quiesce_complete(int r);
+  void quiesce_complete(uint64_t handle, int r);
 
 private:
   enum State {
index 1c7b4d6a9bf93b315a77b11f7e29efafc563c88c..11291813f536fd8da5bc50e63e3a088d8fc88845 100644 (file)
@@ -713,29 +713,25 @@ Context *ImageWatcher<I>::prepare_quiesce_request(
 
   return new LambdaContext(
     [this, request, timeout](int r) {
-      if (r < 0) {
-        std::unique_lock async_request_locker{m_async_request_lock};
-        m_async_pending.erase(request);
-      } else {
-        auto unquiesce_ctx = new LambdaContext(
-          [this, request](int r) {
-            if (r == 0) {
-              ldout(m_image_ctx.cct, 10) << this << " quiesce request "
-                                         << request << " timed out" << dendl;
-            }
+      auto unquiesce_ctx = new LambdaContext(
+        [this, request](int r) {
+          if (r == 0) {
+            ldout(m_image_ctx.cct, 10) << this << " quiesce request "
+                                       << request << " timed out" << dendl;
+          }
 
-            auto on_finish = new LambdaContext(
-              [this, request](int r) {
-                std::unique_lock async_request_locker{m_async_request_lock};
-                m_async_pending.erase(request);
-              });
+          auto on_finish = new LambdaContext(
+            [this, request](int r) {
+              std::unique_lock async_request_locker{m_async_request_lock};
+              m_async_pending.erase(request);
+            });
 
-            m_image_ctx.state->notify_unquiesce(on_finish);
-          });
+          m_image_ctx.state->notify_unquiesce(on_finish);
+        });
+
+      m_task_finisher->add_event_after(Task(TASK_CODE_QUIESCE, request),
+                                       timeout, unquiesce_ctx);
 
-        m_task_finisher->add_event_after(Task(TASK_CODE_QUIESCE, request),
-                                         timeout, unquiesce_ctx);
-      }
       auto ctx = remove_async_request(request);
       ceph_assert(ctx != nullptr);
       ctx = new C_ResponseMessage(static_cast<C_NotifyAck *>(ctx));
index f968ee442fdca5a65077151e42454397034c6c76..1af6381f258f80928649505ad4e56ecb3f87f4ac 100644 (file)
@@ -3071,9 +3071,9 @@ namespace librbd {
     return r;
   }
 
-  void Image::quiesce_complete(int r) {
+  void Image::quiesce_complete(uint64_t handle, int r) {
     ImageCtx *ictx = (ImageCtx *)ctx;
-    ictx->state->quiesce_complete(r);
+    ictx->state->quiesce_complete(handle, r);
   }
 
 } // namespace librbd
@@ -7238,8 +7238,8 @@ extern "C" int rbd_quiesce_unwatch(rbd_image_t image, uint64_t handle)
   return r;
 }
 
-extern "C" void rbd_quiesce_complete(rbd_image_t image, int r)
+extern "C" void rbd_quiesce_complete(rbd_image_t image, uint64_t handle, int r)
 {
   librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
-  ictx->state->quiesce_complete(r);
+  ictx->state->quiesce_complete(handle, r);
 }
index 3227ccc72c7ede69fde2d44865e04d71ac8d0fae..6deca3c8ff005c8944aaaca4ef82a3f6545f6394 100644 (file)
@@ -8210,7 +8210,6 @@ TEST_F(TestLibRBD, QuiesceWatch)
   uint64_t size = 2 << 20;
   ASSERT_EQ(0, create_image(ioctx, name.c_str(), size, &order));
 
-  uint64_t handle1, handle2;
   rbd_image_t image1, image2;
   ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image1, NULL));
   ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image2, NULL));
@@ -8226,6 +8225,7 @@ TEST_F(TestLibRBD, QuiesceWatch)
     }
 
     rbd_image_t &image;
+    uint64_t handle = 0;
     size_t quiesce_count = 0;
     size_t unquiesce_count = 0;
 
@@ -8238,7 +8238,7 @@ TEST_F(TestLibRBD, QuiesceWatch)
     void handle_quiesce() {
       ASSERT_EQ(quiesce_count, unquiesce_count);
       quiesce_count++;
-      rbd_quiesce_complete(image, 0);
+      rbd_quiesce_complete(image, handle, 0);
     }
     void handle_unquiesce() {
       std::unique_lock locker(lock);
@@ -8254,9 +8254,11 @@ TEST_F(TestLibRBD, QuiesceWatch)
   } watcher1(image1), watcher2(image2);
 
   ASSERT_EQ(0, rbd_quiesce_watch(image1, Watcher::quiesce_cb,
-                                 Watcher::unquiesce_cb, &watcher1, &handle1));
+                                 Watcher::unquiesce_cb, &watcher1,
+                                 &watcher1.handle));
   ASSERT_EQ(0, rbd_quiesce_watch(image2, Watcher::quiesce_cb,
-                                 Watcher::unquiesce_cb, &watcher2, &handle2));
+                                 Watcher::unquiesce_cb, &watcher2,
+                                 &watcher2.handle));
 
   ASSERT_EQ(0, rbd_snap_create(image1, "snap1"));
   ASSERT_EQ(1U, watcher1.quiesce_count);
@@ -8270,7 +8272,7 @@ TEST_F(TestLibRBD, QuiesceWatch)
   ASSERT_EQ(2U, watcher2.quiesce_count);
   ASSERT_TRUE(watcher2.wait_for_unquiesce(2U));
 
-  ASSERT_EQ(0, rbd_quiesce_unwatch(image1, handle1));
+  ASSERT_EQ(0, rbd_quiesce_unwatch(image1, watcher1.handle));
 
   ASSERT_EQ(0, rbd_snap_create(image1, "snap3"));
   ASSERT_EQ(2U, watcher1.quiesce_count);
@@ -8278,7 +8280,7 @@ TEST_F(TestLibRBD, QuiesceWatch)
   ASSERT_EQ(3U, watcher2.quiesce_count);
   ASSERT_TRUE(watcher2.wait_for_unquiesce(3U));
 
-  ASSERT_EQ(0, rbd_quiesce_unwatch(image2, handle2));
+  ASSERT_EQ(0, rbd_quiesce_unwatch(image2, watcher2.handle));
 
   ASSERT_EQ(0, rbd_snap_remove(image1, "snap1"));
   ASSERT_EQ(0, rbd_snap_remove(image1, "snap2"));
@@ -8306,6 +8308,7 @@ TEST_F(TestLibRBD, QuiesceWatchPP)
 
     struct Watcher : public librbd::QuiesceWatchCtx {
       librbd::Image &image;
+      uint64_t handle = 0;
       size_t quiesce_count = 0;
       size_t unquiesce_count = 0;
 
@@ -8318,7 +8321,7 @@ TEST_F(TestLibRBD, QuiesceWatchPP)
       void handle_quiesce() override {
         ASSERT_EQ(quiesce_count, unquiesce_count);
         quiesce_count++;
-        image.quiesce_complete(0);
+        image.quiesce_complete(handle, 0);
       }
       void handle_unquiesce() override {
         std::unique_lock locker(lock);
@@ -8332,10 +8335,9 @@ TEST_F(TestLibRBD, QuiesceWatchPP)
                            [this, c]() { return unquiesce_count >= c; });
       }
     } watcher1(image1), watcher2(image2);
-    uint64_t handle1, handle2;
 
-    ASSERT_EQ(0, image1.quiesce_watch(&watcher1, &handle1));
-    ASSERT_EQ(0, image2.quiesce_watch(&watcher2, &handle2));
+    ASSERT_EQ(0, image1.quiesce_watch(&watcher1, &watcher1.handle));
+    ASSERT_EQ(0, image2.quiesce_watch(&watcher2, &watcher2.handle));
 
     ASSERT_EQ(0, image1.snap_create("snap1"));
     ASSERT_EQ(1U, watcher1.quiesce_count);
@@ -8349,7 +8351,7 @@ TEST_F(TestLibRBD, QuiesceWatchPP)
     ASSERT_EQ(2U, watcher2.quiesce_count);
     ASSERT_TRUE(watcher2.wait_for_unquiesce(2U));
 
-    ASSERT_EQ(0, image1.quiesce_unwatch(handle1));
+    ASSERT_EQ(0, image1.quiesce_unwatch(watcher1.handle));
 
     ASSERT_EQ(0, image1.snap_create("snap3"));
     ASSERT_EQ(2U, watcher1.quiesce_count);
@@ -8357,7 +8359,7 @@ TEST_F(TestLibRBD, QuiesceWatchPP)
     ASSERT_EQ(3U, watcher2.quiesce_count);
     ASSERT_TRUE(watcher2.wait_for_unquiesce(3U));
 
-    ASSERT_EQ(0, image2.quiesce_unwatch(handle2));
+    ASSERT_EQ(0, image2.quiesce_unwatch(watcher2.handle));
 
     ASSERT_EQ(0, image1.snap_remove("snap1"));
     ASSERT_EQ(0, image1.snap_remove("snap2"));
@@ -8386,6 +8388,7 @@ TEST_F(TestLibRBD, QuiesceWatchError)
     struct Watcher : public librbd::QuiesceWatchCtx {
       librbd::Image &image;
       int r;
+      uint64_t handle;
       size_t quiesce_count = 0;
       size_t unquiesce_count = 0;
 
@@ -8395,9 +8398,14 @@ TEST_F(TestLibRBD, QuiesceWatchError)
       Watcher(librbd::Image &image, int r) : image(image), r(r) {
       }
 
+      void reset_counters() {
+        quiesce_count = 0;
+        unquiesce_count = 0;
+      }
+
       void handle_quiesce() override {
         quiesce_count++;
-        image.quiesce_complete(r);
+        image.quiesce_complete(handle, r);
       }
 
       void handle_unquiesce() override {
@@ -8406,45 +8414,60 @@ TEST_F(TestLibRBD, QuiesceWatchError)
         cv.notify_one();
       }
 
-      bool wait_for_unquiesce(size_t c) {
+      bool wait_for_unquiesce() {
         std::unique_lock locker(lock);
         return cv.wait_for(locker, seconds(60),
-                           [this, c]() { return unquiesce_count >= c; });
+                           [this]() {
+                             return quiesce_count == unquiesce_count;
+                           });
       }
-    } watcher1(image1, -EINVAL), watcher2(image2, 0);
-    uint64_t handle1, handle2;
+    } watcher10(image1, -EINVAL), watcher11(image1, 0), watcher20(image2, 0);
 
-    ASSERT_EQ(0, image1.quiesce_watch(&watcher1, &handle1));
-    ASSERT_EQ(0, image2.quiesce_watch(&watcher2, &handle2));
+    ASSERT_EQ(0, image1.quiesce_watch(&watcher10, &watcher10.handle));
+    ASSERT_EQ(0, image1.quiesce_watch(&watcher11, &watcher11.handle));
+    ASSERT_EQ(0, image2.quiesce_watch(&watcher20, &watcher20.handle));
 
     ASSERT_EQ(-EINVAL, image1.snap_create("snap1"));
-    ASSERT_LT(0U, watcher1.quiesce_count);
-    ASSERT_EQ(0U, watcher1.unquiesce_count);
-    ASSERT_EQ(1U, watcher2.quiesce_count);
-    ASSERT_TRUE(watcher2.wait_for_unquiesce(1U));
+    ASSERT_GT(watcher10.quiesce_count, 0U);
+    ASSERT_EQ(watcher10.unquiesce_count, 0U);
+    ASSERT_GT(watcher11.quiesce_count, 0U);
+    ASSERT_TRUE(watcher11.wait_for_unquiesce());
+    ASSERT_GT(watcher20.quiesce_count, 0U);
+    ASSERT_TRUE(watcher20.wait_for_unquiesce());
 
     PrintProgress prog_ctx;
-    watcher1.quiesce_count = 0;
+    watcher10.reset_counters();
+    watcher11.reset_counters();
+    watcher20.reset_counters();
     ASSERT_EQ(0, image2.snap_create2("snap2",
                                      RBD_SNAP_CREATE_IGNORE_QUIESCE_ERROR,
                                      prog_ctx));
-    ASSERT_LT(0U, watcher1.quiesce_count);
-    ASSERT_EQ(0U, watcher1.unquiesce_count);
-    ASSERT_EQ(2U, watcher2.quiesce_count);
-    ASSERT_TRUE(watcher2.wait_for_unquiesce(2U));
+    ASSERT_GT(watcher10.quiesce_count, 0U);
+    ASSERT_EQ(watcher10.unquiesce_count, 0U);
+    ASSERT_GT(watcher11.quiesce_count, 0U);
+    ASSERT_TRUE(watcher11.wait_for_unquiesce());
+    ASSERT_GT(watcher20.quiesce_count, 0U);
+    ASSERT_TRUE(watcher20.wait_for_unquiesce());
 
-    ASSERT_EQ(0, image1.quiesce_unwatch(handle1));
+    ASSERT_EQ(0, image1.quiesce_unwatch(watcher10.handle));
 
+    watcher11.reset_counters();
+    watcher20.reset_counters();
     ASSERT_EQ(0, image1.snap_create("snap3"));
-    ASSERT_EQ(3U, watcher2.quiesce_count);
-    ASSERT_TRUE(watcher2.wait_for_unquiesce(3U));
+    ASSERT_GT(watcher11.quiesce_count, 0U);
+    ASSERT_TRUE(watcher11.wait_for_unquiesce());
+    ASSERT_GT(watcher20.quiesce_count, 0U);
+    ASSERT_TRUE(watcher20.wait_for_unquiesce());
+
+    ASSERT_EQ(0, image1.quiesce_unwatch(watcher11.handle));
 
+    watcher20.reset_counters();
     ASSERT_EQ(0, image2.snap_create2("snap4", RBD_SNAP_CREATE_SKIP_QUIESCE,
                                      prog_ctx));
-    ASSERT_EQ(3U, watcher2.quiesce_count);
-    ASSERT_EQ(3U, watcher2.unquiesce_count);
+    ASSERT_EQ(watcher20.quiesce_count, 0U);
+    ASSERT_EQ(watcher20.unquiesce_count, 0U);
 
-    ASSERT_EQ(0, image2.quiesce_unwatch(handle2));
+    ASSERT_EQ(0, image2.quiesce_unwatch(watcher20.handle));
 
     ASSERT_EQ(0, image1.snap_remove("snap2"));
     ASSERT_EQ(0, image1.snap_remove("snap3"));
@@ -8517,10 +8540,10 @@ TEST_F(TestLibRBD, QuiesceWatchTimeout)
 
     std::cerr << "test quiesce is not long enough to time out" << std::endl;
 
-    thread quiesce1([&image, &watcher]() {
+    thread quiesce1([&image, &watcher, handle]() {
       watcher.wait_for_quiesce_count(1);
       sleep(8);
-      image.quiesce_complete(0);
+      image.quiesce_complete(handle, 0);
     });
 
     ASSERT_EQ(0, image.snap_create("snap1"));
@@ -8532,13 +8555,13 @@ TEST_F(TestLibRBD, QuiesceWatchTimeout)
     std::cerr << "test quiesce is timed out" << std::endl;
 
     bool timed_out = false;
-    thread quiesce2([&image, &watcher, &timed_out]() {
+    thread quiesce2([&image, &watcher, handle, &timed_out]() {
       watcher.wait_for_quiesce_count(2);
       for (int i = 0; !timed_out && i < 60; i++) {
         std::cerr << "waiting for timed out ... " << i << std::endl;
         sleep(1);
       }
-      image.quiesce_complete(0);
+      image.quiesce_complete(handle, 0);
     });
 
     ASSERT_EQ(-ETIMEDOUT, image.snap_create("snap2"));
@@ -8548,9 +8571,9 @@ TEST_F(TestLibRBD, QuiesceWatchTimeout)
     watcher.wait_for_unquiesce_count(2);
     ASSERT_EQ(2U, watcher.unquiesce_count);
 
-    thread quiesce3([&image, &watcher]() {
+    thread quiesce3([&image, handle, &watcher]() {
       watcher.wait_for_quiesce_count(3);
-      image.quiesce_complete(0);
+      image.quiesce_complete(handle, 0);
     });
 
     std::cerr << "test retry succeeds" << std::endl;
index 6789c8135c2c54e535c71c1c2592f0c976adbec8..de6a58f51b6feb7fab10e3149332d09ec136e44a 100644 (file)
@@ -154,6 +154,9 @@ static void run_quiesce_hook(const std::string &quiesce_hook,
 
 class NBDServer
 {
+public:
+  uint64_t quiesce_watch_handle = 0;
+
 private:
   int fd;
   librbd::Image &image;
@@ -463,7 +466,8 @@ signal:
 
       wait_inflight_io();
 
-      image.quiesce_complete(0); // TODO: return quiesce hook exit code
+      // TODO: return quiesce hook exit code
+      image.quiesce_complete(quiesce_watch_handle, 0);
 
       wait_unquiesce();
 
@@ -1415,9 +1419,9 @@ static int do_map(int argc, const char *argv[], Config *cfg)
 
   {
     NBDQuiesceWatchCtx quiesce_watch_ctx(server);
-    uint64_t quiesce_watch_handle;
     if (cfg->quiesce) {
-      r = image.quiesce_watch(&quiesce_watch_ctx, &quiesce_watch_handle);
+      r = image.quiesce_watch(&quiesce_watch_ctx,
+                              &server->quiesce_watch_handle);
       if (r < 0) {
         goto close_nbd;
       }
@@ -1436,7 +1440,7 @@ static int do_map(int argc, const char *argv[], Config *cfg)
     run_server(forker, server, use_netlink);
 
     if (cfg->quiesce) {
-      r = image.quiesce_unwatch(quiesce_watch_handle);
+      r = image.quiesce_unwatch(server->quiesce_watch_handle);
       ceph_assert(r == 0);
     }