From: Jason Dillaman Date: Tue, 28 Feb 2017 15:18:16 +0000 (-0500) Subject: librbd: cleanup synchronous open/close memory management X-Git-Tag: v12.0.1~203^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bf6d3470320e1ed09c9b5f794b4e2816a2c3aa20;p=ceph.git librbd: cleanup synchronous open/close memory management Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc index dfac0c5b95bb..71a3fdec3da2 100644 --- a/src/librbd/ImageState.cc +++ b/src/librbd/ImageState.cc @@ -248,7 +248,12 @@ template int ImageState::open(bool skip_open_parent) { C_SaferCond ctx; open(skip_open_parent, &ctx); - return ctx.wait(); + + int r = ctx.wait(); + if (r < 0) { + delete m_image_ctx; + } + return r; } template @@ -556,7 +561,7 @@ void ImageState::complete_action_unlock(State next_state, int r) { ctx->complete(r); } - if (next_state != STATE_CLOSED) { + if (next_state != STATE_UNINITIALIZED && next_state != STATE_CLOSED) { m_lock.Lock(); if (!is_transition_state() && !m_actions_contexts.empty()) { execute_next_action_unlock(); diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 082784a14099..6f32a9f99967 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -674,9 +674,9 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, if (r < 0) { lderr(cct) << "error opening image: " << cpp_strerror(r) << dendl; - delete imctx; return r; } + librbd::NoOpProgressContext prog_ctx; r = imctx->operations->flatten(prog_ctx); if (r < 0) { @@ -700,7 +700,12 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, return r; } } - imctx->state->close(); + + r = imctx->state->close(); + if (r < 0) { + lderr(cct) << "failed to close image: " << cpp_strerror(r) << dendl; + return r; + } } pctx.update_progress(++i, size); assert(i <= size); @@ -1045,7 +1050,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, if (r < 0) { lderr(cct) << "error opening parent image: " << cpp_strerror(r) << dendl; - delete p_imctx; return r; } @@ -1186,7 +1190,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, r = c_imctx->state->open(false); if (r < 0) { lderr(cct) << "Error opening new image: " << cpp_strerror(r) << dendl; - delete c_imctx; goto err_remove; } @@ -1289,7 +1292,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, if (r < 0) { lderr(ictx->cct) << "error opening source image: " << cpp_strerror(r) << dendl; - delete ictx; return r; } BOOST_SCOPE_EXIT((ictx)) { @@ -1804,7 +1806,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, dest_md_ctx, false); r = dest->state->open(false); if (r < 0) { - delete dest; lderr(cct) << "failed to read newly created header" << dendl; return r; } @@ -2847,7 +2848,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, if (r < 0) { lderr(cct) << "error opening image "<< img_pair.first << ": " << cpp_strerror(r) << dendl; - delete img_ctx; return r; } @@ -2895,22 +2895,19 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, if (r < 0) { lderr(cct) << "error opening image id "<< img_id << ": " << cpp_strerror(r) << dendl; - delete img_ctx; return r; } r = mirror_image_disable(img_ctx, false); + int close_r = img_ctx->state->close(); if (r < 0) { lderr(cct) << "error disabling mirroring for image id " << img_id << cpp_strerror(r) << dendl; return r; - } - - r = img_ctx->state->close(); - if (r < 0) { + } else if (close_r < 0) { lderr(cct) << "failed to close image id " << img_id << ": " - << cpp_strerror(r) << dendl; - return r; + << cpp_strerror(close_r) << dendl; + return close_r; } } } diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index 51b93f14821d..762696b9da07 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -81,16 +81,13 @@ struct C_OpenComplete : public Context { void **ictxp; bool reopen; C_OpenComplete(librbd::ImageCtx *ictx, librbd::io::AioCompletion* comp, - void **ictxp, bool reopen = false) - : ictx(ictx), comp(comp), ictxp(ictxp), reopen(reopen) { + void **ictxp) + : ictx(ictx), comp(comp), ictxp(ictxp) { comp->init_time(ictx, librbd::io::AIO_TYPE_OPEN); comp->get(); } void finish(int r) override { ldout(ictx->cct, 20) << "C_OpenComplete::finish: r=" << r << dendl; - if (reopen) { - delete reinterpret_cast(*ictxp); - } if (r < 0) { *ictxp = nullptr; comp->fail(r); @@ -115,7 +112,10 @@ struct C_OpenAfterCloseComplete : public Context { void finish(int r) override { ldout(ictx->cct, 20) << "C_OpenAfterCloseComplete::finish: r=" << r << dendl; - ictx->state->open(false, new C_OpenComplete(ictx, comp, ictxp, true)); + delete reinterpret_cast(*ictxp); + *ictxp = nullptr; + + ictx->state->open(false, new C_OpenComplete(ictx, comp, ictxp)); } }; @@ -240,7 +240,6 @@ namespace librbd { int r = ictx->state->open(false); if (r < 0) { - delete ictx; tracepoint(librbd, open_image_exit, r); return r; } @@ -282,7 +281,6 @@ namespace librbd { int r = ictx->state->open(false); if (r < 0) { - delete ictx; tracepoint(librbd, open_image_exit, r); return r; } @@ -2116,9 +2114,7 @@ extern "C" int rbd_open(rados_ioctx_t p, const char *name, rbd_image_t *image, tracepoint(librbd, open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only); int r = ictx->state->open(false); - if (r < 0) { - delete ictx; - } else { + if (r >= 0) { *image = (rbd_image_t)ictx; } tracepoint(librbd, open_image_exit, r); @@ -2152,9 +2148,7 @@ extern "C" int rbd_open_read_only(rados_ioctx_t p, const char *name, tracepoint(librbd, open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only); int r = ictx->state->open(false); - if (r < 0) { - delete ictx; - } else { + if (r >= 0) { *image = (rbd_image_t)ictx; } tracepoint(librbd, open_image_exit, r); @@ -3550,7 +3544,6 @@ extern "C" int rbd_image_get_group(rados_ioctx_t image_p, librbd::ImageCtx *ictx = new librbd::ImageCtx(image_name, "", "", io_ctx, false); int r = ictx->state->open(false); if (r < 0) { - delete ictx; tracepoint(librbd, open_image_exit, r); return r; } diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index 9893832a4be3..c61aaaaf70e7 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -197,7 +197,6 @@ public: ImageCtx *ictx = new ImageCtx("", m_local_image_id, "", m_local_io_ctx, false); EXPECT_EQ(-ENOENT, ictx->state->open(false)); - delete ictx; cls::rbd::MirrorImage mirror_image; EXPECT_EQ(-ENOENT, cls_client::mirror_image_get(&m_local_io_ctx, diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 4f8cbc50736e..fd420872a62b 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -339,7 +339,6 @@ bool ImageDeleter::process_image_delete() { derr << "error opening image id " << m_active_delete->local_image_id << ": " << cpp_strerror(r) << dendl; enqueue_failed_delete(r); - delete imgctx; return true; }