From 94cf9837aadfb27a79f80581c127f39ce7e9c5ff Mon Sep 17 00:00:00 2001 From: Maciej Szwed Date: Fri, 20 Oct 2017 11:43:07 +0200 Subject: [PATCH] lvol: support callbacks in lvol close/destroy functions Close and destroy lvol functions should support callback functions so that they can be processed sequentially and also give user result. Signed-off-by: Maciej Szwed Change-Id: I9e87fb281916c65c17b2b7e54e91228844962048 Reviewed-on: https://review.gerrithub.io/383230 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris --- include/spdk/lvol.h | 4 +- include/spdk_internal/lvolstore.h | 1 + lib/bdev/lvol/vbdev_lvol.c | 16 ++- lib/lvol/lvol.c | 103 ++++++++++++++++-- .../lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c | 13 ++- test/unit/lib/lvol/lvol.c/lvol_ut.c | 32 ++++-- 6 files changed, 144 insertions(+), 25 deletions(-) diff --git a/include/spdk/lvol.h b/include/spdk/lvol.h index 29e7b0bdc..f84d9799d 100644 --- a/include/spdk/lvol.h +++ b/include/spdk/lvol.h @@ -64,8 +64,8 @@ int spdk_lvs_init(struct spdk_bs_dev *bs_dev, struct spdk_lvs_opts *o, int spdk_lvs_unload(struct spdk_lvol_store *lvol_store, spdk_lvs_op_complete cb_fn, void *cb_arg); int spdk_lvol_create(struct spdk_lvol_store *lvs, uint64_t sz, spdk_lvol_op_with_handle_complete cb_fn, void *cb_arg); -void spdk_lvol_destroy(struct spdk_lvol *lvol); -void spdk_lvol_close(struct spdk_lvol *lvol); +void spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg); +void spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg); struct spdk_io_channel *spdk_lvol_get_io_channel(struct spdk_lvol *lvol); struct lvol_store_bdev *vbdev_get_lvs_bdev_by_lvs(struct spdk_lvol_store *lvs_orig); struct spdk_lvol *vbdev_get_lvol_by_name(const char *name); diff --git a/include/spdk_internal/lvolstore.h b/include/spdk_internal/lvolstore.h index ffd32e355..fd807902d 100644 --- a/include/spdk_internal/lvolstore.h +++ b/include/spdk_internal/lvolstore.h @@ -51,6 +51,7 @@ struct spdk_lvs_req { struct spdk_lvol_req { spdk_lvol_op_complete cb_fn; void *cb_arg; + struct spdk_lvol *lvol; }; struct spdk_lvs_with_handle_req { diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index c3012a7ec..77ac10729 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -276,6 +276,18 @@ vbdev_get_lvol_by_name(const char *name) return NULL; } +static void +_vbdev_lvol_close_cb(void *cb_arg, int lvserrno) +{ + SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol closed\n"); +} + +static void +_vbdev_lvol_destroy_cb(void *cb_arg, int lvserrno) +{ + SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol destroyed\n"); +} + static int vbdev_lvol_destruct(void *ctx) { @@ -284,9 +296,9 @@ vbdev_lvol_destruct(void *ctx) assert(lvol != NULL); free(lvol->bdev); if (lvol->close_only) { - spdk_lvol_close(lvol); + spdk_lvol_close(lvol, _vbdev_lvol_close_cb, NULL); } else { - spdk_lvol_destroy(lvol); + spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, NULL); } return 0; diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index 997e70edd..b52861507 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -301,41 +301,82 @@ _spdk_lvs_destruct_cb(void *cb_arg, int lvserrno) static void _spdk_lvol_close_blob_cb(void *cb_arg, int lvolerrno) { - struct spdk_lvol *lvol = cb_arg; + struct spdk_lvol_req *req = cb_arg; + struct spdk_lvol *lvol = req->lvol; if (lvolerrno < 0) { SPDK_ERRLOG("Could not close blob on lvol\n"); free(lvol->name); free(lvol); - return; + 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); } + SPDK_INFOLOG(SPDK_TRACE_LVOL, "Lvol %s closed\n", lvol->name) ; + free(lvol->name); free(lvol); - SPDK_INFOLOG(SPDK_TRACE_LVOL, "Blob closed on lvol\n"); +end: + req->cb_fn(req->cb_arg, lvolerrno); + free(req); } static void _spdk_lvol_delete_blob_cb(void *cb_arg, int lvolerrno) { - struct spdk_lvol *lvol = cb_arg; + struct spdk_lvol_with_handle_req *req = cb_arg; + struct spdk_lvol_req *lvol_req = req->cb_arg; + struct spdk_lvol *lvol = req->lvol; + + if (lvolerrno < 0) { + SPDK_ERRLOG("Could not delete blob on lvol\n"); + free(lvol->name); + free(lvol); + 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); + } + + SPDK_INFOLOG(SPDK_TRACE_LVOL, "Lvol %s deleted\n", lvol->name); + + free(lvol->name); + free(lvol); + +end: + lvol_req->cb_fn(lvol_req->cb_arg, lvolerrno); + free(lvol_req); + free(req); +} + +static void +_spdk_lvol_destroy_cb(void *cb_arg, int lvolerrno) +{ + struct spdk_lvol_with_handle_req *req = cb_arg; + struct spdk_lvol_req *lvol_req = req->cb_arg; + struct spdk_lvol *lvol = req->lvol; struct spdk_blob_store *bs = lvol->lvol_store->blobstore; if (lvolerrno < 0) { SPDK_ERRLOG("Could not delete blob on lvol\n"); free(lvol->name); free(lvol); + lvol_req->cb_fn(lvol_req->cb_arg, lvolerrno); + free(lvol_req); + free(req); return; } - SPDK_INFOLOG(SPDK_TRACE_LVOL, "Blob closed on lvol\n"); - spdk_bs_md_delete_blob(bs, lvol->blob_id, _spdk_lvol_close_blob_cb, lvol); + SPDK_INFOLOG(SPDK_TRACE_LVOL, "Blob closed on lvol %s\n", lvol->name); + + spdk_bs_md_delete_blob(bs, lvol->blob_id, _spdk_lvol_delete_blob_cb, req); } + static void _spdk_lvol_create_open_cb(void *cb_arg, struct spdk_blob *blob, int lvolerrno) { @@ -504,27 +545,69 @@ invalid: } void -spdk_lvol_destroy(struct spdk_lvol *lvol) +spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg) { + struct spdk_lvol_with_handle_req *req; + struct spdk_lvol_req *lvol_req; + + assert(cb_fn != NULL); + if (lvol == NULL) { SPDK_ERRLOG("lvol does not exist\n"); + cb_fn(cb_arg, -ENODEV); return; } + req = calloc(1, sizeof(*req)); + if (!req) { + SPDK_ERRLOG("Cannot alloc memory for lvol request pointer\n"); + cb_fn(cb_arg, -ENOMEM); + return; + } + + lvol_req = calloc(1, sizeof(*lvol_req)); + if (!req) { + SPDK_ERRLOG("Cannot alloc memory for lvol request pointer\n"); + cb_fn(cb_arg, -ENOMEM); + free(req); + return; + } + + lvol_req->cb_fn = cb_fn; + lvol_req->cb_arg = cb_arg; + req->cb_arg = lvol_req; + req->lvol = lvol; + TAILQ_REMOVE(&lvol->lvol_store->lvols, lvol, link); - spdk_bs_md_close_blob(&(lvol->blob), _spdk_lvol_delete_blob_cb, lvol); + spdk_bs_md_close_blob(&(lvol->blob), _spdk_lvol_destroy_cb, req); } void -spdk_lvol_close(struct spdk_lvol *lvol) +spdk_lvol_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg) { + struct spdk_lvol_req *req; + + assert(cb_fn != NULL); + if (lvol == NULL) { SPDK_ERRLOG("lvol does not exist\n"); + cb_fn(cb_arg, -ENODEV); return; } + req = calloc(1, sizeof(*req)); + if (!req) { + SPDK_ERRLOG("Cannot alloc memory for lvol request pointer\n"); + cb_fn(cb_arg, -ENOMEM); + return; + } + + req->cb_fn = cb_fn; + 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, lvol); + spdk_bs_md_close_blob(&(lvol->blob), _spdk_lvol_close_blob_cb, req); } struct spdk_io_channel * 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 38f73b729..d9de1e228 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,8 +175,13 @@ 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_close(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_arg) { struct spdk_lvs_req *destruct_req; @@ -193,13 +198,15 @@ spdk_lvol_close(struct spdk_lvol *lvol) free(lvol->name); free(lvol); g_lvol = NULL; + cb_fn(NULL, 0); } void -spdk_lvol_destroy(struct spdk_lvol *lvol) +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); + spdk_lvol_close(lvol, close_cb, NULL); + cb_fn(NULL, 0); } void diff --git a/test/unit/lib/lvol/lvol.c/lvol_ut.c b/test/unit/lib/lvol/lvol.c/lvol_ut.c index 25f5fdabd..1b75456a0 100644 --- a/test/unit/lib/lvol/lvol.c/lvol_ut.c +++ b/test/unit/lib/lvol/lvol.c/lvol_ut.c @@ -208,6 +208,18 @@ lvol_store_op_complete(void *cb_arg, int lvserrno) g_lvserrno = lvserrno; } +static void +close_cb(void *cb_arg, int lvolerrno) +{ + g_lvserrno = lvolerrno; +} + +static void +destroy_cb(void *cb_arg, int lvolerrno) +{ + g_lvserrno = lvolerrno; +} + static void lvs_init_unload_success(void) { @@ -239,7 +251,8 @@ lvs_init_unload_success(void) SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); /* Lvol has to be closed (or destroyed) before unloading lvol store. */ - spdk_lvol_close(g_lvol); + spdk_lvol_close(g_lvol, close_cb, NULL); + CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; rc = spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL); @@ -317,7 +330,8 @@ lvol_create_destroy_success(void) CU_ASSERT(g_lvserrno == 0); SPDK_CU_ASSERT_FATAL(g_lvol != NULL); - spdk_lvol_destroy(g_lvol); + spdk_lvol_destroy(g_lvol, destroy_cb, NULL); + CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; rc = spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL); @@ -391,8 +405,8 @@ lvol_destroy_fail(void) CU_ASSERT(g_lvserrno == 0); SPDK_CU_ASSERT_FATAL(g_lvol != NULL); - spdk_lvol_destroy(g_lvol); - // nothing to check here... it should just still work + spdk_lvol_destroy(g_lvol, destroy_cb, NULL); + CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; rc = spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL); @@ -425,8 +439,8 @@ lvol_close_fail(void) CU_ASSERT(g_lvserrno == 0); SPDK_CU_ASSERT_FATAL(g_lvol != NULL); - spdk_lvol_close(g_lvol); - // nothing to check here... it should just still work + spdk_lvol_close(g_lvol, close_cb, NULL); + CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; rc = spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL); @@ -460,7 +474,8 @@ lvol_close_success(void) CU_ASSERT(g_lvserrno == 0); SPDK_CU_ASSERT_FATAL(g_lvol != NULL); - spdk_lvol_close(g_lvol); + spdk_lvol_close(g_lvol, close_cb, NULL); + CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; rc = spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL); @@ -526,7 +541,8 @@ lvol_resize(void) CU_ASSERT(rc != 0); CU_ASSERT(g_lvserrno != 0); - spdk_lvol_destroy(g_lvol); + spdk_lvol_destroy(g_lvol, destroy_cb, NULL); + CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; rc = spdk_lvs_unload(g_lvol_store, lvol_store_op_complete, NULL);