diff --git a/include/spdk_internal/lvolstore.h b/include/spdk_internal/lvolstore.h index fd807902d..c20885ae3 100644 --- a/include/spdk_internal/lvolstore.h +++ b/include/spdk_internal/lvolstore.h @@ -87,6 +87,7 @@ struct spdk_lvol { char *name; bool close_only; struct spdk_bdev *bdev; + int ref_count; TAILQ_ENTRY(spdk_lvol) link; }; diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 77ac10729..af434b81a 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -166,6 +166,7 @@ vbdev_lvs_destruct(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, struct spdk_lvs_req *req; struct lvol_store_bdev *lvs_bdev; struct spdk_lvol *lvol, *tmp; + bool all_lvols_closed = true; lvs_bdev = vbdev_get_lvs_bdev_by_lvs(lvs); TAILQ_REMOVE(&g_spdk_lvol_pairs, lvs_bdev, lvol_stores); @@ -181,7 +182,13 @@ vbdev_lvs_destruct(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, req->cb_fn = cb_fn; req->cb_arg = cb_arg; - if (TAILQ_EMPTY(&lvs->lvols)) { + TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) { + if (lvol->ref_count != 0) { + all_lvols_closed = false; + } + } + + if (all_lvols_closed == true) { spdk_lvs_unload(lvs, _vbdev_lvs_destruct_cb, req); } else { lvs->destruct_req = calloc(1, sizeof(*lvs->destruct_req)); @@ -288,6 +295,15 @@ _vbdev_lvol_destroy_cb(void *cb_arg, int lvserrno) SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol destroyed\n"); } +static void +_vbdev_lvol_destroy_after_close_cb(void *cb_arg, int lvserrno) +{ + struct spdk_lvol *lvol = cb_arg; + + SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol %s closed, begin destroying\n", lvol->name); + spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, NULL); +} + static int vbdev_lvol_destruct(void *ctx) { @@ -298,7 +314,7 @@ vbdev_lvol_destruct(void *ctx) if (lvol->close_only) { spdk_lvol_close(lvol, _vbdev_lvol_close_cb, NULL); } else { - spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, NULL); + spdk_lvol_close(lvol, _vbdev_lvol_destroy_after_close_cb, lvol); } return 0; diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index b52861507..572b83e61 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -249,15 +249,24 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void *cb_arg) { struct spdk_lvs_req *lvs_req; + struct spdk_lvol *lvol, *tmp; if (lvs == NULL) { SPDK_ERRLOG("Lvol store is NULL\n"); return -ENODEV; } - if (!TAILQ_EMPTY(&lvs->lvols)) { - SPDK_ERRLOG("Lvols still open on lvol store\n"); - return -EBUSY; + TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) { + if (lvol->ref_count != 0) { + SPDK_ERRLOG("Lvols still open on lvol store\n"); + return -EBUSY; + } + } + + TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) { + TAILQ_REMOVE(&lvs->lvols, lvol, link); + free(lvol->name); + free(lvol); } lvs_req = calloc(1, sizeof(*lvs_req)); @@ -276,16 +285,6 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, return 0; } -static void -_spdk_lvol_return_to_caller(void *cb_arg, int lvolerrno) -{ - struct spdk_lvol_with_handle_req *req = cb_arg; - - assert(req->cb_fn != NULL); - req->cb_fn(req->cb_arg, req->lvol, lvolerrno); - free(req); -} - static void _spdk_lvs_destruct_cb(void *cb_arg, int lvserrno) { @@ -303,6 +302,8 @@ _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"); @@ -311,14 +312,19 @@ _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno) goto end; } - if (lvol->lvol_store->destruct_req && TAILQ_EMPTY(&lvol->lvol_store->lvols)) { - spdk_lvs_unload(lvol->lvol_store, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req); + 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_INFOLOG(SPDK_TRACE_LVOL, "Lvol %s closed\n", lvol->name) ; - free(lvol->name); - free(lvol); + if (lvol->lvol_store->destruct_req && all_lvols_closed == true) { + spdk_lvs_unload(lvol->lvol_store, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req); + } end: req->cb_fn(req->cb_arg, lvolerrno); @@ -377,6 +383,23 @@ _spdk_lvol_destroy_cb(void *cb_arg, int lvolerrno) } +static void +_spdk_lvol_sync_cb(void *cb_arg, int lvolerrno) +{ + struct spdk_lvol_with_handle_req *req = cb_arg; + struct spdk_lvol *lvol = req->lvol; + + if (lvolerrno != 0) { + spdk_bs_md_close_blob(&lvol->blob, _spdk_lvol_delete_blob_cb, lvol); + } else { + lvol->ref_count++; + } + + assert(req->cb_fn != NULL); + req->cb_fn(req->cb_arg, req->lvol, lvolerrno); + free(req); +} + static void _spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno) { @@ -399,18 +422,19 @@ _spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno) spdk_bs_md_close_blob(&blob, _spdk_lvol_delete_blob_cb, lvol); SPDK_ERRLOG("Cannot alloc memory for lvol name\n"); lvolerrno = -ENOMEM; + free(lvol); goto invalid; } lvolerrno = spdk_bs_md_resize_blob(blob, lvol->num_clusters); if (lvolerrno < 0) { - spdk_bs_md_close_blob(&blob, _spdk_lvol_delete_blob_cb, lvol); + spdk_bs_md_close_blob(&blob, _spdk_lvol_destroy_cb, lvol); goto invalid; } TAILQ_INSERT_TAIL(&lvol->lvol_store->lvols, lvol, link); - spdk_bs_md_sync_blob(blob, _spdk_lvol_return_to_caller, req); + spdk_bs_md_sync_blob(blob, _spdk_lvol_sync_cb, req); return; @@ -549,6 +573,7 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ { struct spdk_lvol_with_handle_req *req; struct spdk_lvol_req *lvol_req; + struct spdk_blob_store *bs = lvol->lvol_store->blobstore; assert(cb_fn != NULL); @@ -558,6 +583,12 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ return; } + if (lvol->ref_count != 0) { + SPDK_ERRLOG("Cannot destroy lvol %s because it is still open\n", lvol->name); + cb_fn(cb_arg, -EBUSY); + return; + } + req = calloc(1, sizeof(*req)); if (!req) { SPDK_ERRLOG("Cannot alloc memory for lvol request pointer\n"); @@ -579,7 +610,7 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ req->lvol = lvol; TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link); - spdk_bs_md_close_blob(&(lvol->blob), _spdk_lvol_destroy_cb, req); + spdk_bs_md_delete_blob(bs, lvol->blob_id, _spdk_lvol_delete_blob_cb, req); } void @@ -606,7 +637,6 @@ spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ar req->cb_arg = cb_arg; req->lvol = lvol; - TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link); spdk_bs_md_close_blob(&(lvol->blob), _spdk_lvol_close_blob_cb, req); } 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 d9de1e228..6efb58631 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 @@ -175,37 +175,39 @@ spdk_bdev_get_by_name(const char *bdev_name) return NULL; } -static void -close_cb(void *cb_arg, int lvolerrno) -{ -} - 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; SPDK_CU_ASSERT_FATAL(lvol == g_lvol); - TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link); + lvol->ref_count--; + + TAILQ_FOREACH_SAFE(iter_lvol, &lvol->lvol_store->lvols, link, tmp) { + if (iter_lvol->ref_count != 0) { + all_lvols_closed = false; + } + } destruct_req = lvol->lvol_store->destruct_req; - if (destruct_req && TAILQ_EMPTY(&lvol->lvol_store->lvols)) { + if (destruct_req && all_lvols_closed == true) { spdk_lvs_unload(lvol->lvol_store, destruct_req->cb_fn, destruct_req->cb_arg); free(destruct_req); } - free(lvol->name); - free(lvol); - g_lvol = NULL; - cb_fn(NULL, 0); + cb_fn(cb_arg, 0); } void spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg) { - /* Lvol destroy and close are effectively the same from UT perspective */ - spdk_lvol_close(lvol, close_cb, NULL); + free(lvol->name); + free(lvol); + g_lvol = NULL; + cb_fn(NULL, 0); } @@ -311,6 +313,7 @@ spdk_lvol_create(struct spdk_lvol_store *lvs, size_t sz, spdk_lvol_op_with_handl SPDK_CU_ASSERT_FATAL(lvol != NULL); lvol->lvol_store = lvs; + lvol->ref_count++; lvol->name = spdk_sprintf_alloc("%s", "UNIT_TEST_UUID"); SPDK_CU_ASSERT_FATAL(lvol->name != NULL); @@ -369,18 +372,19 @@ ut_lvs_destroy(void) uuid_generate_time(lvs->uuid); - /* Suuccessfully create lvol, which should be destroyed with lvs later */ + /* Suuccessfully create lvol, which should be unloaded with lvs later */ g_lvolerrno = -1; rc = vbdev_lvol_create(lvs->uuid, sz, vbdev_lvol_create_complete, NULL); CU_ASSERT(rc == 0); CU_ASSERT(g_lvolerrno == 0); SPDK_CU_ASSERT_FATAL(g_lvol != NULL); - /* Destroy lvol store */ + /* Unload lvol store */ vbdev_lvs_destruct(lvs, lvol_store_op_complete, NULL); CU_ASSERT(g_lvserrno == 0); CU_ASSERT(g_lvol_store == NULL); - CU_ASSERT(g_lvol == NULL); + free(g_lvol->name); + free(g_lvol); } static void diff --git a/test/unit/lib/lvol/lvol.c/lvol_ut.c b/test/unit/lib/lvol/lvol.c/lvol_ut.c index 1b75456a0..9ca6d05c5 100644 --- a/test/unit/lib/lvol/lvol.c/lvol_ut.c +++ b/test/unit/lib/lvol/lvol.c/lvol_ut.c @@ -330,6 +330,8 @@ lvol_create_destroy_success(void) CU_ASSERT(g_lvserrno == 0); SPDK_CU_ASSERT_FATAL(g_lvol != NULL); + spdk_lvol_close(g_lvol, close_cb, NULL); + CU_ASSERT(g_lvserrno == 0); spdk_lvol_destroy(g_lvol, destroy_cb, NULL); CU_ASSERT(g_lvserrno == 0); @@ -405,6 +407,8 @@ lvol_destroy_fail(void) CU_ASSERT(g_lvserrno == 0); SPDK_CU_ASSERT_FATAL(g_lvol != NULL); + spdk_lvol_close(g_lvol, close_cb, NULL); + CU_ASSERT(g_lvserrno == 0); spdk_lvol_destroy(g_lvol, destroy_cb, NULL); CU_ASSERT(g_lvserrno == 0); @@ -541,6 +545,8 @@ lvol_resize(void) CU_ASSERT(rc != 0); CU_ASSERT(g_lvserrno != 0); + spdk_lvol_close(g_lvol, close_cb, NULL); + CU_ASSERT(g_lvserrno == 0); spdk_lvol_destroy(g_lvol, destroy_cb, NULL); CU_ASSERT(g_lvserrno == 0);