librbd: cleanup header update notifications

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 0961d0e181718da87bb6f974b43b0dd5c1125af3..ea2a915ea324287a4b6f39806889d88b32026a51 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -1017,4 +1017,17 @@ struct C_InvalidateCache : public Context {
   Journal<ImageCtx> *ImageCtx::create_journal() {
     return new Journal<ImageCtx>(*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 689e941fa6da8a3be9cbba585339b816c244d2b2..3621c739a12125ed94d2eaccd47e8679d86f759a 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -275,6 +275,9 @@ namespace librbd {
     Journal<ImageCtx> *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 42e92cafbae3ee11d06f992b125164ea5912a402..a85b44375aed1d6f0c229d2db3016d5287adc94a 100644 (file)
--- 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 298cc9acbe3be431ce0ae5996cf0028d0ed82748..76af12858a50526acd963d2e4d637ac32b06ac74 100644 (file)
--- 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 ed75c0b85dfa9c45809eb09b5bb26dd939bfcdc5..34d5a09e4309139693efced7635b812ed0f7c8e1 100644 (file)
--- a/src/librbd/Operations.cc
+++ b/src/librbd/Operations.cc
@@ -26,6 +26,33 @@
 
 namespace librbd {
 
+namespace {
+
+template <typename I>
+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 <typename I>
 Operations<I>::Operations(I &image_ctx) : m_image_ctx(image_ctx) {
 }
@@ -63,8 +90,6 @@ int Operations<I>::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<I>::flatten(ProgressContext &prog_ctx, Context *on_finish) {
   }
 
   operation::FlattenRequest<I> *req = new operation::FlattenRequest<I>(
-    m_image_ctx, on_finish, object_size, overlap_objects, snapc, prog_ctx);
+    m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), object_size,
+    overlap_objects, snapc, prog_ctx);
   req->send();
 }
 
@@ -141,8 +167,6 @@ int Operations<I>::rebuild_object_map(ProgressContext &prog_ctx) {
   if (r < 0) {
     return r;
   }
-
-  notify_change();
   return 0;
 }
 
@@ -166,7 +190,8 @@ void Operations<I>::rebuild_object_map(ProgressContext &prog_ctx,
   }
 
   operation::RebuildObjectMapRequest<I> *req =
-    new operation::RebuildObjectMapRequest<I>(m_image_ctx, on_finish, prog_ctx);
+    new operation::RebuildObjectMapRequest<I>(
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), prog_ctx);
   req->send();
 }
 
@@ -206,10 +231,6 @@ int Operations<I>::rename(const char *dstname) {
       return r;
     }
   }
-
-  if (m_image_ctx.old_format) {
-    notify_change();
-  }
   return 0;
 }
 
@@ -225,8 +246,11 @@ void Operations<I>::rename(const char *dstname, Context *on_finish) {
   ldout(cct, 5) << this << " " << __func__ << ": dest_name=" << dstname
                 << dendl;
 
-  operation::RenameRequest<I> *req =
-    new operation::RenameRequest<I>(m_image_ctx, on_finish, dstname);
+  if (m_image_ctx.old_format) {
+    on_finish = new C_NotifyUpdate<I>(m_image_ctx, on_finish);
+  }
+  operation::RenameRequest<I> *req = new operation::RenameRequest<I>(
+    m_image_ctx, on_finish, dstname);
   req->send();
 }
 
@@ -254,7 +278,6 @@ int Operations<I>::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<I>::resize(uint64_t size, ProgressContext &prog_ctx,
   }
 
   operation::ResizeRequest<I> *req = new operation::ResizeRequest<I>(
-    m_image_ctx, on_finish, size, prog_ctx, journal_op_tid, false);
+    m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), size, prog_ctx,
+    journal_op_tid, false);
   req->send();
 }
 
@@ -317,7 +341,6 @@ int Operations<I>::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<I>::snap_create(const char *snap_name, Context *on_finish,
                 << dendl;
 
   operation::SnapshotCreateRequest<I> *req =
-    new operation::SnapshotCreateRequest<I>(m_image_ctx, on_finish, snap_name,
-                                            journal_op_tid);
+    new operation::SnapshotCreateRequest<I>(
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), snap_name,
+      journal_op_tid);
   req->send();
 }
 
