From b36face57d2bb1bcb78570a04c002f628e56d23f Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Tue, 24 Jul 2018 04:42:23 -0400 Subject: [PATCH] lvol: remove destruct_req from lvol store Since destroy_lvol_bdev was implemented, it is now more convinient to call destroy/unload for lvol store explicitly. Rather than using lvs struct to pass a request checked on each lvol close/destroy. Change-Id: I56ee626e96f8752909d1584a20fe3345c5607fdc Signed-off-by: Tomasz Zawadzki Reviewed-on: https://review.gerrithub.io/420285 Tested-by: SPDK CI Jenkins Chandler-Test-Pool: SPDK Automated Test System Reviewed-by: Jim Harris Reviewed-by: Ziye Yang Reviewed-by: Ben Walker --- include/spdk_internal/lvolstore.h | 1 - lib/bdev/lvol/vbdev_lvol.c | 29 +++++++++------ lib/lvol/lvol.c | 36 ------------------- .../lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c | 27 -------------- 4 files changed, 18 insertions(+), 75 deletions(-) diff --git a/include/spdk_internal/lvolstore.h b/include/spdk_internal/lvolstore.h index b46069783..064bad5dc 100644 --- a/include/spdk_internal/lvolstore.h +++ b/include/spdk_internal/lvolstore.h @@ -85,7 +85,6 @@ struct spdk_lvol_store { struct spdk_blob *super_blob; spdk_blob_id super_blob_id; struct spdk_uuid uuid; - struct spdk_lvs_req *destruct_req; int lvol_count; int lvols_opened; bool destruct; diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 11db6d6a5..7437e6206 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -342,6 +342,7 @@ _vbdev_lvs_remove_lvol_cb(void *cb_arg, int lvolerrno) } if (TAILQ_EMPTY(&lvs->lvols)) { + spdk_lvs_destroy(lvs, _vbdev_lvs_remove_cb, lvs_bdev); return; } @@ -360,9 +361,23 @@ _vbdev_lvs_remove_lvol_cb(void *cb_arg, int lvolerrno) } static void -_vbdev_lvs_unregister_empty_cb(void *cb_arg, int bdeverrno) +_vbdev_lvs_remove_bdev_unregistered_cb(void *cb_arg, int bdeverrno) { - SPDK_DEBUGLOG(SPDK_LOG_VBDEV_LVOL, "Lvol unregistered with errno %d\n", bdeverrno); + struct lvol_store_bdev *lvs_bdev = cb_arg; + struct spdk_lvol_store *lvs = lvs_bdev->lvs; + struct spdk_lvol *lvol, *tmp; + + if (bdeverrno != 0) { + SPDK_DEBUGLOG(SPDK_LOG_VBDEV_LVOL, "Lvol unregistered with errno %d\n", bdeverrno); + } + + TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) { + if (lvol->ref_count != 0) { + /* An lvol is still open, don't unload whole lvol store. */ + return; + } + } + spdk_lvs_unload(lvs, _vbdev_lvs_remove_cb, lvs_bdev); } static void @@ -409,20 +424,12 @@ _vbdev_lvs_remove(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void spdk_lvs_unload(lvs, _vbdev_lvs_remove_cb, lvs_bdev); } } else { - lvs->destruct_req = calloc(1, sizeof(*lvs->destruct_req)); - if (!lvs->destruct_req) { - SPDK_ERRLOG("Cannot alloc memory for vbdev lvol store request pointer\n"); - _vbdev_lvs_remove_cb(lvs_bdev, -ENOMEM); - return; - } - lvs->destruct_req->cb_fn = _vbdev_lvs_remove_cb; - lvs->destruct_req->cb_arg = lvs_bdev; lvs->destruct = destroy; if (destroy) { _vbdev_lvs_remove_lvol_cb(lvs_bdev, 0); } else { TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) { - spdk_bdev_unregister(lvol->bdev, _vbdev_lvs_unregister_empty_cb, NULL); + spdk_bdev_unregister(lvol->bdev, _vbdev_lvs_remove_bdev_unregistered_cb, lvs_bdev); } } } diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index 90e848aa8..db1c8ae98 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -789,19 +789,6 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, return 0; } -static void -_spdk_lvs_destruct_cb(void *cb_arg, int lvserrno) -{ - struct spdk_lvs_req *req = cb_arg; - - SPDK_INFOLOG(SPDK_LOG_LVOL, "Lvol store bdev deleted\n"); - - if (req->cb_fn != NULL) { - req->cb_fn(req->cb_arg, lvserrno); - } - free(req); -} - static void _lvs_destroy_cb(void *cb_arg, int lvserrno) { @@ -876,8 +863,6 @@ _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno) { struct spdk_lvol_req *req = cb_arg; struct spdk_lvol *lvol = req->lvol; - struct spdk_lvol *iter_lvol, *tmp; - bool all_lvols_closed = true; if (lvolerrno < 0) { SPDK_ERRLOG("Could not close blob on lvol\n"); @@ -886,23 +871,9 @@ _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno) } lvol->ref_count--; - - TAILQ_FOREACH_SAFE(iter_lvol, &lvol->lvol_store->lvols, link, tmp) { - if (iter_lvol->ref_count != 0) { - all_lvols_closed = false; - } - } - lvol->action_in_progress = false; - SPDK_INFOLOG(SPDK_LOG_LVOL, "Lvol %s closed\n", lvol->unique_id); - if (lvol->lvol_store->destruct_req && all_lvols_closed == true) { - if (!lvol->lvol_store->destruct) { - spdk_lvs_unload(lvol->lvol_store, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req); - } - } - end: req->cb_fn(req->cb_arg, lvolerrno); free(req); @@ -929,13 +900,6 @@ _spdk_lvol_delete_blob_cb(void *cb_arg, int lvolerrno) } TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link); - - if (lvol->lvol_store->destruct_req && TAILQ_EMPTY(&lvol->lvol_store->lvols)) { - if (lvol->lvol_store->destruct) { - spdk_lvs_destroy(lvol->lvol_store, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req); - } - } - SPDK_INFOLOG(SPDK_LOG_LVOL, "Lvol %s deleted\n", lvol->unique_id); end: diff --git a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c b/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c index 160512cfa..2d41380ea 100644 --- a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c +++ b/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c @@ -413,28 +413,10 @@ spdk_bdev_get_by_name(const char *bdev_name) void spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg) { - struct spdk_lvs_req *destruct_req; - struct spdk_lvol *iter_lvol, *tmp; - bool all_lvols_closed = true; - lvol->ref_count--; - TAILQ_FOREACH_SAFE(iter_lvol, &lvol->lvol_store->lvols, link, tmp) { - if (iter_lvol->ref_count != 0) { - all_lvols_closed = false; - } - } - SPDK_CU_ASSERT_FATAL(cb_fn != NULL); cb_fn(cb_arg, 0); - - destruct_req = lvol->lvol_store->destruct_req; - if (destruct_req && all_lvols_closed == true) { - if (!lvol->lvol_store->destruct) { - spdk_lvs_unload(lvol->lvol_store, destruct_req->cb_fn, destruct_req->cb_arg); - free(destruct_req); - } - } } bool @@ -446,8 +428,6 @@ spdk_lvol_deletable(struct spdk_lvol *lvol) void spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg) { - struct spdk_lvs_req *destruct_req; - if (lvol->ref_count != 0) { cb_fn(cb_arg, -ENODEV); } @@ -457,13 +437,6 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ SPDK_CU_ASSERT_FATAL(cb_fn != NULL); cb_fn(cb_arg, 0); - destruct_req = lvol->lvol_store->destruct_req; - if (destruct_req && TAILQ_EMPTY(&lvol->lvol_store->lvols)) { - if (lvol->lvol_store->destruct) { - spdk_lvs_destroy(lvol->lvol_store, destruct_req->cb_fn, destruct_req->cb_arg); - free(destruct_req); - } - } g_lvol = NULL; free(lvol->unique_id); free(lvol);