From: Mykola Golub Date: Sat, 30 May 2020 08:45:31 +0000 (+0100) Subject: librbd: extend quiesce_complete API to return error code X-Git-Tag: v16.1.0~2015^2~11 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cd579e37efaa2258efa5aa12f8e9922b3f6ba6f8;p=ceph.git librbd: extend quiesce_complete API to return error code Signed-off-by: Mykola Golub --- diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h index b90cd3eb5fa6..9da0175221a1 100644 --- a/src/include/rbd/librbd.h +++ b/src/include/rbd/librbd.h @@ -1385,9 +1385,9 @@ CEPH_RBD_API int rbd_quiesce_watch(rbd_image_t image, * Notify quiesce is complete * * @param image the image to notify - * @returns 0 on success + * @param r the return code */ -CEPH_RADOS_API void rbd_quiesce_complete(rbd_image_t image); +CEPH_RADOS_API void rbd_quiesce_complete(rbd_image_t image, int r); /** * Unregister a quiesce/unquiesce watcher. diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp index 8d3944a01e94..43b04f8a9b06 100644 --- a/src/include/rbd/librbd.hpp +++ b/src/include/rbd/librbd.hpp @@ -784,7 +784,7 @@ public: int quiesce_watch(QuiesceWatchCtx *ctx, uint64_t *handle); int quiesce_unwatch(uint64_t handle); - void quiesce_complete(); + void quiesce_complete(int r); private: friend class RBD; diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc index 889a36609dac..f6b4e2c4d85b 100644 --- a/src/librbd/ImageState.cc +++ b/src/librbd/ImageState.cc @@ -295,7 +295,7 @@ public: notify(UNQUIESCE, on_finish); } - void quiesce_complete() { + void quiesce_complete(int r) { Context *on_notify = nullptr; { std::lock_guard locker{m_lock}; @@ -311,7 +311,7 @@ public: std::swap(on_notify, m_on_notify); } - on_notify->complete(0); + on_notify->complete(r); } private: @@ -980,8 +980,8 @@ void ImageState::notify_unquiesce(Context *on_finish) { } template -void ImageState::quiesce_complete() { - m_quiesce_watchers->quiesce_complete(); +void ImageState::quiesce_complete(int r) { + m_quiesce_watchers->quiesce_complete(r); } } // namespace librbd diff --git a/src/librbd/ImageState.h b/src/librbd/ImageState.h index 446028805d8d..424e81882adc 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(); + void quiesce_complete(int r); private: enum State { diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index a2acaf9d6007..72788d2899b7 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -366,10 +366,15 @@ void ImageWatcher::notify_quiesce(const AsyncRequestId &async_request_id, << dendl; ceph_assert(attempts > 0); + auto notify_response = new watcher::NotifyResponse(); auto on_notify = new LambdaContext( - [this, async_request_id, &prog_ctx, on_finish, attempts=attempts-1](int r) { + [notify_response=std::unique_ptr(notify_response), + this, async_request_id, &prog_ctx, on_finish, attempts=attempts-1](int r) { auto total_attempts = m_image_ctx.config.template get_val( "rbd_quiesce_notification_attempts"); + if (total_attempts < attempts) { + total_attempts = attempts; + } prog_ctx.update_progress(total_attempts - attempts, total_attempts); if (r == -ETIMEDOUT) { @@ -379,6 +384,28 @@ void ImageWatcher::notify_quiesce(const AsyncRequestId &async_request_id, notify_quiesce(async_request_id, attempts, prog_ctx, on_finish); return; } + } else if (r == 0) { + for (auto &[client_id, bl] : notify_response->acks) { + if (bl.length() == 0) { + continue; + } + try { + auto iter = bl.cbegin(); + + ResponseMessage response_message; + using ceph::decode; + decode(response_message, iter); + + if (response_message.result != -EOPNOTSUPP) { + r = response_message.result; + } + } catch (const buffer::error &err) { + r = -EINVAL; + } + if (r < 0) { + break; + } + } } if (r < 0) { lderr(m_image_ctx.cct) << this << " failed to notify quiesce: " @@ -387,7 +414,9 @@ void ImageWatcher::notify_quiesce(const AsyncRequestId &async_request_id, on_finish->complete(r); }); - send_notify(new QuiescePayload(async_request_id), on_notify); + bufferlist bl; + encode(NotifyMessage(new QuiescePayload(async_request_id)), bl); + Watcher::send_notify(bl, notify_response, on_notify); } template @@ -691,11 +720,13 @@ Context *ImageWatcher::prepare_quiesce_request( return new LambdaContext( [this, request, unquiesce_ctx, timeout](int r) { - ceph_assert(r == 0); - - m_task_finisher->add_event_after(Task(TASK_CODE_QUIESCE, request), - timeout, unquiesce_ctx); - + if (r < 0) { + std::unique_lock async_request_locker{m_async_request_lock}; + m_async_pending.erase(request); + } else { + 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 fdb9002510c3..a2d9243550cf 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -3036,9 +3036,9 @@ namespace librbd { return r; } - void Image::quiesce_complete() { + void Image::quiesce_complete(int r) { ImageCtx *ictx = (ImageCtx *)ctx; - ictx->state->quiesce_complete(); + ictx->state->quiesce_complete(r); } } // namespace librbd @@ -7162,8 +7162,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) +extern "C" void rbd_quiesce_complete(rbd_image_t image, int r) { librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; - ictx->state->quiesce_complete(); + ictx->state->quiesce_complete(r); } diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc index a3e539fd54b4..e3ffc536098b 100644 --- a/src/test/librbd/test_librbd.cc +++ b/src/test/librbd/test_librbd.cc @@ -8210,7 +8210,7 @@ TEST_F(TestLibRBD, QuiesceWatch) void handle_quiesce() { ASSERT_EQ(quiesce_count, unquiesce_count); quiesce_count++; - rbd_quiesce_complete(image); + rbd_quiesce_complete(image, 0); } void handle_unquiesce() { unquiesce_count++; @@ -8280,7 +8280,7 @@ TEST_F(TestLibRBD, QuiesceWatchPP) void handle_quiesce() override { ASSERT_EQ(quiesce_count, unquiesce_count); quiesce_count++; - image.quiesce_complete(); + image.quiesce_complete(0); } void handle_unquiesce() override { unquiesce_count++; @@ -8323,6 +8323,85 @@ TEST_F(TestLibRBD, QuiesceWatchPP) ioctx.close(); } +TEST_F(TestLibRBD, QuiesceWatchError) +{ + librbd::RBD rbd; + librados::IoCtx ioctx; + ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx)); + std::string name = get_temp_image_name(); + int order = 0; + uint64_t size = 2 << 20; + ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), size, &order)); + + { + librbd::Image image1, image2; + ASSERT_EQ(0, rbd.open(ioctx, image1, name.c_str(), NULL)); + ASSERT_EQ(0, rbd.open(ioctx, image2, name.c_str(), NULL)); + + struct Watcher : public librbd::QuiesceWatchCtx { + librbd::Image ℑ + int r; + mutex m_lock; + condition_variable m_cond; + size_t quiesce_count = 0; + size_t unquiesce_count = 0; + + Watcher(librbd::Image &image, int r) : image(image), r(r) { + } + + void handle_quiesce() override { + lock_guard locker(m_lock); + quiesce_count++; + image.quiesce_complete(r); + m_cond.notify_one(); + } + + void handle_unquiesce() override { + lock_guard locker(m_lock); + unquiesce_count++; + m_cond.notify_one(); + } + + void wait_for_unquiesce_count(size_t count) { + unique_lock locker(m_lock); + ASSERT_TRUE(m_cond.wait_for(locker, seconds(60), + [this, count] { + return this->unquiesce_count == count; + })); + } + } watcher1(image1, -EINVAL), watcher2(image2, 0); + uint64_t handle1, handle2; + + ASSERT_EQ(0, image1.quiesce_watch(&watcher1, &handle1)); + ASSERT_EQ(0, image2.quiesce_watch(&watcher2, &handle2)); + + ASSERT_EQ(-EINVAL, image1.snap_create("snap1")); + ASSERT_EQ(1U, watcher2.quiesce_count); + watcher2.wait_for_unquiesce_count(1U); + ASSERT_EQ(1U, watcher1.quiesce_count); + ASSERT_EQ(0U, watcher1.unquiesce_count); + + ASSERT_EQ(-EINVAL, image2.snap_create("snap2")); + ASSERT_EQ(2U, watcher2.quiesce_count); + watcher2.wait_for_unquiesce_count(2U); + ASSERT_EQ(2U, watcher1.quiesce_count); + ASSERT_EQ(0U, watcher1.unquiesce_count); + + ASSERT_EQ(0, image1.quiesce_unwatch(handle1)); + + ASSERT_EQ(0, image1.snap_create("snap3")); + ASSERT_EQ(3U, watcher2.quiesce_count); + ASSERT_EQ(3U, watcher2.unquiesce_count); + + ASSERT_EQ(0, image2.quiesce_unwatch(handle2)); + + ASSERT_EQ(0, image1.snap_remove("snap3")); + } + + ASSERT_EQ(0, rbd.remove(ioctx, name.c_str())); + ioctx.close(); +} + TEST_F(TestLibRBD, QuiesceWatchTimeout) { REQUIRE(!is_librados_test_stub(_rados)); @@ -8388,7 +8467,7 @@ TEST_F(TestLibRBD, QuiesceWatchTimeout) thread quiesce1([&image, &watcher]() { watcher.wait_for_quiesce_count(1); sleep(8); - image.quiesce_complete(); + image.quiesce_complete(0); }); ASSERT_EQ(0, image.snap_create("snap1")); @@ -8406,7 +8485,7 @@ TEST_F(TestLibRBD, QuiesceWatchTimeout) std::cerr << "waiting for timed out ... " << i << std::endl; sleep(1); } - image.quiesce_complete(); + image.quiesce_complete(0); }); ASSERT_EQ(-ETIMEDOUT, image.snap_create("snap2")); @@ -8418,7 +8497,7 @@ TEST_F(TestLibRBD, QuiesceWatchTimeout) thread quiesce3([&image, &watcher]() { watcher.wait_for_quiesce_count(3); - image.quiesce_complete(); + image.quiesce_complete(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 936dc3a05ca4..6789c8135c2c 100644 --- a/src/tools/rbd_nbd/rbd-nbd.cc +++ b/src/tools/rbd_nbd/rbd-nbd.cc @@ -463,7 +463,7 @@ signal: wait_inflight_io(); - image.quiesce_complete(); + image.quiesce_complete(0); // TODO: return quiesce hook exit code wait_unquiesce();