From dd2e4c13ff6d88edb25f90af62af16ba825c15c9 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 8 Apr 2015 21:37:50 -0400 Subject: [PATCH] librbd: internal AIO methods no longer return result All failures should be returned via the AioCompletion. Signed-off-by: Jason Dillaman (cherry picked from commit 9ab42d613128ab08c688ddbea93df4c95068b9cd) Conflicts: src/librbd/AioRequest.cc: trivial resolution src/librbd/internal.cc: trivial resolution --- src/librbd/AioRequest.cc | 11 +-- src/librbd/CopyupRequest.cc | 10 +-- src/librbd/ImageWatcher.cc | 7 +- src/librbd/ImageWatcher.h | 6 +- src/librbd/internal.cc | 148 +++++++++++++----------------------- src/librbd/internal.h | 16 ++-- 6 files changed, 71 insertions(+), 127 deletions(-) diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc index 3bbbab9ec6e94..388c28ec659d2 100644 --- 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 a58ec84f6ccda..3d780c6cc1b63 100644 --- 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 c33517697e654..860f7090eb19a 100644 --- 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& restart_op, AioCompletion* c) { +void ImageWatcher::request_lock( + const boost::function& 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 ba126ac2ee6f1..45a97ba939f69 100644 --- 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& restart_op, - AioCompletion* c); + void request_lock(const boost::function& restart_op, + AioCompletion* c); void prepare_unlock(); void cancel_unlock(); int unlock(); @@ -80,7 +80,7 @@ namespace librbd { }; typedef std::pair AsyncRequest; - typedef std::pair, + typedef std::pair, AioCompletion *> AioRequest; class Task { diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index ba5452dcc7a39..a5bbc63a54a88 100644 --- 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 > 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 >& image_extents, - char *buf, bufferlist *pbl, AioCompletion *c, int op_flags) + void aio_read(ImageCtx *ictx, const vector >& 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 eda119e15288b..419f929c48a3d 100644 --- 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 >& 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 >& 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); -- 2.39.5