From: Mykola Golub Date: Tue, 1 Sep 2020 08:55:42 +0000 (+0100) Subject: librbd: make quiesce_complete to pass handle X-Git-Tag: v16.1.0~1123^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=25df4eac8cb49f68d4715e48db29d57a29a06ac5;p=ceph.git librbd: make quiesce_complete to pass handle Use it to distinguish failed watchers to skip calling unquiesce callback for those. Signed-off-by: Mykola Golub --- diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h index 30abbd0a0d4f..810b03693365 100644 --- a/src/include/rbd/librbd.h +++ b/src/include/rbd/librbd.h @@ -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. diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp index 76acfa9b393e..ec96f72b8fab 100644 --- a/src/include/rbd/librbd.hpp +++ b/src/include/rbd/librbd.hpp @@ -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; diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc index 4de2dc4ba405..ab63b69bddae 100644 --- a/src/librbd/ImageState.cc +++ b/src/librbd/ImageState.cc @@ -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 m_pending_notify; std::map m_pending_unregister; uint64_t m_handle_quiesce_cnt = 0; + std::set 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::notify_unquiesce(Context *on_finish) { } template -void ImageState::quiesce_complete(int r) { +void ImageState::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 diff --git a/src/librbd/ImageState.h b/src/librbd/ImageState.h index 424e81882adc..5107c1a17a9a 100644 --- a/src/librbd/ImageState.h +++ b/src/librbd/ImageState.h @@ -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 { diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index 1c7b4d6a9bf9..11291813f536 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -713,29 +713,25 @@ Context *ImageWatcher::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(ctx)); diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index f968ee442fdc..1af6381f258f 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -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); } diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc index 3227ccc72c7e..6deca3c8ff00 100644 --- a/src/test/librbd/test_librbd.cc +++ b/src/test/librbd/test_librbd.cc @@ -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 ℑ + 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 ℑ + 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 ℑ 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; diff --git a/src/tools/rbd_nbd/rbd-nbd.cc b/src/tools/rbd_nbd/rbd-nbd.cc index 6789c8135c2c..de6a58f51b6f 100644 --- a/src/tools/rbd_nbd/rbd-nbd.cc +++ b/src/tools/rbd_nbd/rbd-nbd.cc @@ -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 ℑ @@ -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); }