librbd: internal AIO methods no longer return result

All failures should be returned via the AioCompletion.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 9ab42d613128ab08c688ddbea93df4c95068b9cd)

Conflicts:
	src/librbd/AioRequest.cc: trivial resolution
	src/librbd/internal.cc: trivial resolution

diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc
index 3bbbab9ec6e94285dcb9eae218e092182aefe917..388c28ec659d28da70dea2ca17c1e7a3c5b62dcb 100644 (file)
--- a/src/librbd/AioRequest.cc
+++ b/src/librbd/AioRequest.cc
@@ -76,15 +76,8 @@ namespace librbd {
                           << " parent completion " << m_parent_completion
                           << " extents " << image_extents
                           << dendl;
-    int r = aio_read(m_ictx->parent, image_extents, NULL, &m_read_data,
-                    m_parent_completion, 0);
-    if (r < 0) {
-      lderr(m_ictx->cct) << "read_from_parent " << this
-                         << ": error reading from parent: "
-                         << cpp_strerror(r) << dendl;
-      m_parent_completion->release();
-      complete(r);
-    }
+    aio_read(m_ictx->parent, image_extents, NULL, &m_read_data,
+             m_parent_completion, 0);
   }
 
   static inline bool is_copy_on_read(ImageCtx *ictx, librados::snap_t snap_id) {

diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index a58ec84f6ccdaa43cc6c60d3051c7bd09ea101c2..3d780c6cc1b635bd1dbd6e94ba076b1dd540b9c6 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -91,15 +91,7 @@ namespace librbd {
                           << ", oid " << m_oid
                            << ", extents " << m_image_extents
                            << dendl;
-    int r = aio_read(m_ictx->parent, m_image_extents, NULL, &m_copyup_data,
-                    comp, 0);
-    if (r < 0) {
-      lderr(m_ictx->cct) << __func__ << " " << this
-                         << ": error reading from parent: "
-                         << cpp_strerror(r) << dendl;
-      comp->release();
-      complete(r);
-    }
+    aio_read(m_ictx->parent, m_image_extents, NULL, &m_copyup_data, comp, 0);
   }
 
   void CopyupRequest::queue_send()

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index c33517697e65440f6e7d16e38bde610e23ac5e0d..860f7090eb19ac56f7ae2b0f67e5cd068f6bdf92 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -178,8 +178,8 @@ int ImageWatcher::try_lock() {
   return 0;
 }
 
-int ImageWatcher::request_lock(
-    const boost::function<int(AioCompletion*)>& restart_op, AioCompletion* c) {
+void ImageWatcher::request_lock(
+    const boost::function<void(AioCompletion*)>& restart_op, AioCompletion* c) {
   assert(m_image_ctx.owner_lock.is_locked());
   assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED);
 
@@ -192,7 +192,7 @@ int ImageWatcher::request_lock(
     c->get();
     m_aio_requests.push_back(std::make_pair(restart_op, c));
     if (request_pending) {
-      return 0;
+      return;
     }
   }
 
@@ -205,7 +205,6 @@ int ImageWatcher::request_lock(
       boost::bind(&ImageWatcher::notify_request_lock, this));
     m_task_finisher->queue(TASK_CODE_REQUEST_LOCK, ctx);
   }
-  return 0;
 }
 
 bool ImageWatcher::try_request_lock() {

diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h
index ba126ac2ee6f18f9cb89bbde9de8c5e2310d020a..45a97ba939f691f68b8795267f72594168c21777 100644 (file)
--- a/src/librbd/ImageWatcher.h
+++ b/src/librbd/ImageWatcher.h
@@ -37,8 +37,8 @@ namespace librbd {
     int unregister_watch();
 
     int try_lock();
-    int request_lock(const boost::function<int(AioCompletion*)>& restart_op,
-                    AioCompletion* c);
+    void request_lock(const boost::function<void(AioCompletion*)>& restart_op,
+                     AioCompletion* c);
     void prepare_unlock();
     void cancel_unlock();
     int unlock();
@@ -80,7 +80,7 @@ namespace librbd {
     };
 
     typedef std::pair<Context *, ProgressContext *> AsyncRequest;
-    typedef std::pair<boost::function<int(AioCompletion *)>,
+    typedef std::pair<boost::function<void(AioCompletion *)>,
                      AioCompletion *> AioRequest;
 
     class Task {

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index ba5452dcc7a398c9148a1bf314b21c78a1dd73e8..a5bbc63a54a88d90539caf4184485023319bf787 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2294,13 +2294,7 @@ reprotect_and_return_err:
 
       Context *ctx = new C_CopyWrite(m_throttle, m_bl);
       AioCompletion *comp = aio_create_completion_internal(ctx, rbd_ctx_cb);
-      r = aio_write(m_dest, m_offset, m_bl->length(), m_bl->c_str(), comp, 0);
-      if (r < 0) {
-       ctx->complete(r);
-       comp->release();
-       lderr(m_dest->cct) << "error writing to destination image at offset "
-                          << m_offset << ": " << cpp_strerror(r) << dendl;
-      }
+      aio_write(m_dest, m_offset, m_bl->length(), m_bl->c_str(), comp, 0);
     }
   private:
     SimpleThrottle *m_throttle;
@@ -2329,20 +2323,15 @@ reprotect_and_return_err:
     SimpleThrottle throttle(cct->_conf->rbd_concurrent_management_ops, false);
     uint64_t period = src->get_stripe_period();
     for (uint64_t offset = 0; offset < src_size; offset += period) {
+      if (throttle.pending_error()) {
+        return throttle.wait_for_ret();
+      }
+
       uint64_t len = min(period, src_size - offset);
       bufferlist *bl = new bufferlist();
       Context *ctx = new C_CopyRead(&throttle, dest, offset, bl);
       AioCompletion *comp = aio_create_completion_internal(ctx, rbd_ctx_cb);
-      r = aio_read(src, offset, len, NULL, bl, comp, 0);
-      if (r < 0) {
-       ctx->complete(r);
-       comp->release();
-       throttle.wait_for_ret();
-       lderr(cct) << "could not read from source image from "
-                  << offset << " to " << offset + len << ": "
-                  << cpp_strerror(r) << dendl;
-       return r;
-      }
+      aio_read(src, offset, len, NULL, bl, comp, 0);
       prog_ctx.update_progress(offset, src_size);
     }
 
@@ -2793,12 +2782,7 @@ reprotect_and_return_err:
 
       Context *ctx = new C_SafeCond(&mylock, &cond, &done, &ret);
       AioCompletion *c = aio_create_completion_internal(ctx, rbd_ctx_cb);
-      r = aio_read(ictx, off, read_len, NULL, &bl, c, 0);
-      if (r < 0) {
-       c->release();
-       delete ctx;
-       return r;
-      }
+      aio_read(ictx, off, read_len, NULL, &bl, c, 0);
 
       mylock.Lock();
       while (!done)
@@ -3031,12 +3015,7 @@ reprotect_and_return_err:
 
     Context *ctx = new C_SafeCond(&mylock, &cond, &done, &ret);
     AioCompletion *c = aio_create_completion_internal(ctx, rbd_ctx_cb);
-    int r = aio_read(ictx, image_extents, buf, pbl, c, op_flags);
-    if (r < 0) {
-      c->release();
-      delete ctx;
-      return r;
-    }
+    aio_read(ictx, image_extents, buf, pbl, c, op_flags);
 
     mylock.Lock();
     while (!done)
@@ -3068,12 +3047,7 @@ reprotect_and_return_err:
 
     Context *ctx = new C_SafeCond(&mylock, &cond, &done, &ret);
     AioCompletion *c = aio_create_completion_internal(ctx, rbd_ctx_cb);
-    r = aio_write(ictx, off, mylen, buf, c, op_flags);
-    if (r < 0) {
-      c->release();
-      delete ctx;
-      return r;
-    }
+    aio_write(ictx, off, mylen, buf, c, op_flags);
 
     mylock.Lock();
     while (!done)
@@ -3105,12 +3079,7 @@ reprotect_and_return_err:
 
     Context *ctx = new C_SafeCond(&mylock, &cond, &done, &ret);
     AioCompletion *c = aio_create_completion_internal(ctx, rbd_ctx_cb);
-    int r = aio_discard(ictx, off, len, c);
-    if (r < 0) {
-      c->release();
-      delete ctx;
-      return r;
-    }
+    aio_discard(ictx, off, len, c);
 
     mylock.Lock();
     while (!done)
@@ -3226,20 +3195,20 @@ reprotect_and_return_err:
     return 0;
   }
 
-  int aio_flush(ImageCtx *ictx, AioCompletion *c)
+  void aio_flush(ImageCtx *ictx, AioCompletion *c)
   {
     CephContext *cct = ictx->cct;
     ldout(cct, 20) << "aio_flush " << ictx << " completion " << c <<  dendl;
 
+    c->get();
     int r = ictx_check(ictx);
     if (r < 0) {
-      return r;
+      c->fail(cct, r);
+      return;
     }
 
     ictx->user_flushed();
 
-    c->get();
-
     C_AioWrite *flush_ctx = new C_AioWrite(cct, c);
     c->add_request();
     ictx->flush_async_operations(flush_ctx);
@@ -3258,8 +3227,6 @@ reprotect_and_return_err:
     c->finish_adding_requests(cct);
     c->put();
     ictx->perfcounter->inc(l_librbd_aio_flush);
-
-    return 0;
   }
 
   int flush(ImageCtx *ictx)
@@ -3313,16 +3280,18 @@ reprotect_and_return_err:
     return r;
   }
 
