From: Jason Dillaman Date: Thu, 9 Apr 2015 01:37:50 +0000 (-0400) Subject: librbd: internal AIO methods no longer return result X-Git-Tag: v9.0.2~116^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9ab42d613128ab08c688ddbea93df4c95068b9cd;p=ceph.git librbd: internal AIO methods no longer return result All failures should be returned via the AioCompletion. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc index ed48c28f0252..c7240c9ffd45 100644 --- a/src/librbd/AioRequest.cc +++ b/src/librbd/AioRequest.cc @@ -267,15 +267,8 @@ namespace librbd { << " parent completion " << m_parent_completion << " extents " << parent_extents << dendl; - int r = aio_read(m_ictx->parent, parent_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, parent_extents, NULL, &m_read_data, + m_parent_completion, 0); } /** write **/ diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc index 8851b4c4a150..6e8f922c54d0 100644 --- a/src/librbd/CopyupRequest.cc +++ b/src/librbd/CopyupRequest.cc @@ -186,15 +186,7 @@ private: << ", 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 2b0d93cabaeb..e7b7a1ba3dd9 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -180,8 +180,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); @@ -194,7 +194,7 @@ int ImageWatcher::request_lock( c->get(); m_aio_requests.push_back(std::make_pair(restart_op, c)); if (request_pending) { - return 0; + return; } } @@ -207,7 +207,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 ded4fe928763..73538148f291 100644 --- a/src/librbd/ImageWatcher.h +++ b/src/librbd/ImageWatcher.h @@ -38,8 +38,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(); @@ -84,7 +84,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 fbe9e5a58d32..27b300e78ac5 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -2634,13 +2634,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; @@ -2683,20 +2677,15 @@ reprotect_and_return_err: SimpleThrottle throttle(src->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); } @@ -3200,12 +3189,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) @@ -3483,12 +3467,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) @@ -3518,12 +3497,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) @@ -3557,12 +3531,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_discard(ictx, off, mylen, c); - if (r < 0) { - c->release(); - delete ctx; - return r; - } + aio_discard(ictx, off, mylen, c); mylock.Lock(); while (!done) @@ -3674,20 +3643,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); @@ -3706,8 +3675,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) @@ -3761,16 +3728,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); @@ -3783,25 +3752,27 @@ reprotect_and_return_err: // pending async operation RWLock::RLocker snap_locker(ictx->snap_lock); if (ictx->snap_id != CEPH_NOSNAP || ictx->read_only) { - return -EROFS; + c->fail(cct, -EROFS); + return; } r = clip_io(ictx, off, &clip_len); if (r < 0) { - return r; + c->fail(cct, r); + return; } snapc = ictx->snapc; - 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 @@ -3833,19 +3804,15 @@ 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_wr); ictx->perfcounter->inc(l_librbd_wr_bytes, clip_len); - return r; } int metadata_get(ImageCtx *ictx, const string &key, string *value) @@ -3901,16 +3868,18 @@ reprotect_and_return_err: return cls_client::metadata_list(&ictx->md_ctx, ictx->header_oid, start, max, pairs); } - - 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); @@ -3923,26 +3892,28 @@ reprotect_and_return_err: // pending async operation RWLock::RLocker snap_locker(ictx->snap_lock); if (ictx->snap_id != CEPH_NOSNAP || ictx->read_only) { - return -EROFS; + c->fail(cct, -EROFS); + return; } r = clip_io(ictx, off, &clip_len); if (r < 0) { - return r; + c->fail(cct, r); + return; } // TODO: check for snap snapc = ictx->snapc; - 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 @@ -3973,13 +3944,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); @@ -3987,8 +3954,6 @@ reprotect_and_return_err: c->finish_adding_requests(ictx->cct); c->put(); - - return r; } void rbd_req_cb(completion_t cb, void *arg) @@ -3998,13 +3963,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 { @@ -4065,14 +4030,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 @@ -4097,22 +4065,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; @@ -4139,24 +4106,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_rd); ictx->perfcounter->inc(l_librbd_rd_bytes, buffer_ofs); - - return ret; } AioCompletion *aio_create_completion() { diff --git a/src/librbd/internal.h b/src/librbd/internal.h index bd7ce0df3fff..860654f2805f 100644 --- a/src/librbd/internal.h +++ b/src/librbd/internal.h @@ -194,14 +194,14 @@ namespace librbd { int async_rebuild_object_map(ImageCtx *ictx, Context *ctx, 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);