librbd: ensure that AIO ops are started prior to request

Fixes: http://tracker.ceph.com/issues/16708
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc
index 7292978dba5506e7091b304a3d0d5c54b14466b8..efc895c4db3da1d85c7bfed44fe82095ecb1ea27 100644 (file)
--- a/src/librbd/AioImageRequest.cc
+++ b/src/librbd/AioImageRequest.cc
@@ -79,10 +79,7 @@ void AioImageRequest<I>::aio_read(
     I *ictx, AioCompletion *c,
     const std::vector<std::pair<uint64_t,uint64_t> > &extents,
     char *buf, bufferlist *pbl, int op_flags) {
-  c->init_time(ictx, librbd::AIO_TYPE_READ);
-
   AioImageRead req(*ictx, c, extents, buf, pbl, op_flags);
-  req.start_op();
   req.send();
 }
 
@@ -90,10 +87,7 @@ template <typename I>
 void AioImageRequest<I>::aio_read(I *ictx, AioCompletion *c,
                                   uint64_t off, size_t len, char *buf,
                                   bufferlist *pbl, int op_flags) {
-  c->init_time(ictx, librbd::AIO_TYPE_READ);
-
   AioImageRead req(*ictx, c, off, len, buf, pbl, op_flags);
-  req.start_op();
   req.send();
 }
 
@@ -101,35 +95,29 @@ template <typename I>
 void AioImageRequest<I>::aio_write(I *ictx, AioCompletion *c,
                                    uint64_t off, size_t len, const char *buf,
                                    int op_flags) {
-  c->init_time(ictx, librbd::AIO_TYPE_WRITE);
-
   AioImageWrite req(*ictx, c, off, len, buf, op_flags);
-  req.start_op();
   req.send();
 }
 
 template <typename I>
 void AioImageRequest<I>::aio_discard(I *ictx, AioCompletion *c,
                                      uint64_t off, uint64_t len) {
-  c->init_time(ictx, librbd::AIO_TYPE_DISCARD);
-
   AioImageDiscard req(*ictx, c, off, len);
-  req.start_op();
   req.send();
 }
 
 template <typename I>
 void AioImageRequest<I>::aio_flush(I *ictx, AioCompletion *c) {
-  c->init_time(ictx, librbd::AIO_TYPE_FLUSH);
-
+  assert(c->is_initialized(AIO_TYPE_FLUSH));
   AioImageFlush req(*ictx, c);
-  req.start_op();
   req.send();
 }
 
 template <typename I>
 void AioImageRequest<I>::send() {
   assert(m_image_ctx.owner_lock.is_locked());
+  assert(m_aio_comp->is_initialized(get_aio_type()));
+  assert(m_aio_comp->is_started() ^ (get_aio_type() == AIO_TYPE_FLUSH));
 
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 20) << get_request_type() << ": ictx=" << &m_image_ctx << ", "

diff --git a/src/librbd/AioImageRequest.h b/src/librbd/AioImageRequest.h
index c3987a873105e77b95dc1d315513f8e7ea2a981b..2fbccc2269912b5bfaed54613d43fde23f7c8099 100644 (file)
--- a/src/librbd/AioImageRequest.h
+++ b/src/librbd/AioImageRequest.h
@@ -58,6 +58,7 @@ protected:
     : m_image_ctx(image_ctx), m_aio_comp(aio_comp) {}
 
   virtual void send_request() = 0;
+  virtual aio_type_t get_aio_type() const = 0;
   virtual const char *get_request_type() const = 0;
 };
 
@@ -79,6 +80,9 @@ public:
 
 protected:
   virtual void send_request();
+  virtual aio_type_t get_aio_type() const {
+    return AIO_TYPE_READ;
+  }
   virtual const char *get_request_type() const {
     return "aio_read";
   }
@@ -143,6 +147,9 @@ public:
   }
 
 protected:
+  virtual aio_type_t get_aio_type() const {
+    return AIO_TYPE_WRITE;
+  }
   virtual const char *get_request_type() const {
     return "aio_write";
   }
@@ -175,6 +182,9 @@ public:
   }
 
 protected:
+  virtual aio_type_t get_aio_type() const {
+    return AIO_TYPE_DISCARD;
+  }
   virtual const char *get_request_type() const {
     return "aio_discard";
   }