-  int aio_write(ImageCtx *ictx, uint64_t off, size_t len, const char *buf,
-               AioCompletion *c, int op_flags)
+  void aio_write(ImageCtx *ictx, uint64_t off, size_t len, const char *buf,
+                AioCompletion *c, int op_flags)
   {
     CephContext *cct = ictx->cct;
     ldout(cct, 20) << "aio_write " << ictx << " off = " << off << " len = "
                   << len << " buf = " << (void*)buf << dendl;
 
+    c->get();
     int r = ictx_check(ictx);
     if (r < 0) {
-      return r;
+      c->fail(cct, r);
+      return;
     }
 
     RWLock::RLocker owner_locker(ictx->owner_lock);
@@ -3338,7 +3307,8 @@ reprotect_and_return_err:
       RWLock::RLocker snap_locker(ictx->snap_lock);
       r = clip_io(ictx, off, &clip_len);
       if (r < 0) {
-        return r;
+        c->fail(cct, r);
+        return;
       }
 
       snap_id = ictx->snap_id;
@@ -3348,20 +3318,21 @@ reprotect_and_return_err:
       ictx->parent_lock.put_read();
 
       if (snap_id != CEPH_NOSNAP || ictx->read_only) {
-        return -EROFS;
+        c->fail(cct, -EROFS);
+        return;
       }
 
       ldout(cct, 20) << "  parent overlap " << overlap << dendl;
 
-      c->get();
       c->init_time(ictx, AIO_TYPE_WRITE);
     }
 
     if (ictx->image_watcher->is_lock_supported() &&
        !ictx->image_watcher->is_lock_owner()) {
       c->put();
-      return ictx->image_watcher->request_lock(
+      ictx->image_watcher->request_lock(
        boost::bind(&librbd::aio_write, ictx, off, len, buf, _1, op_flags), c);
+      return;
     }
 
     // map
@@ -3401,30 +3372,28 @@ reprotect_and_return_err:
 
        req->set_op_flags(op_flags);
        r = req->send();
-       if (r < 0) {
-         req->complete(r);
-         goto done;
-       }
+        assert(r == 0);
       }
     }
