From: Mykola Golub Date: Wed, 20 May 2020 14:36:55 +0000 (+0100) Subject: librbd: refactor quiesce notify request X-Git-Tag: v16.1.0~2202^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=44ef805579007e25e60867b2fbf156712ddfdd80;p=ceph.git librbd: refactor quiesce notify request Generate request id inside notify request instead fo passing from outside. Signed-off-by: Mykola Golub --- diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index 65c82a8ac461..a2acaf9d6007 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -340,9 +340,10 @@ void ImageWatcher::notify_header_update(librados::IoCtx &io_ctx, } template -void ImageWatcher::notify_quiesce(uint64_t request_id, - ProgressContext &prog_ctx, - Context *on_finish) { +uint64_t ImageWatcher::notify_quiesce(ProgressContext &prog_ctx, + Context *on_finish) { + uint64_t request_id = m_image_ctx.operations->reserve_async_request_id(); + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": request_id=" << request_id << dendl; @@ -352,6 +353,8 @@ void ImageWatcher::notify_quiesce(uint64_t request_id, "rbd_quiesce_notification_attempts"); notify_quiesce(async_request_id, attempts, prog_ctx, on_finish); + + return request_id; } template diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h index 130ba6354136..f141f4a6f24f 100644 --- a/src/librbd/ImageWatcher.h +++ b/src/librbd/ImageWatcher.h @@ -72,8 +72,7 @@ public: static void notify_header_update(librados::IoCtx &io_ctx, const std::string &oid); - void notify_quiesce(uint64_t request_id, ProgressContext &prog_ctx, - Context *on_finish); + uint64_t notify_quiesce(ProgressContext &prog_ctx, Context *on_finish); void notify_unquiesce(uint64_t request_id, Context *on_finish); private: diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index 3e31c71f8892..43098d7764d6 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -778,12 +778,10 @@ void Operations::execute_snap_create(const cls::rbd::SnapshotNamespace &snap_ } m_image_ctx.image_lock.unlock_shared(); - uint64_t request_id = ++m_async_request_seq; operation::SnapshotCreateRequest *req = new operation::SnapshotCreateRequest( m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), - snap_namespace, snap_name, journal_op_tid, request_id, skip_object_map, - prog_ctx); + snap_namespace, snap_name, journal_op_tid, skip_object_map, prog_ctx); req->send(); } diff --git a/src/librbd/Operations.h b/src/librbd/Operations.h index 7c46dedb3aff..b1fbfe1dd66a 100644 --- a/src/librbd/Operations.h +++ b/src/librbd/Operations.h @@ -24,6 +24,10 @@ class Operations { public: Operations(ImageCtxT &image_ctx); + uint64_t reserve_async_request_id() { + return ++m_async_request_seq; + } + int flatten(ProgressContext &prog_ctx); void execute_flatten(ProgressContext &prog_ctx, Context *on_finish); @@ -114,7 +118,7 @@ public: private: ImageCtxT &m_image_ctx; - std::atomic m_async_request_seq; + std::atomic m_async_request_seq; int invoke_async_request(const std::string& name, exclusive_lock::OperationRequestType request_type, diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc index 7a957b002159..7c5f14244d86 100644 --- a/src/librbd/operation/SnapshotCreateRequest.cc +++ b/src/librbd/operation/SnapshotCreateRequest.cc @@ -30,13 +30,11 @@ SnapshotCreateRequest::SnapshotCreateRequest(I &image_ctx, const cls::rbd::SnapshotNamespace &snap_namespace, const std::string &snap_name, uint64_t journal_op_tid, - uint64_t request_id, bool skip_object_map, ProgressContext &prog_ctx) : Request(image_ctx, on_finish, journal_op_tid), m_snap_namespace(snap_namespace), m_snap_name(snap_name), - m_request_id(request_id), m_skip_object_map(skip_object_map), - m_prog_ctx(prog_ctx) { + m_skip_object_map(skip_object_map), m_prog_ctx(prog_ctx) { } template @@ -60,8 +58,8 @@ void SnapshotCreateRequest::send_notify_quiesce() { CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << dendl; - image_ctx.image_watcher->notify_quiesce( - m_request_id, m_prog_ctx, create_async_context_callback( + m_request_id = image_ctx.image_watcher->notify_quiesce( + m_prog_ctx, create_async_context_callback( image_ctx, create_context_callback, &SnapshotCreateRequest::handle_notify_quiesce>(this))); } diff --git a/src/librbd/operation/SnapshotCreateRequest.h b/src/librbd/operation/SnapshotCreateRequest.h index c4840d38995e..544c04165c88 100644 --- a/src/librbd/operation/SnapshotCreateRequest.h +++ b/src/librbd/operation/SnapshotCreateRequest.h @@ -71,8 +71,7 @@ public: SnapshotCreateRequest(ImageCtxT &image_ctx, Context *on_finish, const cls::rbd::SnapshotNamespace &snap_namespace, const std::string &snap_name, uint64_t journal_op_tid, - uint64_t request_id, bool skip_object_map, - ProgressContext &prog_ctx); + bool skip_object_map, ProgressContext &prog_ctx); protected: void send_op() override; @@ -89,10 +88,10 @@ protected: private: cls::rbd::SnapshotNamespace m_snap_namespace; std::string m_snap_name; - uint64_t m_request_id; bool m_skip_object_map; ProgressContext &m_prog_ctx; + uint64_t m_request_id = 0; int m_ret_val = 0; uint64_t m_snap_id = CEPH_NOSNAP; diff --git a/src/test/librbd/mock/MockImageWatcher.h b/src/test/librbd/mock/MockImageWatcher.h index 0cca579b297a..f58bb2ba9622 100644 --- a/src/test/librbd/mock/MockImageWatcher.h +++ b/src/test/librbd/mock/MockImageWatcher.h @@ -25,7 +25,7 @@ struct MockImageWatcher { MOCK_METHOD0(notify_released_lock, void()); MOCK_METHOD0(notify_request_lock, void()); - MOCK_METHOD3(notify_quiesce, void(uint64_t, ProgressContext &, Context *)); + MOCK_METHOD2(notify_quiesce, uint64_t(ProgressContext &, Context *)); MOCK_METHOD2(notify_unquiesce, void(uint64_t, Context *)); }; diff --git a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc index 650369748a51..c80f7627e6b8 100644 --- a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc +++ b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc @@ -63,9 +63,10 @@ public: typedef mirror::snapshot::SetImageStateRequest MockSetImageStateRequest; void expect_notify_quiesce(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(*mock_image_ctx.image_watcher, notify_quiesce(_, _, _)) - .WillOnce(WithArg<2>( - CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue))); + EXPECT_CALL(*mock_image_ctx.image_watcher, notify_quiesce(_, _)) + .WillOnce(DoAll(WithArg<1>(CompleteContext( + r, mock_image_ctx.image_ctx->op_work_queue)), + Return(0))); } void expect_block_writes(MockImageCtx &mock_image_ctx) { @@ -187,7 +188,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, Success) { librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false, prog_ctx); + "snap1", 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -210,7 +211,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, NotifyQuiesceError) { librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false, prog_ctx); + "snap1", 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -243,7 +244,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, AllocateSnapIdError) { librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false, prog_ctx); + "snap1", 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -285,7 +286,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, CreateSnapStale) { librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false, prog_ctx); + "snap1", 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -319,7 +320,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, CreateSnapError) { librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false, prog_ctx); + "snap1", 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -353,7 +354,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, ReleaseSnapIdError) { librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false, prog_ctx); + "snap1", 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -393,7 +394,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, SkipObjectMap) { librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, true, prog_ctx); + "snap1", 0, true, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -440,7 +441,7 @@ TEST_F(TestMockOperationSnapshotCreateRequest, SetImageState) { mock_image_ctx, &cond_ctx, cls::rbd::MirrorSnapshotNamespace{ cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {}, "", CEPH_NOSNAP}, - "snap1", 0, 0, false, prog_ctx); + "snap1", 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send();