From 9d0b5b9e5ea3552740950399ea90737708b7bb8d Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 15 Feb 2016 23:25:28 -0500 Subject: [PATCH] librbd: cleanup header update notifications Signed-off-by: Jason Dillaman --- src/librbd/ImageCtx.cc | 13 ++++ src/librbd/ImageCtx.h | 3 + src/librbd/ImageWatcher.cc | 21 +++---- src/librbd/ImageWatcher.h | 4 +- src/librbd/Operations.cc | 94 ++++++++++++++++------------ src/librbd/Operations.h | 1 - src/librbd/internal.cc | 18 ++---- src/librbd/internal.h | 2 - src/test/librbd/mock/MockImageCtx.h | 3 + src/test/librbd/test_ImageWatcher.cc | 2 +- 10 files changed, 90 insertions(+), 71 deletions(-) diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 0961d0e181718..ea2a915ea3242 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -1017,4 +1017,17 @@ struct C_InvalidateCache : public Context { Journal *ImageCtx::create_journal() { return new Journal(*this); } + + void ImageCtx::notify_update() { + state->handle_update_notification(); + + C_SaferCond ctx; + image_watcher->notify_header_update(&ctx); + ctx.wait(); + } + + void ImageCtx::notify_update(Context *on_finish) { + state->handle_update_notification(); + image_watcher->notify_header_update(on_finish); + } } diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index 689e941fa6da8..3621c739a1212 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -275,6 +275,9 @@ namespace librbd { Journal *create_journal(); void clear_pending_completions(); + + void notify_update(); + void notify_update(Context *on_finish); }; } diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index 42e92cafbae3e..a85b44375aed1 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -1,5 +1,6 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab + #include "librbd/ImageWatcher.h" #include "librbd/AioCompletion.h" #include "librbd/ExclusiveLock.h" @@ -108,8 +109,8 @@ int ImageWatcher::notify_async_progress(const AsyncRequestId &request, bufferlist bl; ::encode(NotifyMessage(AsyncProgressPayload(request, offset, total)), bl); - - m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, NULL); + m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, + Notifier::NOTIFY_TIMEOUT, NULL); return 0; } @@ -129,8 +130,7 @@ int ImageWatcher::notify_async_complete(const AsyncRequestId &request, ::encode(NotifyMessage(AsyncCompletePayload(request, r)), bl); if (r >= 0) { - librbd::notify_change(m_image_ctx.md_ctx, m_image_ctx.header_oid, - &m_image_ctx); + m_image_ctx.notify_update(); } int ret = m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, Notifier::NOTIFY_TIMEOUT, NULL); @@ -251,14 +251,11 @@ int ImageWatcher::notify_rename(const std::string &image_name) { return notify_lock_owner(bl); } -void ImageWatcher::notify_header_update(librados::IoCtx &io_ctx, - const std::string &oid) -{ +void ImageWatcher::notify_header_update(Context *on_finish) { // supports legacy (empty buffer) clients bufferlist bl; ::encode(NotifyMessage(HeaderUpdatePayload()), bl); - - io_ctx.notify2(oid, bl, NOTIFY_TIMEOUT, NULL); + m_notifier.notify(bl, nullptr, on_finish); } void ImageWatcher::schedule_cancel_async_requests() { @@ -300,7 +297,8 @@ void ImageWatcher::notify_acquired_lock() { bufferlist bl; ::encode(NotifyMessage(AcquiredLockPayload(client_id)), bl); - m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, NULL); + m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, + Notifier::NOTIFY_TIMEOUT, NULL); } void ImageWatcher::notify_released_lock() { @@ -313,7 +311,8 @@ void ImageWatcher::notify_released_lock() { bufferlist bl; ::encode(NotifyMessage(ReleasedLockPayload(get_client_id())), bl); - m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, NULL); + m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, + Notifier::NOTIFY_TIMEOUT, NULL); } void ImageWatcher::schedule_request_lock(bool use_timer, int timer_delay) { diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h index 298cc9acbe3be..76af12858a505 100644 --- a/src/librbd/ImageWatcher.h +++ b/src/librbd/ImageWatcher.h @@ -1,5 +1,6 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab + #ifndef CEPH_LIBRBD_IMAGE_WATCHER_H #define CEPH_LIBRBD_IMAGE_WATCHER_H @@ -50,8 +51,7 @@ public: void notify_released_lock(); void notify_request_lock(); - static void notify_header_update(librados::IoCtx &io_ctx, - const std::string &oid); + void notify_header_update(Context *on_finish); uint64_t get_watch_handle() const { RWLock::RLocker watch_locker(m_watch_lock); diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index ed75c0b85dfa9..34d5a09e43091 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -26,6 +26,33 @@ namespace librbd { +namespace { + +template +struct C_NotifyUpdate : public Context { + I &image_ctx; + Context *on_finish; + bool notified = false; + + C_NotifyUpdate(I &image_ctx, Context *on_finish) + : image_ctx(image_ctx), on_finish(on_finish) { + } + + virtual void complete(int r) override { + if (r < 0 || notified) { + Context::complete(r); + } else { + notified = true; + image_ctx.notify_update(this); + } + } + virtual void finish(int r) override { + on_finish->complete(r); + } +}; + +} // anonymous namespace + template Operations::Operations(I &image_ctx) : m_image_ctx(image_ctx) { } @@ -63,8 +90,6 @@ int Operations::flatten(ProgressContext &prog_ctx) { if (r < 0 && r != -EINVAL) { return r; } - - notify_change(); ldout(cct, 20) << "flatten finished" << dendl; return 0; } @@ -115,7 +140,8 @@ void Operations::flatten(ProgressContext &prog_ctx, Context *on_finish) { } operation::FlattenRequest *req = new operation::FlattenRequest( - m_image_ctx, on_finish, object_size, overlap_objects, snapc, prog_ctx); + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), object_size, + overlap_objects, snapc, prog_ctx); req->send(); } @@ -141,8 +167,6 @@ int Operations::rebuild_object_map(ProgressContext &prog_ctx) { if (r < 0) { return r; } - - notify_change(); return 0; } @@ -166,7 +190,8 @@ void Operations::rebuild_object_map(ProgressContext &prog_ctx, } operation::RebuildObjectMapRequest *req = - new operation::RebuildObjectMapRequest(m_image_ctx, on_finish, prog_ctx); + new operation::RebuildObjectMapRequest( + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), prog_ctx); req->send(); } @@ -206,10 +231,6 @@ int Operations::rename(const char *dstname) { return r; } } - - if (m_image_ctx.old_format) { - notify_change(); - } return 0; } @@ -225,8 +246,11 @@ void Operations::rename(const char *dstname, Context *on_finish) { ldout(cct, 5) << this << " " << __func__ << ": dest_name=" << dstname << dendl; - operation::RenameRequest *req = - new operation::RenameRequest(m_image_ctx, on_finish, dstname); + if (m_image_ctx.old_format) { + on_finish = new C_NotifyUpdate(m_image_ctx, on_finish); + } + operation::RenameRequest *req = new operation::RenameRequest( + m_image_ctx, on_finish, dstname); req->send(); } @@ -254,7 +278,6 @@ int Operations::resize(uint64_t size, ProgressContext& prog_ctx) { size, boost::ref(prog_ctx))); m_image_ctx.perfcounter->inc(l_librbd_resize); - notify_change(); ldout(cct, 2) << "resize finished" << dendl; return r; } @@ -282,7 +305,8 @@ void Operations::resize(uint64_t size, ProgressContext &prog_ctx, } operation::ResizeRequest *req = new operation::ResizeRequest( - m_image_ctx, on_finish, size, prog_ctx, journal_op_tid, false); + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), size, prog_ctx, + journal_op_tid, false); req->send(); } @@ -317,7 +341,6 @@ int Operations::snap_create(const char *snap_name) { } m_image_ctx.perfcounter->inc(l_librbd_snap_create); - notify_change(); return 0; } @@ -333,8 +356,9 @@ void Operations::snap_create(const char *snap_name, Context *on_finish, << dendl; operation::SnapshotCreateRequest *req = - new operation::SnapshotCreateRequest(m_image_ctx, on_finish, snap_name, - journal_op_tid); + new operation::SnapshotCreateRequest( + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), snap_name, + journal_op_tid); req->send(); } @@ -385,7 +409,6 @@ int Operations::snap_rollback(const char *snap_name, } m_image_ctx.perfcounter->inc(l_librbd_snap_rollback); - notify_change(); return r; } @@ -414,8 +437,9 @@ void Operations::snap_rollback(const char *snap_name, // async mode used for journal replay operation::SnapshotRollbackRequest *request = - new operation::SnapshotRollbackRequest(m_image_ctx, on_finish, snap_name, - snap_id, new_size, prog_ctx); + new operation::SnapshotRollbackRequest( + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), snap_name, + snap_id, new_size, prog_ctx); request->send(); } @@ -463,7 +487,6 @@ int Operations::snap_remove(const char *snap_name) { } m_image_ctx.perfcounter->inc(l_librbd_snap_remove); - notify_change(); return 0; } @@ -504,8 +527,9 @@ void Operations::snap_remove(const char *snap_name, Context *on_finish) { } operation::SnapshotRemoveRequest *req = - new operation::SnapshotRemoveRequest(m_image_ctx, on_finish, snap_name, - snap_id); + new operation::SnapshotRemoveRequest( + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), snap_name, + snap_id); req->send(); } @@ -558,7 +582,6 @@ int Operations::snap_rename(const char *srcname, const char *dstname) { } m_image_ctx.perfcounter->inc(l_librbd_snap_rename); - notify_change(); return 0; } @@ -577,8 +600,9 @@ void Operations::snap_rename(const uint64_t src_snap_id, << "new_snap_name=" << dst_name << dendl; operation::SnapshotRenameRequest *req = - new operation::SnapshotRenameRequest(m_image_ctx, on_finish, src_snap_id, - dst_name); + new operation::SnapshotRenameRequest( + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), src_snap_id, + dst_name); req->send(); } @@ -630,8 +654,6 @@ int Operations::snap_protect(const char *snap_name) { return r; } } - - notify_change(); return 0; } @@ -648,7 +670,8 @@ void Operations::snap_protect(const char *snap_name, Context *on_finish) { << dendl; operation::SnapshotProtectRequest *request = - new operation::SnapshotProtectRequest(m_image_ctx, on_finish, snap_name); + new operation::SnapshotProtectRequest( + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), snap_name); request->send(); } @@ -700,8 +723,6 @@ int Operations::snap_unprotect(const char *snap_name) { return r; } } - - notify_change(); return 0; } @@ -718,8 +739,8 @@ void Operations::snap_unprotect(const char *snap_name, Context *on_finish) { << dendl; operation::SnapshotUnprotectRequest *request = - new operation::SnapshotUnprotectRequest(m_image_ctx, on_finish, - snap_name); + new operation::SnapshotUnprotectRequest( + m_image_ctx, new C_NotifyUpdate(m_image_ctx, on_finish), snap_name); request->send(); } @@ -800,13 +821,6 @@ int Operations::invoke_async_request(const std::string& request_type, return r; } -template -void Operations::notify_change() { - m_image_ctx.state->handle_update_notification(); - ImageWatcher::notify_header_update(m_image_ctx.md_ctx, - m_image_ctx.header_oid); -} - } // namespace librbd template class librbd::Operations; diff --git a/src/librbd/Operations.h b/src/librbd/Operations.h index 48fde18899a2d..d1f4371540036 100644 --- a/src/librbd/Operations.h +++ b/src/librbd/Operations.h @@ -65,7 +65,6 @@ private: bool permit_snapshot, const boost::function& local_request, const boost::function& remote_request); - void notify_change(); }; } // namespace librbd diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 928f4049d22c8..36beb1d368d48 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -307,16 +307,6 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { return 0; } - int notify_change(IoCtx& io_ctx, const string& oid, ImageCtx *ictx) - { - if (ictx) { - ictx->state->handle_update_notification(); - } - - ImageWatcher::notify_header_update(io_ctx, oid); - return 0; - } - int read_header(IoCtx& io_ctx, const string& header_oid, struct rbd_obj_header_ondisk *header, uint64_t *ver) { @@ -1453,7 +1443,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { } } - notify_change(ictx->md_ctx, ictx->header_oid, ictx); + ictx->notify_update(); return 0; } @@ -1988,7 +1978,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { } } - notify_change(ictx->md_ctx, ictx->header_oid, ictx); + ictx->notify_update(); return 0; } @@ -2010,7 +2000,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { } } - notify_change(ictx->md_ctx, ictx->header_oid, ictx); + ictx->notify_update(); return 0; } @@ -2073,7 +2063,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { RBD_LOCK_NAME, cookie, lock_client); if (r < 0) return r; - notify_change(ictx->md_ctx, ictx->header_oid, ictx); + ictx->notify_update(); return 0; } diff --git a/src/librbd/internal.h b/src/librbd/internal.h index 9fac1c14cad06..b5179df173a7c 100644 --- a/src/librbd/internal.h +++ b/src/librbd/internal.h @@ -142,8 +142,6 @@ namespace librbd { int read_header_bl(librados::IoCtx& io_ctx, const std::string& md_oid, ceph::bufferlist& header, uint64_t *ver); - int notify_change(librados::IoCtx& io_ctx, const std::string& oid, - ImageCtx *ictx); int read_header(librados::IoCtx& io_ctx, const std::string& md_oid, struct rbd_obj_header_ondisk *header, uint64_t *ver); int tmap_set(librados::IoCtx& io_ctx, const std::string& imgname); diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index 0c9b7a1a4a6fa..41a6fbeb6e635 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -141,6 +141,9 @@ struct MockImageCtx { MOCK_METHOD1(create_object_map, MockObjectMap*(uint64_t)); MOCK_METHOD0(create_journal, MockJournal*()); + MOCK_METHOD0(notify_update, void()); + MOCK_METHOD1(notify_update, void(Context *)); + ImageCtx *image_ctx; CephContext *cct; diff --git a/src/test/librbd/test_ImageWatcher.cc b/src/test/librbd/test_ImageWatcher.cc index 7ae6643bc4658..c8b245b392dc2 100644 --- a/src/test/librbd/test_ImageWatcher.cc +++ b/src/test/librbd/test_ImageWatcher.cc @@ -352,7 +352,7 @@ TEST_F(TestImageWatcher, NotifyHeaderUpdate) { ASSERT_EQ(0, register_image_watch(*ictx)); m_notify_acks = {{NOTIFY_OP_HEADER_UPDATE, {}}}; - librbd::ImageWatcher::notify_header_update(m_ioctx, ictx->header_oid); + ictx->notify_update(); ASSERT_TRUE(wait_for_notifies(*ictx)); -- 2.39.5