From 2baeea7dd43483689a430ab2f03091373a626a7b Mon Sep 17 00:00:00 2001 From: Maciej Szwed Date: Wed, 25 Oct 2017 11:11:59 +0200 Subject: [PATCH] bdev: add callback to spdk bdev unregister and bdev destruct Currently deleting bdev does not support asynchronous delete operations. Because of that results are returned before device is actually deleted and some operation can be peformed on that device after removal of this device started. Signed-off-by: Maciej Szwed Change-Id: I305c302d8abd5d7c2c0f947fca70c58396872132 Reviewed-on: https://review.gerrithub.io/383732 Reviewed-by: Daniel Verkamp Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris --- include/spdk_internal/bdev.h | 14 ++++++++--- lib/bdev/bdev.c | 23 +++++++++++++---- lib/bdev/lvol/vbdev_lvol.c | 25 ++++++++++++++++--- lib/bdev/nvme/bdev_nvme.c | 2 +- lib/bdev/rpc/bdev_rpc.c | 25 ++++++++++++------- test/unit/lib/bdev/bdev.c/bdev_ut.c | 6 ++--- test/unit/lib/bdev/mt/bdev.c/bdev_ut.c | 2 +- .../lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c | 9 +++++-- 8 files changed, 78 insertions(+), 28 deletions(-) diff --git a/include/spdk_internal/bdev.h b/include/spdk_internal/bdev.h index 8db683e5b..de3687893 100644 --- a/include/spdk_internal/bdev.h +++ b/include/spdk_internal/bdev.h @@ -124,6 +124,8 @@ struct spdk_bdev_module_if { TAILQ_ENTRY(spdk_bdev_module_if) tailq; }; +typedef void (*spdk_bdev_unregister_cb)(void *cb_arg, int rc); + /** * Function table for a block device backend. * @@ -235,6 +237,12 @@ struct spdk_bdev { */ struct spdk_bdev_module_if *claim_module; + /** Callback function that will be called after bdev destruct is completed. */ + spdk_bdev_unregister_cb unregister_cb; + + /** Unregister call context */ + void *unregister_ctx; + /** List of open descriptors for this block device. */ TAILQ_HEAD(, spdk_bdev_desc) open_descs; @@ -364,11 +372,11 @@ struct spdk_bdev_io { }; void spdk_bdev_register(struct spdk_bdev *bdev); -void spdk_bdev_unregister(struct spdk_bdev *bdev); - +void spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg); +void spdk_bdev_unregister_done(struct spdk_bdev *bdev, int bdeverrno); void spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, int base_bdev_count); -void spdk_vbdev_unregister(struct spdk_bdev *vbdev); +void spdk_vbdev_unregister(struct spdk_bdev *vbdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg); void spdk_bdev_module_examine_done(struct spdk_bdev_module_if *module); void spdk_bdev_module_init_done(struct spdk_bdev_module_if *module); diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 340018579..2a1930f8c 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -1734,7 +1734,15 @@ spdk_vbdev_register(struct spdk_bdev *vbdev, struct spdk_bdev **base_bdevs, int } void -spdk_bdev_unregister(struct spdk_bdev *bdev) +spdk_bdev_unregister_done(struct spdk_bdev *bdev, int bdeverrno) +{ + if (bdev->unregister_cb != NULL) { + bdev->unregister_cb(bdev->unregister_ctx, bdeverrno); + } +} + +void +spdk_bdev_unregister(struct spdk_bdev *bdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg) { struct spdk_bdev_desc *desc, *tmp; int rc; @@ -1745,6 +1753,8 @@ spdk_bdev_unregister(struct spdk_bdev *bdev) pthread_mutex_lock(&bdev->mutex); bdev->status = SPDK_BDEV_STATUS_REMOVING; + bdev->unregister_cb = cb_fn; + bdev->unregister_ctx = cb_arg; TAILQ_FOREACH_SAFE(desc, &bdev->open_descs, link, tmp) { if (desc->remove_cb) { @@ -1771,10 +1781,13 @@ spdk_bdev_unregister(struct spdk_bdev *bdev) if (rc < 0) { SPDK_ERRLOG("destruct failed\n"); } + if (rc <= 0 && cb_fn != NULL) { + cb_fn(cb_arg, rc); + } } void -spdk_vbdev_unregister(struct spdk_bdev *vbdev) +spdk_vbdev_unregister(struct spdk_bdev *vbdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg) { struct spdk_bdev *base_bdev; @@ -1782,7 +1795,7 @@ spdk_vbdev_unregister(struct spdk_bdev *vbdev) TAILQ_FOREACH(base_bdev, &vbdev->base_bdevs, base_bdev_link) { TAILQ_REMOVE(&base_bdev->vbdevs, vbdev, vbdev_link); } - spdk_bdev_unregister(vbdev); + spdk_bdev_unregister(vbdev, cb_fn, cb_arg); } int @@ -1835,7 +1848,7 @@ spdk_bdev_close(struct spdk_bdev_desc *desc) pthread_mutex_unlock(&bdev->mutex); if (do_unregister == true) { - spdk_bdev_unregister(bdev); + spdk_bdev_unregister(bdev, bdev->unregister_cb, bdev->unregister_ctx); } } @@ -1965,7 +1978,7 @@ spdk_bdev_part_base_hotremove(struct spdk_bdev *base_bdev, struct bdev_part_tail TAILQ_FOREACH_SAFE(part, tailq, tailq, tmp) { if (part->base->bdev == base_bdev) { - spdk_vbdev_unregister(&part->bdev); + spdk_vbdev_unregister(&part->bdev, NULL, NULL); } } } diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 149690858..0a78b2992 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -210,7 +210,7 @@ vbdev_lvs_destruct(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, lvs->destruct_req->cb_arg = req; TAILQ_FOREACH_SAFE(lvol, &lvs->lvols, link, tmp) { lvol->close_only = true; - spdk_vbdev_unregister(lvol->bdev); + spdk_vbdev_unregister(lvol->bdev, NULL, NULL); } } @@ -316,16 +316,29 @@ _vbdev_lvol_close_cb(void *cb_arg, int lvserrno) static void _vbdev_lvol_destroy_cb(void *cb_arg, int lvserrno) { + struct spdk_bdev *bdev = cb_arg; + SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol destroyed\n"); + + spdk_bdev_unregister_done(bdev, lvserrno); + free(bdev); } static void _vbdev_lvol_destroy_after_close_cb(void *cb_arg, int lvserrno) { struct spdk_lvol *lvol = cb_arg; + struct spdk_bdev *bdev = lvol->bdev; + + if (lvserrno != 0) { + SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Could not close Lvol %s\n", lvol->old_name); + spdk_bdev_unregister_done(bdev, lvserrno); + free(bdev); + return; + } SPDK_INFOLOG(SPDK_TRACE_VBDEV_LVOL, "Lvol %s closed, begin destroying\n", lvol->old_name); - spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, NULL); + spdk_lvol_destroy(lvol, _vbdev_lvol_destroy_cb, bdev); } static int @@ -334,14 +347,18 @@ vbdev_lvol_destruct(void *ctx) struct spdk_lvol *lvol = ctx; assert(lvol != NULL); - free(lvol->bdev); + if (lvol->close_only) { + free(lvol->bdev); spdk_lvol_close(lvol, _vbdev_lvol_close_cb, NULL); } else { spdk_lvol_close(lvol, _vbdev_lvol_destroy_after_close_cb, lvol); } - return 0; + /* return 1 to indicate we have an operation that must finish asynchronously before the + * lvol is closed + */ + return 1; } static int diff --git a/lib/bdev/nvme/bdev_nvme.c b/lib/bdev/nvme/bdev_nvme.c index ecb1f16cd..e1e8f17a1 100644 --- a/lib/bdev/nvme/bdev_nvme.c +++ b/lib/bdev/nvme/bdev_nvme.c @@ -875,7 +875,7 @@ remove_cb(void *cb_ctx, struct spdk_nvme_ctrlr *ctrlr) TAILQ_FOREACH_SAFE(nvme_bdev, &removed_bdevs, link, btmp) { TAILQ_REMOVE(&removed_bdevs, nvme_bdev, link); - spdk_bdev_unregister(&nvme_bdev->disk); + spdk_bdev_unregister(&nvme_bdev->disk, NULL, NULL); } } diff --git a/lib/bdev/rpc/bdev_rpc.c b/lib/bdev/rpc/bdev_rpc.c index 4232785e5..6d07e389c 100644 --- a/lib/bdev/rpc/bdev_rpc.c +++ b/lib/bdev/rpc/bdev_rpc.c @@ -172,13 +172,27 @@ static const struct spdk_json_object_decoder rpc_delete_bdev_decoders[] = { {"name", offsetof(struct rpc_delete_bdev, name), spdk_json_decode_string}, }; +static void +_spdk_rpc_delete_bdev_cb(void *cb_arg, int bdeverrno) +{ + struct spdk_jsonrpc_request *request = cb_arg; + struct spdk_json_write_ctx *w; + + w = spdk_jsonrpc_begin_result(request); + if (w == NULL) { + return; + } + + spdk_json_write_bool(w, bdeverrno == 0); + spdk_jsonrpc_end_result(request, w); +} + static void spdk_rpc_delete_bdev(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params) { struct rpc_delete_bdev req = {}; struct spdk_bdev *bdev; - struct spdk_json_write_ctx *w; if (spdk_json_decode_object(params, rpc_delete_bdev_decoders, sizeof(rpc_delete_bdev_decoders) / sizeof(*rpc_delete_bdev_decoders), @@ -198,17 +212,10 @@ spdk_rpc_delete_bdev(struct spdk_jsonrpc_request *request, goto invalid; } - spdk_bdev_unregister(bdev); + spdk_bdev_unregister(bdev, _spdk_rpc_delete_bdev_cb, request); free_rpc_delete_bdev(&req); - w = spdk_jsonrpc_begin_result(request); - if (w == NULL) { - return; - } - - spdk_json_write_bool(w, true); - spdk_jsonrpc_end_result(request, w); return; invalid: diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index c40c7e676..31508dea0 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -172,14 +172,14 @@ allocate_vbdev(char *name, struct spdk_bdev *base1, struct spdk_bdev *base2) static void free_bdev(struct spdk_bdev *bdev) { - spdk_bdev_unregister(bdev); + spdk_bdev_unregister(bdev, NULL, NULL); free(bdev); } static void free_vbdev(struct spdk_bdev *bdev) { - spdk_vbdev_unregister(bdev); + spdk_vbdev_unregister(bdev, NULL, NULL); free(bdev); } @@ -400,7 +400,7 @@ part_test(void) CU_ASSERT(TAILQ_EMPTY(&bdev_base.vbdevs)); spdk_bdev_part_base_free(base); - spdk_bdev_unregister(&bdev_base); + spdk_bdev_unregister(&bdev_base, NULL, NULL); } int diff --git a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c index 9e37164bd..80e2dd9e9 100644 --- a/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/mt/bdev.c/bdev_ut.c @@ -183,7 +183,7 @@ unregister_bdev(void) { /* Handle any deferred messages. */ poll_threads(); - spdk_bdev_unregister(&g_bdev.bdev); + spdk_bdev_unregister(&g_bdev.bdev, NULL, NULL); spdk_io_device_unregister(&g_bdev.io_target, NULL); memset(&g_bdev, 0, sizeof(g_bdev)); } 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 ea8ecb90d..5dd8f0518 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 @@ -59,6 +59,11 @@ bool lvol_store_initialize_cb_fail = false; bool lvol_already_opened = false; bool g_examine_done = false; +void +spdk_bdev_unregister_done(struct spdk_bdev *bdev, int bdeverrno) +{ +} + void spdk_lvol_open(struct spdk_lvol *lvol, spdk_lvol_op_with_handle_complete cb_fn, void *cb_arg) { @@ -104,7 +109,7 @@ spdk_bs_bdev_claim(struct spdk_bs_dev *bs_dev, struct spdk_bdev_module_if *modul } void -spdk_vbdev_unregister(struct spdk_bdev *vbdev) +spdk_vbdev_unregister(struct spdk_bdev *vbdev, spdk_bdev_unregister_cb cb_fn, void *cb_arg) { SPDK_CU_ASSERT_FATAL(vbdev != NULL); vbdev->fn_table->destruct(vbdev->ctxt); @@ -255,7 +260,7 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ free(lvol); g_lvol = NULL; - cb_fn(NULL, 0); + cb_fn(cb_arg, 0); } void