librbd: delay completion of AioRequest::read_from_parent

If the object map is enabled, it's possible for a read request to
instantly complete due to the skipped librados operations.  Now
AioRequest will block the completion of read_from_parent requests
to prevent the possibility of the parent image being closed while
the read_from_parent method invocation is in-progress.

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

diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc
index 9ed4b0ef527ac4011f24efa65d538c124007e3f4..37fda2bf08c78fbf4cc2d15e761eb328ec7355f7 100644 (file)
--- a/src/librbd/AioRequest.cc
+++ b/src/librbd/AioRequest.cc
@@ -60,10 +60,18 @@ namespace librbd {
     }
   }
 
-  void AioRequest::read_from_parent(vector<pair<uint64_t,uint64_t> >& image_extents)
+  void AioRequest::read_from_parent(vector<pair<uint64_t,uint64_t> >& image_extents,
+                                    bool block_completion)
   {
     assert(!m_parent_completion);
     m_parent_completion = aio_create_completion_internal(this, rbd_req_cb);
+    if (block_completion) {
+      // prevent the parent image from being deleted while this
+      // request is still in-progress
+      m_parent_completion->get();
+      m_parent_completion->block();
+    }
+
     ldout(m_ictx->cct, 20) << "read_from_parent this = " << this
                           << " parent completion " << m_parent_completion
                           << " extents " << image_extents
@@ -135,36 +143,46 @@ namespace librbd {
 
       // This is the step to read from parent
       if (!m_tried_parent && r == -ENOENT) {
-        RWLock::RLocker l(m_ictx->snap_lock);
-        RWLock::RLocker l2(m_ictx->parent_lock);
-
-        if (m_ictx->parent == NULL) {
-         ldout(m_ictx->cct, 20) << "parent is gone; do nothing" << dendl;
-         m_state = LIBRBD_AIO_READ_FLAT;
-         finished = false;
-         break;
-       }
+        {
+          RWLock::RLocker l(m_ictx->snap_lock);
+          RWLock::RLocker l2(m_ictx->parent_lock);
+          if (m_ictx->parent == NULL) {
+           ldout(m_ictx->cct, 20) << "parent is gone; do nothing" << dendl;
+           m_state = LIBRBD_AIO_READ_FLAT;
+           finished = false;
+           break;
+         }
 
-        // calculate reverse mapping onto the image
-        vector<pair<uint64_t,uint64_t> > image_extents;
-        Striper::extent_to_file(m_ictx->cct, &m_ictx->layout,
-                               m_object_no, m_object_off, m_object_len,
-                               image_extents);
+          // calculate reverse mapping onto the image
+          vector<pair<uint64_t,uint64_t> > image_extents;
+          Striper::extent_to_file(m_ictx->cct, &m_ictx->layout,
+                                 m_object_no, m_object_off, m_object_len,
+                                 image_extents);
 
-        uint64_t image_overlap = 0;
-        r = m_ictx->get_parent_overlap(m_snap_id, &image_overlap);
-        if (r < 0) {
-          assert(0 == "FIXME");
+          uint64_t image_overlap = 0;
+          r = m_ictx->get_parent_overlap(m_snap_id, &image_overlap);
+          if (r < 0) {
+            assert(0 == "FIXME");
+          }
+          uint64_t object_overlap = m_ictx->prune_parent_extents(image_extents,
+                                                                 image_overlap);
+          if (object_overlap) {
+            m_tried_parent = true;
+            if (is_copy_on_read(m_ictx, m_snap_id)) {
+              m_state = LIBRBD_AIO_READ_COPYUP;
+           }
+
+            read_from_parent(image_extents, true);
+            finished = false;
+          }
         }
-        uint64_t object_overlap = m_ictx->prune_parent_extents(image_extents, image_overlap);
-        if (object_overlap) {
-          m_tried_parent = true;
-          if (is_copy_on_read(m_ictx, m_snap_id)) {
-            m_state = LIBRBD_AIO_READ_COPYUP; 
-         }
 
-          read_from_parent(image_extents);
-          finished = false;
+        if (m_tried_parent) {
+          // release reference to the parent read completion.  this request
+          // might be completed after unblock is invoked.
+          AioCompletion *parent_completion = m_parent_completion;
+          parent_completion->unblock(m_ictx->cct);
+          parent_completion->put();
         }
       }
       break;
@@ -355,7 +373,7 @@ namespace librbd {
               m_ictx->copyup_list_lock.Unlock();
             }
           } else {
-            read_from_parent(m_object_image_extents);
+            read_from_parent(m_object_image_extents, false);
           }
        } else {
          ldout(m_ictx->cct, 20) << "should_complete(" << this

diff --git a/src/librbd/AioRequest.h b/src/librbd/AioRequest.h
index 7c16ca50ed279e68e1bab585ec01b8625681c1d6..60be8fc978eed648ef1b02e2c6de17c409189358 100644 (file)
--- a/src/librbd/AioRequest.h
+++ b/src/librbd/AioRequest.h
@@ -40,7 +40,8 @@ namespace librbd {
     virtual int send() = 0;
 
   protected:
-    void read_from_parent(vector<pair<uint64_t,uint64_t> >& image_extents);
+    void read_from_parent(vector<pair<uint64_t,uint64_t> >& image_extents,
+                          bool block_completion);
 
     ImageCtx *m_ictx;
     std::string m_oid;