From d263cba089b9e3921007d35e32e0aa9237aad48a Mon Sep 17 00:00:00 2001 From: Pawel Wodkowski Date: Thu, 17 Jan 2019 10:14:59 +0100 Subject: [PATCH] lvol, blob: dont fail removal if IO fails Everything need to be removed to get the remove callback called. Otherwise we will end up with dangling devices and user callbacks possibly not called. Fixes #567 Change-Id: I37259f6cd97268060170a6b17a0c0df4d543a224 Signed-off-by: Pawel Wodkowski Signed-off-by: Tomasz Zawadzki Reviewed-on: https://review.gerrithub.io/c/440890 Tested-by: SPDK CI Jenkins Reviewed-by: Darek Stojaczyk Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/bdev/lvol/vbdev_lvol.c | 8 ++++---- lib/blob/blobstore.c | 11 +---------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 13ca16e07..1949442f4 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -318,12 +318,12 @@ _vbdev_lvs_remove_cb(void *cb_arg, int lvserrno) struct spdk_lvs_req *req = lvs_bdev->req; if (lvserrno != 0) { - SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Could not remove lvol store bdev\n"); - } else { - TAILQ_REMOVE(&g_spdk_lvol_pairs, lvs_bdev, lvol_stores); - free(lvs_bdev); + SPDK_INFOLOG(SPDK_LOG_VBDEV_LVOL, "Lvol store removed with error: %d.\n", lvserrno); } + TAILQ_REMOVE(&g_spdk_lvol_pairs, lvs_bdev, lvol_stores); + free(lvs_bdev); + if (req->cb_fn != NULL) { req->cb_fn(req->cb_arg, lvserrno); } diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index a24447feb..69bb7e4d6 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -2507,7 +2507,6 @@ struct spdk_bs_load_ctx { uint32_t page_index; uint32_t cur_page; struct spdk_blob_md_page *page; - bool is_load; spdk_bs_sequence_t *seq; spdk_blob_op_with_handle_complete iter_cb_fn; @@ -2521,13 +2520,7 @@ _spdk_bs_load_ctx_fail(spdk_bs_sequence_t *seq, struct spdk_bs_load_ctx *ctx, in spdk_dma_free(ctx->super); spdk_bs_sequence_finish(seq, bserrno); - /* - * Only free the blobstore when a load fails. If an unload fails (for some reason) - * we want to keep the blobstore in case the caller wants to try again. - */ - if (ctx->is_load) { - _spdk_bs_free(ctx->bs); - } + _spdk_bs_free(ctx->bs); free(ctx); } @@ -3179,7 +3172,6 @@ spdk_bs_load(struct spdk_bs_dev *dev, struct spdk_bs_opts *o, } ctx->bs = bs; - ctx->is_load = true; ctx->iter_cb_fn = opts.iter_cb_fn; ctx->iter_cb_arg = opts.iter_cb_arg; @@ -3844,7 +3836,6 @@ spdk_bs_unload(struct spdk_blob_store *bs, spdk_bs_op_complete cb_fn, void *cb_a } ctx->bs = bs; - ctx->is_load = false; ctx->super = spdk_dma_zmalloc(sizeof(*ctx->super), 0x1000, NULL); if (!ctx->super) {