-  done:
+
     c->finish_adding_requests(ictx->cct);
     c->put();
 
     ictx->perfcounter->inc(l_librbd_aio_wr);
     ictx->perfcounter->inc(l_librbd_aio_wr_bytes, clip_len);
-    return r;
   }
 
-  int aio_discard(ImageCtx *ictx, uint64_t off, uint64_t len, AioCompletion *c)
+  void aio_discard(ImageCtx *ictx, uint64_t off, uint64_t len, AioCompletion *c)
   {
     CephContext *cct = ictx->cct;
     ldout(cct, 20) << "aio_discard " << ictx << " off = " << off << " len = "
                   << len << dendl;
 
+    c->get();
     int r = ictx_check(ictx);
     if (r < 0) {
-      return r;
+      c->fail(cct, r);
+      return;
     }
 
     RWLock::RLocker owner_locker(ictx->owner_lock);
@@ -3440,7 +3409,8 @@ reprotect_and_return_err:
       RWLock::RLocker snap_locker(ictx->snap_lock);
       r = clip_io(ictx, off, &clip_len);
       if (r < 0) {
-        return r;
+        c->fail(cct, r);
+        return;
       }
 
       // TODO: check for snap
@@ -3451,18 +3421,19 @@ reprotect_and_return_err:
       ictx->parent_lock.put_read();
 
       if (snap_id != CEPH_NOSNAP || ictx->read_only) {
-        return -EROFS;
+        c->fail(cct, -EROFS);
+        return;
       }
 
-      c->get();
       c->init_time(ictx, AIO_TYPE_DISCARD);
     }
 
     if (ictx->image_watcher->is_lock_supported() &&
        !ictx->image_watcher->is_lock_owner()) {
       c->put();
-      return ictx->image_watcher->request_lock(
+      ictx->image_watcher->request_lock(
        boost::bind(&librbd::aio_discard, ictx, off, len, _1), c);
+      return;
     }
 
     // map
@@ -3502,13 +3473,9 @@ reprotect_and_return_err:
       }
 
       r = req->send();