@@ -385,7 +409,6 @@ int Operations<I>::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<I>::snap_rollback(const char *snap_name,
 
   // async mode used for journal replay
   operation::SnapshotRollbackRequest<I> *request =
-    new operation::SnapshotRollbackRequest<I>(m_image_ctx, on_finish, snap_name,
-                                              snap_id, new_size, prog_ctx);
+    new operation::SnapshotRollbackRequest<I>(
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), snap_name,
+      snap_id, new_size, prog_ctx);
   request->send();
 }
 
@@ -463,7 +487,6 @@ int Operations<I>::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<I>::snap_remove(const char *snap_name, Context *on_finish) {
   }
 
   operation::SnapshotRemoveRequest<I> *req =
-    new operation::SnapshotRemoveRequest<I>(m_image_ctx, on_finish, snap_name,
-                                            snap_id);
+    new operation::SnapshotRemoveRequest<I>(
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), snap_name,
+      snap_id);
   req->send();
 }
 
@@ -558,7 +582,6 @@ int Operations<I>::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<I>::snap_rename(const uint64_t src_snap_id,
                 << "new_snap_name=" << dst_name << dendl;
 
   operation::SnapshotRenameRequest<I> *req =
-    new operation::SnapshotRenameRequest<I>(m_image_ctx, on_finish, src_snap_id,
-                                            dst_name);
+    new operation::SnapshotRenameRequest<I>(
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), src_snap_id,
+      dst_name);
   req->send();
 }
 
@@ -630,8 +654,6 @@ int Operations<I>::snap_protect(const char *snap_name) {
       return r;
     }
   }
-
-  notify_change();
   return 0;
 }
 
@@ -648,7 +670,8 @@ void Operations<I>::snap_protect(const char *snap_name, Context *on_finish) {
                 << dendl;
 
   operation::SnapshotProtectRequest<I> *request =
-    new operation::SnapshotProtectRequest<I>(m_image_ctx, on_finish, snap_name);
+    new operation::SnapshotProtectRequest<I>(
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), snap_name);
   request->send();
 }
 
@@ -700,8 +723,6 @@ int Operations<I>::snap_unprotect(const char *snap_name) {
       return r;
     }
   }
-
-  notify_change();
   return 0;
 }
 
@@ -718,8 +739,8 @@ void Operations<I>::snap_unprotect(const char *snap_name, Context *on_finish) {
                 << dendl;
 
   operation::SnapshotUnprotectRequest<I> *request =
-    new operation::SnapshotUnprotectRequest<I>(m_image_ctx, on_finish,
-                                               snap_name);
+    new operation::SnapshotUnprotectRequest<I>(
+      m_image_ctx, new C_NotifyUpdate<I>(m_image_ctx, on_finish), snap_name);
   request->send();
 }
 
@@ -800,13 +821,6 @@ int Operations<I>::invoke_async_request(const std::string& request_type,
   return r;
 }
 
-template <typename I>
-void Operations<I>::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<librbd::ImageCtx>;

diff --git a/src/librbd/Operations.h b/src/librbd/Operations.h
index 48fde18899a2ddf606cde3deb2aa34b98aad73f0..d1f4371540036cba5d164fa5db7e5f66f1146d17 100644 (file)
--- a/src/librbd/Operations.h
+++ b/src/librbd/Operations.h
@@ -65,7 +65,6 @@ private:
                            bool permit_snapshot,
                            const boost::function<void(Context*)>& local_request,
                            const boost::function<int()>& remote_request);
-  void notify_change();
 };
 
 } // namespace librbd

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 928f4049d22c8cf9bab7278fe3f03ab9cd08ff24..36beb1d368d482eac57a26d78971ddd1a8e568fd 100644 (file)
--- 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 9fac1c14cad06fb402c8f10aad28d2661220dcbe..b5179df173a7cb61e86bd3efbe94c5b8c3debe1c 100644 (file)
--- 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 0c9b7a1a4a6fa3921fea259e380aef5674af1e98..41a6fbeb6e635687addfa05db6d0fe15d7888b0a 100644 (file)
--- 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 7ae6643bc46584eebe888a4e4672e824d210ea97..c8b245b392dc2d136b5354c2268502a60ecd9307 100644 (file)
--- 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));