@@ -204,6 +214,9 @@ public:
 
 protected:
   virtual void send_request();
+  virtual aio_type_t get_aio_type() const {
+    return AIO_TYPE_FLUSH;
+  }
   virtual const char *get_request_type() const {
     return "aio_flush";
   }

diff --git a/src/librbd/AioImageRequestWQ.cc b/src/librbd/AioImageRequestWQ.cc
index 725e96a8770d08d2257a7ddd8b8960311e3b8118..b145edf94bca5650d75ab7a8556ff2bf0f3f1536 100644 (file)
--- a/src/librbd/AioImageRequestWQ.cc
+++ b/src/librbd/AioImageRequestWQ.cc
@@ -122,6 +122,7 @@ void AioImageRequestWQ::aio_read(AioCompletion *c, uint64_t off, uint64_t len,
       lock_required) {
     queue(new AioImageRead(m_image_ctx, c, off, len, buf, pbl, op_flags));
   } else {
+    c->start_op();
     AioImageRequest<>::aio_read(&m_image_ctx, c, off, len, buf, pbl, op_flags);
     finish_in_flight_op();
   }
@@ -148,6 +149,7 @@ void AioImageRequestWQ::aio_write(AioCompletion *c, uint64_t off, uint64_t len,
   if (m_image_ctx.non_blocking_aio || writes_blocked()) {
     queue(new AioImageWrite(m_image_ctx, c, off, len, buf, op_flags));
   } else {
+    c->start_op();
     AioImageRequest<>::aio_write(&m_image_ctx, c, off, len, buf, op_flags);
     finish_in_flight_op();
   }
@@ -173,6 +175,7 @@ void AioImageRequestWQ::aio_discard(AioCompletion *c, uint64_t off,
   if (m_image_ctx.non_blocking_aio || writes_blocked()) {
     queue(new AioImageDiscard(m_image_ctx, c, off, len));
   } else {
+    c->start_op();
     AioImageRequest<>::aio_discard(&m_image_ctx, c, off, len);
     finish_in_flight_op();
   }

diff --git a/src/librbd/AioObjectRequest.cc b/src/librbd/AioObjectRequest.cc
index 152a55fa55febf417005b6284e758efbc6012d2b..d1f34d86dc666b5b517913a7283e4f7c1d788e65 100644 (file)
--- a/src/librbd/AioObjectRequest.cc
+++ b/src/librbd/AioObjectRequest.cc
@@ -268,7 +268,8 @@ namespace librbd {
   void AioObjectRead::read_from_parent(const vector<pair<uint64_t,uint64_t> >& parent_extents)
   {
     assert(!m_parent_completion);
-    m_parent_completion = AioCompletion::create<AioObjectRequest>(this);
+    m_parent_completion = AioCompletion::create_and_start<AioObjectRequest>(
+      this, m_ictx, AIO_TYPE_READ);
 
     // prevent the parent image from being deleted while this
     // request is still in-progress

diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index 484aea8ae2f2ba31c6fdcf7a34218be015241065..da7c43ae286a0fd748b4b46a98f38a903e427214 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -181,7 +181,8 @@ private:
   void CopyupRequest::send()
   {
     m_state = STATE_READ_FROM_PARENT;
-    AioCompletion *comp = AioCompletion::create(this);
+    AioCompletion *comp = AioCompletion::create_and_start(
+      this, m_ictx, AIO_TYPE_READ);
 
     ldout(m_ictx->cct, 20) << __func__ << " " << this
                            << ": completion " << comp

diff --git a/src/librbd/Utils.h b/src/librbd/Utils.h
index 6e231bb928e9b5685d529be31ab905c6adea8bf7..dcdad1d6fbb6bc92b81da73ad5da40279d2f70af 100644 (file)
--- a/src/librbd/Utils.h
+++ b/src/librbd/Utils.h
@@ -154,6 +154,10 @@ Context *create_async_context_callback(I &image_ctx, Context *on_finish) {
       image_ctx.op_work_queue, on_finish);
 }
 
+inline ImageCtx *get_image_ctx(ImageCtx *image_ctx) {
+  return image_ctx;
+}
+
 } // namespace util
 } // namespace librbd
 

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 05a42713cb465dad6809d78da5720452d65a3d78..28ed041cdd90371fab5b2c26adc6cd9aa222aa60 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2440,7 +2440,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       uint64_t len = min(period, src_size - offset);
       bufferlist *bl = new bufferlist();
       Context *ctx = new C_CopyRead(&throttle, dest, offset, bl);
-      AioCompletion *comp = AioCompletion::create(ctx);
+      AioCompletion *comp = AioCompletion::create_and_start(ctx, src,
+                                                            AIO_TYPE_READ);
       AioImageRequest<>::aio_read(src, comp, offset, len, NULL, bl,
                                   fadvise_flags);
       prog_ctx.update_progress(offset, src_size);
@@ -2665,7 +2666,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       bufferlist bl;
 
       C_SaferCond ctx;
-      AioCompletion *c = AioCompletion::create(&ctx);
+      AioCompletion *c = AioCompletion::create_and_start(&ctx, ictx,
+                                                         AIO_TYPE_READ);
       AioImageRequest<>::aio_read(ictx, c, off, read_len, NULL, &bl, 0);
 
       int ret = ctx.wait();

diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc
index e306b708466f66bf395b1aeea75d9e0d062c7739..177a4532a1bd7b09159bf0177a0ee1583256d114 100644 (file)
--- a/src/librbd/journal/Replay.cc
+++ b/src/librbd/journal/Replay.cc
@@ -297,6 +297,7 @@ void Replay<I>::handle_event(const journal::AioDiscardEvent &event,
 
   bool flush_required;
   AioCompletion *aio_comp = create_aio_modify_completion(on_ready, on_safe,
+                                                         AIO_TYPE_DISCARD,
                                                          &flush_required);
   AioImageRequest<I>::aio_discard(&m_image_ctx, aio_comp, event.offset,
                                   event.length);
@@ -318,6 +319,7 @@ void Replay<I>::handle_event(const journal::AioWriteEvent &event,
   bufferlist data = event.data;
   bool flush_required;
   AioCompletion *aio_comp = create_aio_modify_completion(on_ready, on_safe,
+                                                         AIO_TYPE_WRITE,
                                                          &flush_required);
   AioImageRequest<I>::aio_write(&m_image_ctx, aio_comp, event.offset,
                                 event.length, data.c_str(), 0);
@@ -837,6 +839,7 @@ void Replay<I>::handle_op_complete(uint64_t op_tid, int r) {
 template <typename I>
 AioCompletion *Replay<I>::create_aio_modify_completion(Context *on_ready,
                                                        Context *on_safe,
+                                                       aio_type_t aio_type,
                                                        bool *flush_required) {
   Mutex::Locker locker(m_lock);
   CephContext *cct = m_image_ctx.cct;
@@ -871,8 +874,9 @@ AioCompletion *Replay<I>::create_aio_modify_completion(Context *on_ready,
   // when the modification is ACKed by librbd, we can process the next
   // event. when flushed, the completion of the next flush will fire the
   // on_safe callback
-  AioCompletion *aio_comp = AioCompletion::create<Context>(
-    new C_AioModifyComplete(this, on_ready, on_safe));
+  AioCompletion *aio_comp = AioCompletion::create_and_start<Context>(
+    new C_AioModifyComplete(this, on_ready, on_safe),
+    util::get_image_ctx(&m_image_ctx), aio_type);
   return aio_comp;
 }
 
@@ -883,9 +887,10 @@ AioCompletion *Replay<I>::create_aio_flush_completion(Context *on_safe) {
   ++m_in_flight_aio_flush;
 
   // associate all prior write/discard ops to this flush request
-  AioCompletion *aio_comp = AioCompletion::create<Context>(
+  AioCompletion *aio_comp = AioCompletion::create_and_start<Context>(
       new C_AioFlushComplete(this, on_safe,
-                             std::move(m_aio_modify_unsafe_contexts)));
+                             std::move(m_aio_modify_unsafe_contexts)),
+      util::get_image_ctx(&m_image_ctx), AIO_TYPE_FLUSH);
   m_aio_modify_unsafe_contexts.clear();
   return aio_comp;
 }

diff --git a/src/librbd/journal/Replay.h b/src/librbd/journal/Replay.h
index a4faeb7d0d990ee29643f6476414fbd868d3d779..8e5ef6bf33e5d7265d9b43a622ae8bb0df85d411 100644 (file)
--- a/src/librbd/journal/Replay.h
+++ b/src/librbd/journal/Replay.h
@@ -8,6 +8,7 @@
 #include "include/buffer_fwd.h"
 #include "include/Context.h"
 #include "common/Mutex.h"
+#include "librbd/AioCompletion.h"
 #include "librbd/journal/Types.h"
 #include <boost/variant.hpp>
 #include <list>
@@ -171,6 +172,7 @@ private:
 
   AioCompletion *create_aio_modify_completion(Context *on_ready,
                                               Context *on_safe,
+                                              aio_type_t aio_type,
                                               bool *flush_required);
   AioCompletion *create_aio_flush_completion(Context *on_safe);
   void handle_aio_completion(AioCompletion *aio_comp);

diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc
index 87b7d08c6134fce7d5edf65da3311f2a61b51fa6..4a74a092eb5c43ab80da8ba833a7d602c983699e 100644 (file)
--- a/src/test/librbd/journal/test_Replay.cc
+++ b/src/test/librbd/journal/test_Replay.cc
@@ -251,37 +251,12 @@ TEST_F(TestJournalReplay, AioFlushEvent) {
 
   // inject a flush operation into the journal
   inject_into_journal(ictx, librbd::journal::AioFlushEvent());
-
-  // start an AIO write op
-  librbd::Journal<> *journal = ictx->journal;
-  ictx->journal = NULL;
-
-  std::string payload(m_image_size, '1');
-  librbd::AioCompletion *aio_comp = new librbd::AioCompletion();
-  {
-    RWLock::RLocker owner_lock(ictx->owner_lock);
-    librbd::AioImageRequest<>::aio_write(ictx, aio_comp, 0, payload.size(),
-                                         payload.c_str(), 0);
-  }
-  ictx->journal = journal;
   close_image(ictx);
 
   // re-open the journal so that it replays the new entry
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, when_acquired_lock(ictx));
 
-  ASSERT_TRUE(aio_comp->is_complete());
-  ASSERT_EQ(0, aio_comp->wait_for_complete());
-  aio_comp->release();
-
-  std::string read_payload(m_image_size, '\0');
-  aio_comp = new librbd::AioCompletion();
-  ictx->aio_work_queue->aio_read(aio_comp, 0, read_payload.size(),
-                                 &read_payload[0], NULL, 0);
-  ASSERT_EQ(0, aio_comp->wait_for_complete());
-  aio_comp->release();
-  ASSERT_EQ(payload, read_payload);
-
   // check the commit position is properly updated
   int64_t current_tag;
   int64_t current_entry;
@@ -301,7 +276,7 @@ TEST_F(TestJournalReplay, AioFlushEvent) {
   ASSERT_EQ(1, current_entry);
 
   // verify lock ordering constraints
-  aio_comp = new librbd::AioCompletion();
+  librbd::AioCompletion *aio_comp = new librbd::AioCompletion();
   ictx->aio_work_queue->aio_flush(aio_comp);
   ASSERT_EQ(0, aio_comp->wait_for_complete());
   aio_comp->release();

diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc
index 016f217ae2110bf4dc8c47830358cb2438393ea4..750f256226fe82ac31aaa4536bbe81c4f02092fb 100644 (file)
--- a/src/test/librbd/journal/test_mock_Replay.cc
+++ b/src/test/librbd/journal/test_mock_Replay.cc
@@ -54,8 +54,16 @@ struct AioImageRequest<MockReplayImageCtx> {
 
 AioImageRequest<MockReplayImageCtx> *AioImageRequest<MockReplayImageCtx>::s_instance = nullptr;
 
+namespace util {
+
+inline ImageCtx *get_image_ctx(librbd::MockReplayImageCtx *image_ctx) {
+  return image_ctx->image_ctx;
 }
 
+} // namespace util
+
+} // namespace librbd
+
 // template definitions
 #include "librbd/journal/Replay.cc"
 template class librbd::journal::Replay<librbd::MockReplayImageCtx>;