-      if (r < 0) {
-       req->complete(r);
-       goto done;
-      }
+      assert(r == 0);
     }
-    r = 0;
-  done:
+
     if (ictx->object_cacher) {
       Mutex::Locker l(ictx->cache_lock);
       ictx->object_cacher->discard_set(ictx->object_set, extents);
@@ -3519,7 +3486,6 @@ reprotect_and_return_err:
 
     ictx->perfcounter->inc(l_librbd_aio_discard);
     ictx->perfcounter->inc(l_librbd_aio_discard_bytes, clip_len);
-    return r;
   }
 
   void rbd_req_cb(completion_t cb, void *arg)
@@ -3529,13 +3495,13 @@ reprotect_and_return_err:
     req->complete(comp->get_return_value());
   }
 
-  int aio_read(ImageCtx *ictx, uint64_t off, size_t len,
+  void aio_read(ImageCtx *ictx, uint64_t off, size_t len,
               char *buf, bufferlist *bl,
               AioCompletion *c, int op_flags)
   {
     vector<pair<uint64_t,uint64_t> > image_extents(1);
     image_extents[0] = make_pair(off, len);
-    return aio_read(ictx, image_extents, buf, bl, c, op_flags);
+    aio_read(ictx, image_extents, buf, bl, c, op_flags);
   }
 
   struct C_RBD_Readahead : public Context {
@@ -3597,14 +3563,17 @@ reprotect_and_return_err:
     }
   }
 
