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>
(cherry picked from commit 6064f2346de0a8bf2878bf5bfac9a992cda7c4ca)

diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc
index 413e0e62f930199d2f13deb1c36137ba2f979b30..a01a3d85d474c1c55336b3efcf97e5713df19cd1 100644 (file)
--- a/src/librbd/AioImageRequest.cc
+++ b/src/librbd/AioImageRequest.cc
@@ -82,10 +82,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();
 }
 
@@ -93,10 +90,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();
 }
 
@@ -104,35 +98,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 b30cc30f417a77e6470972ad1457ddc977f17187..0ddbbaf74e4a81a2a108a9db81260b77d61d4dd2 100644 (file)
--- a/src/librbd/AioImageRequest.h
+++ b/src/librbd/AioImageRequest.h
@@ -57,6 +57,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;
 };
 
@@ -78,6 +79,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";
   }
@@ -142,6 +146,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";
   }
@@ -174,6 +181,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";
   }
@@ -203,6 +213,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 22ad364273568a15584f367ce57bea3840660879..55889e8f250e1df78f1c90540698d6f2ecdc4f39 100644 (file)
--- a/src/librbd/AioImageRequestWQ.cc
+++ b/src/librbd/AioImageRequestWQ.cc
@@ -125,6 +125,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();
   }
@@ -151,6 +152,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();
   }
@@ -176,6 +178,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 1246a2a65f3ec4f6741ab27ebd4b15121e149d4d..af34e8237ee937511a188e6ed1012dad62ded90b 100644 (file)
--- a/src/librbd/AioObjectRequest.cc
+++ b/src/librbd/AioObjectRequest.cc
@@ -269,7 +269,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 29d8a4ac40fb5a9c1da30adbf21217c6f8fe4440..4c22f175fc4abaeb2b92d52834c882b0ff007881 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -183,7 +183,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 fd881f67f25e17d8c8b2f359689f3f7523fa5e87..68fd94a6edffcadb26d0905998a9d0a194f09c48 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 579688c8d7ed4b3bc9f0e146aa2fbe82a39947ee..e726b9ef53667af11fd4030ebde7f727849babfa 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2391,7 +2391,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);
@@ -2616,7 +2617,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 1b174bc9f9a1fa671154e6bd0435a20986ca81b8..fb506ec3d72cb24e42b96c2789538e206e0c2c19 100644 (file)
--- a/src/librbd/journal/Replay.cc
+++ b/src/librbd/journal/Replay.cc
@@ -294,6 +294,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);
@@ -315,6 +316,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);
@@ -806,6 +808,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;
@@ -840,8 +843,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;
 }
 
@@ -852,9 +856,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 648b5318cfe67603c99c45e4ad48d9ca77f064d2..8490d3cc98ad1cce0c22193379583522c0705a0a 100644 (file)
--- a/src/librbd/journal/Replay.h
+++ b/src/librbd/journal/Replay.h
@@ -9,6 +9,7 @@
 #include "include/Context.h"
 #include "include/rbd/librbd.hpp"
 #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 037006e0027858760f446e66044fada8826fa0db..ba63588f3b9f95facf181256a93ee8e9d2ebba0f 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 ca12bdda9cd2288395d9b5e2fa9a1bb968b6488f..8f1ee395b53174f554dc19ea79d7e51ea60a51a5 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>;