From: Mykola Golub Date: Wed, 29 Apr 2020 17:49:17 +0000 (+0100) Subject: librbd: make "snapshot create" notification be "async" X-Git-Tag: v17.0.0~2440^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=63e14334406a2976a14eeaf80a20f089ec44b24b;p=ceph.git librbd: make "snapshot create" notification be "async" After adding quiesce/unquiesce callbacks the "snapshot create" request may run long time. Signed-off-by: Mykola Golub --- diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index 08bc01e6d7bf7..65c82a8ac4616 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -193,15 +193,21 @@ void ImageWatcher::notify_resize(uint64_t request_id, uint64_t size, } template -void ImageWatcher::notify_snap_create(const cls::rbd::SnapshotNamespace &snap_namespace, +void ImageWatcher::notify_snap_create(uint64_t request_id, + const cls::rbd::SnapshotNamespace &snap_namespace, const std::string &snap_name, + ProgressContext &prog_ctx, Context *on_finish) { ceph_assert(ceph_mutex_is_locked(m_image_ctx.owner_lock)); ceph_assert(m_image_ctx.exclusive_lock && !m_image_ctx.exclusive_lock->is_lock_owner()); - notify_lock_owner(new SnapCreatePayload(snap_namespace, snap_name), - on_finish); + AsyncRequestId async_request_id(get_client_id(), request_id); + + notify_async_request(async_request_id, + new SnapCreatePayload(async_request_id, snap_namespace, + snap_name), + prog_ctx, on_finish); } template @@ -334,7 +340,9 @@ void ImageWatcher::notify_header_update(librados::IoCtx &io_ctx, } template -void ImageWatcher::notify_quiesce(uint64_t request_id, Context *on_finish) { +void ImageWatcher::notify_quiesce(uint64_t request_id, + ProgressContext &prog_ctx, + Context *on_finish) { ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": request_id=" << request_id << dendl; @@ -343,24 +351,29 @@ void ImageWatcher::notify_quiesce(uint64_t request_id, Context *on_finish) { auto attempts = m_image_ctx.config.template get_val( "rbd_quiesce_notification_attempts"); - notify_quiesce(async_request_id, attempts, on_finish); + notify_quiesce(async_request_id, attempts, prog_ctx, on_finish); } template void ImageWatcher::notify_quiesce(const AsyncRequestId &async_request_id, - size_t attempts, Context *on_finish) { + size_t attempts, ProgressContext &prog_ctx, + Context *on_finish) { ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": async_request_id=" << async_request_id << " attempts=" << attempts << dendl; ceph_assert(attempts > 0); auto on_notify = new LambdaContext( - [this, async_request_id, on_finish, attempts=attempts-1](int r) { + [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"); + prog_ctx.update_progress(total_attempts - attempts, total_attempts); + if (r == -ETIMEDOUT) { ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": async_request_id=" << async_request_id << " timed out" << dendl; if (attempts > 0) { - notify_quiesce(async_request_id, attempts, on_finish); + notify_quiesce(async_request_id, attempts, prog_ctx, on_finish); return; } } @@ -922,14 +935,36 @@ bool ImageWatcher::handle_payload(const SnapCreatePayload &payload, } if (m_image_ctx.exclusive_lock->accept_request(request_type, &r)) { - ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: " - << payload.snap_name << dendl; + bool new_request; + Context *ctx; + ProgressContext *prog_ctx; + bool complete; + if (payload.async_request_id) { + r = prepare_async_request(payload.async_request_id, &new_request, + &ctx, &prog_ctx); + encode(ResponseMessage(r), ack_ctx->out); + complete = true; + } else { + new_request = true; + prog_ctx = new NoOpProgressContext(); + ctx = new LambdaContext( + [prog_ctx, on_finish=new C_ResponseMessage(ack_ctx)](int r) { + delete prog_ctx; + on_finish->complete(r); + }); + complete = false; + } + if (r == 0 && new_request) { + ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: " + << payload.async_request_id << " " + << payload.snap_namespace << " " + << payload.snap_name << dendl; - m_image_ctx.operations->execute_snap_create(payload.snap_namespace, - payload.snap_name, - new C_ResponseMessage(ack_ctx), - 0, false); - return false; + m_image_ctx.operations->execute_snap_create(payload.snap_namespace, + payload.snap_name, + ctx, 0, false, *prog_ctx); + } + return complete; } else if (r < 0) { encode(ResponseMessage(r), ack_ctx->out); } diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h index 203b6763cf607..130ba63541368 100644 --- a/src/librbd/ImageWatcher.h +++ b/src/librbd/ImageWatcher.h @@ -34,8 +34,10 @@ public: Context *on_finish); void notify_resize(uint64_t request_id, uint64_t size, bool allow_shrink, ProgressContext &prog_ctx, Context *on_finish); - void notify_snap_create(const cls::rbd::SnapshotNamespace &snap_namespace, + void notify_snap_create(uint64_t request_id, + const cls::rbd::SnapshotNamespace &snap_namespace, const std::string &snap_name, + ProgressContext &prog_ctx, Context *on_finish); void notify_snap_rename(const snapid_t &src_snap_id, const std::string &dst_snap_name, @@ -70,7 +72,8 @@ public: static void notify_header_update(librados::IoCtx &io_ctx, const std::string &oid); - void notify_quiesce(uint64_t request_id, Context *on_finish); + void notify_quiesce(uint64_t request_id, ProgressContext &prog_ctx, + Context *on_finish); void notify_unquiesce(uint64_t request_id, Context *on_finish); private: @@ -206,7 +209,8 @@ private: Context *prepare_unquiesce_request(const watch_notify::AsyncRequestId &request); void notify_quiesce(const watch_notify::AsyncRequestId &async_request_id, - size_t attempts, Context *on_finish); + size_t attempts, ProgressContext &prog_ctx, + Context *on_finish); bool handle_payload(const watch_notify::HeaderUpdatePayload& payload, C_NotifyAck *ctx); diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index fce2d1d6c2b88..3e31c71f88920 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -730,13 +730,22 @@ void Operations::snap_create(const cls::rbd::SnapshotNamespace &snap_namespac } m_image_ctx.image_lock.unlock_shared(); + auto prog_ctx = new NoOpProgressContext(); + on_finish = new LambdaContext( + [prog_ctx, on_finish](int r) { + delete prog_ctx; + on_finish->complete(r); + }); + + uint64_t request_id = ++m_async_request_seq; C_InvokeAsyncRequest *req = new C_InvokeAsyncRequest( m_image_ctx, "snap_create", exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, true, boost::bind(&Operations::execute_snap_create, this, snap_namespace, snap_name, - _1, 0, false), + _1, 0, false, boost::ref(*prog_ctx)), boost::bind(&ImageWatcher::notify_snap_create, m_image_ctx.image_watcher, - snap_namespace, snap_name, _1), + request_id, snap_namespace, snap_name, boost::ref(*prog_ctx), + _1), {-EEXIST}, on_finish); req->send(); } @@ -746,7 +755,8 @@ void Operations::execute_snap_create(const cls::rbd::SnapshotNamespace &snap_ const std::string &snap_name, Context *on_finish, uint64_t journal_op_tid, - bool skip_object_map) { + bool skip_object_map, + ProgressContext &prog_ctx) { ceph_assert(ceph_mutex_is_locked(m_image_ctx.owner_lock)); ceph_assert(m_image_ctx.exclusive_lock == nullptr || m_image_ctx.exclusive_lock->is_lock_owner()); @@ -772,7 +782,8 @@ void Operations::execute_snap_create(const cls::rbd::SnapshotNamespace &snap_ 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); + snap_namespace, snap_name, journal_op_tid, request_id, skip_object_map, + prog_ctx); req->send(); } diff --git a/src/librbd/Operations.h b/src/librbd/Operations.h index 253c9f9267333..7c46dedb3aff6 100644 --- a/src/librbd/Operations.h +++ b/src/librbd/Operations.h @@ -52,7 +52,8 @@ public: void execute_snap_create(const cls::rbd::SnapshotNamespace &snap_namespace, const std::string &snap_name, Context *on_finish, - uint64_t journal_op_tid, bool skip_object_map); + uint64_t journal_op_tid, bool skip_object_map, + ProgressContext &prog_ctx); int snap_rollback(const cls::rbd::SnapshotNamespace& snap_namespace, const std::string& snap_name, diff --git a/src/librbd/WatchNotifyTypes.cc b/src/librbd/WatchNotifyTypes.cc index 39599541b61db..6712f899a2516 100644 --- a/src/librbd/WatchNotifyTypes.cc +++ b/src/librbd/WatchNotifyTypes.cc @@ -193,7 +193,9 @@ void SnapPayloadBase::dump(Formatter *f) const { } void SnapCreatePayload::encode(bufferlist &bl) const { + using ceph::encode; SnapPayloadBase::encode(bl); + encode(async_request_id, bl); } void SnapCreatePayload::decode(__u8 version, bufferlist::const_iterator &iter) { @@ -202,9 +204,15 @@ void SnapCreatePayload::decode(__u8 version, bufferlist::const_iterator &iter) { if (version == 5) { decode(snap_namespace, iter); } + if (version >= 7) { + decode(async_request_id, iter); + } } void SnapCreatePayload::dump(Formatter *f) const { + f->open_object_section("async_request_id"); + async_request_id.dump(f); + f->close_section(); SnapPayloadBase::dump(f); } @@ -288,7 +296,7 @@ bool NotifyMessage::check_for_refresh() const { } void NotifyMessage::encode(bufferlist& bl) const { - ENCODE_START(6, 1, bl); + ENCODE_START(7, 1, bl); encode(static_cast(payload->get_notify_op()), bl); payload->encode(bl); ENCODE_FINISH(bl); @@ -385,7 +393,8 @@ void NotifyMessage::generate_test_instances(std::list &o) { o.push_back(new NotifyMessage(new AsyncCompletePayload(AsyncRequestId(ClientId(0, 1), 2), 3))); o.push_back(new NotifyMessage(new FlattenPayload(AsyncRequestId(ClientId(0, 1), 2)))); o.push_back(new NotifyMessage(new ResizePayload(123, true, AsyncRequestId(ClientId(0, 1), 2)))); - o.push_back(new NotifyMessage(new SnapCreatePayload(cls::rbd::UserSnapshotNamespace(), + o.push_back(new NotifyMessage(new SnapCreatePayload(AsyncRequestId(ClientId(0, 1), 2), + cls::rbd::UserSnapshotNamespace(), "foo"))); o.push_back(new NotifyMessage(new SnapRemovePayload(cls::rbd::UserSnapshotNamespace(), "foo"))); o.push_back(new NotifyMessage(new SnapProtectPayload(cls::rbd::UserSnapshotNamespace(), "foo"))); diff --git a/src/librbd/WatchNotifyTypes.h b/src/librbd/WatchNotifyTypes.h index c5fc125774391..e1ac5e1611f48 100644 --- a/src/librbd/WatchNotifyTypes.h +++ b/src/librbd/WatchNotifyTypes.h @@ -47,6 +47,9 @@ struct AsyncRequestId { inline bool operator!=(const AsyncRequestId &rhs) const { return (client_id != rhs.client_id || request_id != rhs.request_id); } + inline operator bool() const { + return (*this != AsyncRequestId()); + } }; enum NotifyOp { @@ -258,10 +261,14 @@ protected: }; struct SnapCreatePayload : public SnapPayloadBase { + AsyncRequestId async_request_id; + SnapCreatePayload() {} - SnapCreatePayload(const cls::rbd::SnapshotNamespace &_snap_namespace, + SnapCreatePayload(const AsyncRequestId &id, + const cls::rbd::SnapshotNamespace &snap_namespace, const std::string &name) - : SnapPayloadBase(_snap_namespace, name) {} + : SnapPayloadBase(snap_namespace, name), async_request_id(id) { + } NotifyOp get_notify_op() const override { return NOTIFY_OP_SNAP_CREATE; diff --git a/src/librbd/deep_copy/SnapshotCreateRequest.cc b/src/librbd/deep_copy/SnapshotCreateRequest.cc index 23ebb1c666d9e..2674da3dd31a8 100644 --- a/src/librbd/deep_copy/SnapshotCreateRequest.cc +++ b/src/librbd/deep_copy/SnapshotCreateRequest.cc @@ -82,9 +82,8 @@ void SnapshotCreateRequest::send_create_snap() { }); std::shared_lock owner_locker{m_dst_image_ctx->owner_lock}; m_dst_image_ctx->operations->execute_snap_create(m_snap_namespace, - m_snap_name.c_str(), - ctx, - 0U, true); + m_snap_name.c_str(), ctx, 0U, + true, m_prog_ctx); } template diff --git a/src/librbd/deep_copy/SnapshotCreateRequest.h b/src/librbd/deep_copy/SnapshotCreateRequest.h index 8c228063a24f3..41f7f54e421e0 100644 --- a/src/librbd/deep_copy/SnapshotCreateRequest.h +++ b/src/librbd/deep_copy/SnapshotCreateRequest.h @@ -9,6 +9,8 @@ #include "common/snap_types.h" #include "librbd/ImageCtx.h" #include "librbd/Types.h" +#include "librbd/internal.h" + #include #include #include @@ -72,6 +74,7 @@ private: Context *m_on_finish; CephContext *m_cct; + NoOpProgressContext m_prog_ctx; void send_set_head(); void handle_set_head(int r); diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 1652efb653514..f5b30e57bb5f1 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -42,7 +42,8 @@ struct ExecuteOp : public Context { image_ctx.operations->execute_snap_create(event.snap_namespace, event.snap_name, on_op_complete, - event.op_tid, false); + event.op_tid, false, + no_op_progress_callback); } void execute(const journal::SnapRemoveEvent &_) { diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc index a8649153a1db2..b16cdf24ffb7d 100644 --- a/src/librbd/operation/SnapshotCreateRequest.cc +++ b/src/librbd/operation/SnapshotCreateRequest.cc @@ -31,10 +31,12 @@ SnapshotCreateRequest::SnapshotCreateRequest(I &image_ctx, const std::string &snap_name, uint64_t journal_op_tid, uint64_t request_id, - bool skip_object_map) + 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_request_id(request_id), m_skip_object_map(skip_object_map), + m_prog_ctx(prog_ctx) { } template @@ -59,7 +61,7 @@ void SnapshotCreateRequest::send_notify_quiesce() { ldout(cct, 5) << this << " " << __func__ << dendl; image_ctx.image_watcher->notify_quiesce( - m_request_id, create_async_context_callback( + m_request_id, 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 ea6d67449c9c6..c4840d38995e5 100644 --- a/src/librbd/operation/SnapshotCreateRequest.h +++ b/src/librbd/operation/SnapshotCreateRequest.h @@ -14,6 +14,7 @@ class Context; namespace librbd { class ImageCtx; +class ProgressContext; namespace operation { @@ -70,7 +71,8 @@ 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); + uint64_t request_id, bool skip_object_map, + ProgressContext &prog_ctx); protected: void send_op() override; @@ -89,6 +91,7 @@ private: std::string m_snap_name; uint64_t m_request_id; bool m_skip_object_map; + ProgressContext &m_prog_ctx; int m_ret_val = 0; diff --git a/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc index 27435b72a254c..17ed8b71cf144 100644 --- a/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_SnapshotCreateRequest.cc @@ -107,7 +107,8 @@ public: void expect_snap_create(librbd::MockTestImageCtx &mock_image_ctx, const std::string &snap_name, uint64_t snap_id, int r) { - EXPECT_CALL(*mock_image_ctx.operations, execute_snap_create(_, StrEq(snap_name), _, 0, true)) + EXPECT_CALL(*mock_image_ctx.operations, + execute_snap_create(_, StrEq(snap_name), _, 0, true, _)) .WillOnce(DoAll(InvokeWithoutArgs([&mock_image_ctx, snap_id, snap_name]() { inject_snap(mock_image_ctx, snap_id, snap_name); }), diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index 452011146b087..37a5ce7536fb9 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -219,7 +219,7 @@ public: Context **on_finish, const char *snap_name, uint64_t op_tid) { EXPECT_CALL(*mock_image_ctx.operations, execute_snap_create(_, StrEq(snap_name), _, - op_tid, false)) + op_tid, false, _)) .WillOnce(DoAll(SaveArg<2>(on_finish), NotifyInvoke(&m_invoke_lock, &m_invoke_cond))); } diff --git a/src/test/librbd/mock/MockImageWatcher.h b/src/test/librbd/mock/MockImageWatcher.h index 8044c9e97653f..0cca579b297a5 100644 --- a/src/test/librbd/mock/MockImageWatcher.h +++ b/src/test/librbd/mock/MockImageWatcher.h @@ -10,6 +10,8 @@ class Context; namespace librbd { +class ProgressContext; + struct MockImageWatcher { MOCK_METHOD0(is_registered, bool()); MOCK_METHOD0(is_unregistered, bool()); @@ -22,8 +24,8 @@ struct MockImageWatcher { MOCK_METHOD0(notify_acquired_lock, void()); MOCK_METHOD0(notify_released_lock, void()); MOCK_METHOD0(notify_request_lock, void()); - - MOCK_METHOD2(notify_quiesce, void(uint64_t, Context *)); + + MOCK_METHOD3(notify_quiesce, void(uint64_t, ProgressContext &, Context *)); MOCK_METHOD2(notify_unquiesce, void(uint64_t, Context *)); }; diff --git a/src/test/librbd/mock/MockOperations.h b/src/test/librbd/mock/MockOperations.h index 49876d197c5b3..ca30e55defe4c 100644 --- a/src/test/librbd/mock/MockOperations.h +++ b/src/test/librbd/mock/MockOperations.h @@ -28,11 +28,12 @@ struct MockOperations { MOCK_METHOD3(snap_create, void(const cls::rbd::SnapshotNamespace &snapshot_namespace, const std::string &snap_name, Context *on_finish)); - MOCK_METHOD5(execute_snap_create, void(const cls::rbd::SnapshotNamespace &snapshot_namespace, + MOCK_METHOD6(execute_snap_create, void(const cls::rbd::SnapshotNamespace &snapshot_namespace, const std::string &snap_name, Context *on_finish, uint64_t journal_op_tid, - bool skip_object_map)); + bool skip_object_map, + ProgressContext &prog_ctx)); MOCK_METHOD3(snap_remove, void(const cls::rbd::SnapshotNamespace &snap_namespace, const std::string &snap_name, Context *on_finish)); diff --git a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc index e32eda54bc675..905199d9e7295 100644 --- a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc +++ b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc @@ -63,8 +63,8 @@ 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<1>( + EXPECT_CALL(*mock_image_ctx.image_watcher, notify_quiesce(_, _, _)) + .WillOnce(WithArg<2>( CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue))); } @@ -184,9 +184,10 @@ TEST_F(TestMockOperationSnapshotCreateRequest, Success) { expect_notify_unquiesce(mock_image_ctx, -EINVAL); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false); + "snap1", 0, 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -206,9 +207,10 @@ TEST_F(TestMockOperationSnapshotCreateRequest, NotifyQuiesceError) { expect_notify_quiesce(mock_image_ctx, -EINVAL); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false); + "snap1", 0, 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -238,9 +240,10 @@ TEST_F(TestMockOperationSnapshotCreateRequest, AllocateSnapIdError) { expect_notify_unquiesce(mock_image_ctx, 0); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false); + "snap1", 0, 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -279,9 +282,10 @@ TEST_F(TestMockOperationSnapshotCreateRequest, CreateSnapStale) { expect_notify_unquiesce(mock_image_ctx, 0); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false); + "snap1", 0, 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -312,9 +316,10 @@ TEST_F(TestMockOperationSnapshotCreateRequest, CreateSnapError) { expect_notify_unquiesce(mock_image_ctx, 0); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false); + "snap1", 0, 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -345,9 +350,10 @@ TEST_F(TestMockOperationSnapshotCreateRequest, ReleaseSnapIdError) { expect_notify_unquiesce(mock_image_ctx, 0); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, false); + "snap1", 0, 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -384,9 +390,10 @@ TEST_F(TestMockOperationSnapshotCreateRequest, SkipObjectMap) { expect_notify_unquiesce(mock_image_ctx, 0); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), - "snap1", 0, 0, true); + "snap1", 0, 0, true, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); @@ -428,11 +435,12 @@ TEST_F(TestMockOperationSnapshotCreateRequest, SetImageState) { expect_notify_unquiesce(mock_image_ctx, 0); C_SaferCond cond_ctx; + librbd::NoOpProgressContext prog_ctx; MockSnapshotCreateRequest *req = new MockSnapshotCreateRequest( mock_image_ctx, &cond_ctx, cls::rbd::MirrorSnapshotNamespace{ cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY, {}, "", CEPH_NOSNAP}, - "snap1", 0, 0, false); + "snap1", 0, 0, false, prog_ctx); { std::shared_lock owner_locker{mock_image_ctx.owner_lock}; req->send(); diff --git a/src/test/librbd/test_ImageWatcher.cc b/src/test/librbd/test_ImageWatcher.cc index 5d4824aefa09a..96fdafef2e344 100644 --- a/src/test/librbd/test_ImageWatcher.cc +++ b/src/test/librbd/test_ImageWatcher.cc @@ -154,7 +154,7 @@ public: bufferlist payload = m_notify_payloads[op]; auto iter = payload.cbegin(); - + switch (op) { case NOTIFY_OP_FLATTEN: { @@ -170,6 +170,13 @@ public: *id = payload.async_request_id; } return true; + case NOTIFY_OP_SNAP_CREATE: + { + SnapCreatePayload payload; + payload.decode(7, iter); + *id = payload.async_request_id; + } + return true; case NOTIFY_OP_REBUILD_OBJECT_MAP: { RebuildObjectMapPayload payload; @@ -275,6 +282,23 @@ struct ResizeTask { } }; +struct SnapCreateTask { + librbd::ImageCtx *ictx; + ProgressContext *progress_context; + int result; + + SnapCreateTask(librbd::ImageCtx *ictx_, ProgressContext *ctx) + : ictx(ictx_), progress_context(ctx), result(0) {} + + void operator()() { + std::shared_lock l{ictx->owner_lock}; + C_SaferCond ctx; + ictx->image_watcher->notify_snap_create(0, cls::rbd::UserSnapshotNamespace(), + "snap", *progress_context, &ctx); + ASSERT_EQ(0, ctx.wait()); + } +}; + struct RebuildObjectMapTask { librbd::ImageCtx *ictx; ProgressContext *progress_context; @@ -424,15 +448,27 @@ TEST_F(TestImageWatcher, NotifySnapCreate) { m_notify_acks = {{NOTIFY_OP_SNAP_CREATE, create_response_message(0)}}; - std::shared_lock l{ictx->owner_lock}; - C_SaferCond notify_ctx; - ictx->image_watcher->notify_snap_create(cls::rbd::UserSnapshotNamespace(), - "snap", ¬ify_ctx); - ASSERT_EQ(0, notify_ctx.wait()); + ProgressContext progress_context; + SnapCreateTask snap_create_task(ictx, &progress_context); + boost::thread thread(boost::ref(snap_create_task)); + + ASSERT_TRUE(wait_for_notifies(*ictx)); NotifyOps expected_notify_ops; expected_notify_ops += NOTIFY_OP_SNAP_CREATE; ASSERT_EQ(expected_notify_ops, m_notifies); + + AsyncRequestId async_request_id; + ASSERT_TRUE(extract_async_request_id(NOTIFY_OP_SNAP_CREATE, + &async_request_id)); + + ASSERT_EQ(0, notify_async_progress(ictx, async_request_id, 1, 10)); + ASSERT_TRUE(progress_context.wait(ictx, 1, 10)); + + ASSERT_EQ(0, notify_async_complete(ictx, async_request_id, 0)); + + ASSERT_TRUE(thread.timed_join(boost::posix_time::seconds(10))); + ASSERT_EQ(0, snap_create_task.result); } TEST_F(TestImageWatcher, NotifySnapCreateError) { @@ -449,8 +485,9 @@ TEST_F(TestImageWatcher, NotifySnapCreateError) { std::shared_lock l{ictx->owner_lock}; C_SaferCond notify_ctx; - ictx->image_watcher->notify_snap_create(cls::rbd::UserSnapshotNamespace(), - "snap", ¬ify_ctx); + librbd::NoOpProgressContext prog_ctx; + ictx->image_watcher->notify_snap_create(0, cls::rbd::UserSnapshotNamespace(), + "snap", prog_ctx, ¬ify_ctx); ASSERT_EQ(-EEXIST, notify_ctx.wait()); NotifyOps expected_notify_ops;