-  int aio_read(ImageCtx *ictx, const vector<pair<uint64_t,uint64_t> >& image_extents,
-              char *buf, bufferlist *pbl, AioCompletion *c, int op_flags)
+  void aio_read(ImageCtx *ictx, const vector<pair<uint64_t,uint64_t> >& image_extents,
+               char *buf, bufferlist *pbl, AioCompletion *c, int op_flags)
   {
-    ldout(ictx->cct, 20) << "aio_read " << ictx << " completion " << c << " " << image_extents << dendl;
+    CephContext *cct = ictx->cct;
+    ldout(cct, 20) << "aio_read " << ictx << " completion " << c << " " << image_extents << dendl;
 
+    c->get();
     int r = ictx_check(ictx);
     if (r < 0) {
-      return r;
+      c->fail(cct, r);
+      return;
     }
 
     // readahead
@@ -3632,22 +3601,21 @@ reprotect_and_return_err:
         uint64_t len = p->second;
         r = clip_io(ictx, p->first, &len);
         if (r < 0) {
-         return r;
+          c->fail(cct, r);
+         return;
         }
         if (len == 0) {
          continue;
         }
 
-        Striper::file_to_extents(ictx->cct, ictx->format_string, &ictx->layout,
+        Striper::file_to_extents(cct, ictx->format_string, &ictx->layout,
                                 p->first, len, 0, object_extents, buffer_ofs);
         buffer_ofs += len;
       }
 
-      c->get();
       c->init_time(ictx, AIO_TYPE_READ);
     }
 
-    int64_t ret;
     c->read_buf = buf;
     c->read_buf_len = buffer_ofs;
     c->read_bl = pbl;
@@ -3672,24 +3640,16 @@ reprotect_and_return_err:
                                    cache_comp, op_flags);
        } else {
          r = req->send();
-         if (r == -ENOENT)
-           r = 0;
-         if (r < 0) {
-           ret = r;
-           goto done;
-         }
+          assert(r == 0);
        }
       }
     }
-    ret = buffer_ofs;
-  done:
-    c->finish_adding_requests(ictx->cct);
+
+    c->finish_adding_requests(cct);
     c->put();
 
     ictx->perfcounter->inc(l_librbd_aio_rd);
     ictx->perfcounter->inc(l_librbd_aio_rd_bytes, buffer_ofs);
-
-    return ret;
   }
 
   AioCompletion *aio_create_completion() {

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index eda119e15288b9367d0bc3e024e30af6b45c3cae..419f929c48a3d2b0756cd089e6fa5e0ac7cec2f0 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -197,14 +197,14 @@ namespace librbd {
   void async_resize_helper(ImageCtx *ictx, Context *ctx, uint64_t new_size,
                            ProgressContext& prog_ctx);
 
-  int aio_write(ImageCtx *ictx, uint64_t off, size_t len, const char *buf,
-               AioCompletion *c, int op_flags);
-  int aio_discard(ImageCtx *ictx, uint64_t off, uint64_t len, AioCompletion *c);
-  int aio_read(ImageCtx *ictx, uint64_t off, size_t len,
-              char *buf, bufferlist *pbl, AioCompletion *c, int op_flags);
-  int aio_read(ImageCtx *ictx, const vector<pair<uint64_t,uint64_t> >& image_extents,
-              char *buf, bufferlist *pbl, AioCompletion *c, int op_flags);
-  int aio_flush(ImageCtx *ictx, AioCompletion *c);
+  void aio_write(ImageCtx *ictx, uint64_t off, size_t len, const char *buf,
+                AioCompletion *c, int op_flags);
+  void aio_discard(ImageCtx *ictx, uint64_t off, uint64_t len, AioCompletion *c);
+  void aio_read(ImageCtx *ictx, uint64_t off, size_t len,
+               char *buf, bufferlist *pbl, AioCompletion *c, int op_flags);
+  void aio_read(ImageCtx *ictx, const vector<pair<uint64_t,uint64_t> >& image_extents,
+               char *buf, bufferlist *pbl, AioCompletion *c, int op_flags);
+  void aio_flush(ImageCtx *ictx, AioCompletion *c);
   int flush(ImageCtx *ictx);
   int _flush(ImageCtx *ictx);
   int invalidate_cache(ImageCtx *ictx);