From 8f633fa1c331383d74e6e529481c7bb6bae4c8aa Mon Sep 17 00:00:00 2001 From: Kai Li Date: Tue, 9 Nov 2021 21:05:00 -0500 Subject: [PATCH] bdev/nvme: display all ctrlrs for this bdev when dump bdev nvme controller After multipath feature is supported, one bdev will have more than one nvme ctrlr. Fore ease of view, display each ctrlr's trid info. Moreover, rename nvme_bdev_ctrlr_get as nvme_bdev_ctrlr_get_by_name here to keep consistent with nvme_ctrlr_get_by_name. Signed-off-by: Kai Li Change-Id: I417506699bbea6ed13dac0fee942749757d2ae47 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10129 Reviewed-by: Konrad Sztyber Reviewed-by: Aleksey Marchuk Reviewed-by: Shuhei Matsumoto Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins --- module/bdev/nvme/bdev_nvme.c | 19 +++--- module/bdev/nvme/bdev_nvme.h | 6 +- module/bdev/nvme/bdev_nvme_rpc.c | 60 ++++++++++--------- .../lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c | 54 ++++++++--------- 4 files changed, 71 insertions(+), 68 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index f4f319de1..4f9eac413 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -243,8 +243,8 @@ struct nvme_bdev_ctrlrs g_nvme_bdev_ctrlrs = TAILQ_HEAD_INITIALIZER(g_nvme_bdev_ pthread_mutex_t g_bdev_nvme_mutex = PTHREAD_MUTEX_INITIALIZER; bool g_bdev_nvme_module_finish; -static struct nvme_bdev_ctrlr * -nvme_bdev_ctrlr_get(const char *name) +struct nvme_bdev_ctrlr * +nvme_bdev_ctrlr_get_by_name(const char *name) { struct nvme_bdev_ctrlr *nbdev_ctrlr; @@ -344,7 +344,7 @@ nvme_ctrlr_get_by_name(const char *name) } pthread_mutex_lock(&g_bdev_nvme_mutex); - nbdev_ctrlr = nvme_bdev_ctrlr_get(name); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name(name); if (nbdev_ctrlr != NULL) { nvme_ctrlr = TAILQ_FIRST(&nbdev_ctrlr->ctrlrs); } @@ -354,16 +354,13 @@ nvme_ctrlr_get_by_name(const char *name) } void -nvme_ctrlr_for_each(nvme_ctrlr_for_each_fn fn, void *ctx) +nvme_bdev_ctrlr_for_each(nvme_bdev_ctrlr_for_each_fn fn, void *ctx) { struct nvme_bdev_ctrlr *nbdev_ctrlr; - struct nvme_ctrlr *nvme_ctrlr; pthread_mutex_lock(&g_bdev_nvme_mutex); TAILQ_FOREACH(nbdev_ctrlr, &g_nvme_bdev_ctrlrs, tailq) { - TAILQ_FOREACH(nvme_ctrlr, &nbdev_ctrlr->ctrlrs, tailq) { - fn(nvme_ctrlr, ctx); - } + fn(nbdev_ctrlr, ctx); } pthread_mutex_unlock(&g_bdev_nvme_mutex); } @@ -2977,7 +2974,7 @@ nvme_bdev_ctrlr_create(const char *name, struct nvme_ctrlr *nvme_ctrlr) pthread_mutex_lock(&g_bdev_nvme_mutex); - nbdev_ctrlr = nvme_bdev_ctrlr_get(name); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name(name); if (nbdev_ctrlr != NULL) { if (!bdev_nvme_check_multipath(nbdev_ctrlr, ctrlr)) { rc = -EINVAL; @@ -3588,7 +3585,7 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, ctx->opts.keep_alive_timeout_ms = g_opts.keep_alive_timeout_ms; ctx->opts.disable_read_ana_log_page = true; - if (nvme_bdev_ctrlr_get(base_name) == NULL || multipath) { + if (nvme_bdev_ctrlr_get_by_name(base_name) == NULL || multipath) { attach_cb = connect_attach_cb; } else { attach_cb = connect_set_failover_cb; @@ -3617,7 +3614,7 @@ bdev_nvme_delete(const char *name, const struct nvme_path_id *path_id) return -EINVAL; } - nbdev_ctrlr = nvme_bdev_ctrlr_get(name); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name(name); if (nbdev_ctrlr == NULL) { SPDK_ERRLOG("Failed to find NVMe bdev controller\n"); return -ENODEV; diff --git a/module/bdev/nvme/bdev_nvme.h b/module/bdev/nvme/bdev_nvme.h index ba06cd77a..f1c1a73cf 100644 --- a/module/bdev/nvme/bdev_nvme.h +++ b/module/bdev/nvme/bdev_nvme.h @@ -194,9 +194,11 @@ struct nvme_poll_group { struct nvme_ctrlr *nvme_ctrlr_get_by_name(const char *name); -typedef void (*nvme_ctrlr_for_each_fn)(struct nvme_ctrlr *nvme_ctrlr, void *ctx); +struct nvme_bdev_ctrlr *nvme_bdev_ctrlr_get_by_name(const char *name); -void nvme_ctrlr_for_each(nvme_ctrlr_for_each_fn fn, void *ctx); +typedef void (*nvme_bdev_ctrlr_for_each_fn)(struct nvme_bdev_ctrlr *nbdev_ctrlr, void *ctx); + +void nvme_bdev_ctrlr_for_each(nvme_bdev_ctrlr_for_each_fn fn, void *ctx); void nvme_bdev_dump_trid_json(const struct spdk_nvme_transport_id *trid, struct spdk_json_write_ctx *w); diff --git a/module/bdev/nvme/bdev_nvme_rpc.c b/module/bdev/nvme/bdev_nvme_rpc.c index b47a6e69f..4eff5dfba 100644 --- a/module/bdev/nvme/bdev_nvme_rpc.c +++ b/module/bdev/nvme/bdev_nvme_rpc.c @@ -500,38 +500,42 @@ SPDK_RPC_REGISTER("bdev_nvme_attach_controller", rpc_bdev_nvme_attach_controller SPDK_RPC_REGISTER_ALIAS_DEPRECATED(bdev_nvme_attach_controller, construct_nvme_bdev) static void -rpc_dump_nvme_controller_info(struct nvme_ctrlr *nvme_ctrlr, void *ctx) +rpc_dump_nvme_bdev_controller_info(struct nvme_bdev_ctrlr *nbdev_ctrlr, void *ctx) { struct spdk_json_write_ctx *w = ctx; - struct spdk_nvme_transport_id *trid; + struct spdk_nvme_transport_id *trid; + struct nvme_ctrlr *nvme_ctrlr; const struct spdk_nvme_ctrlr_opts *opts; - trid = &nvme_ctrlr->active_path_id->trid; - spdk_json_write_object_begin(w); - spdk_json_write_named_string(w, "name", nvme_ctrlr->nbdev_ctrlr->name); + spdk_json_write_named_string(w, "name", nbdev_ctrlr->name); + spdk_json_write_named_array_begin(w, "ctrlrs"); + TAILQ_FOREACH(nvme_ctrlr, &nbdev_ctrlr->ctrlrs, tailq) { + spdk_json_write_object_begin(w); #ifdef SPDK_CONFIG_NVME_CUSE - size_t cuse_name_size = 128; - char cuse_name[cuse_name_size]; + size_t cuse_name_size = 128; + char cuse_name[cuse_name_size]; - int rc = spdk_nvme_cuse_get_ctrlr_name(nvme_ctrlr->ctrlr, cuse_name, &cuse_name_size); - if (rc == 0) { - spdk_json_write_named_string(w, "cuse_device", cuse_name); - } + int rc = spdk_nvme_cuse_get_ctrlr_name(nvme_ctrlr->ctrlr, cuse_name, &cuse_name_size); + if (rc == 0) { + spdk_json_write_named_string(w, "cuse_device", cuse_name); + } #endif + trid = &nvme_ctrlr->active_path_id->trid; + spdk_json_write_named_object_begin(w, "trid"); + nvme_bdev_dump_trid_json(trid, w); + spdk_json_write_object_end(w); - spdk_json_write_named_object_begin(w, "trid"); - nvme_bdev_dump_trid_json(trid, w); - spdk_json_write_object_end(w); - - opts = spdk_nvme_ctrlr_get_opts(nvme_ctrlr->ctrlr); - - spdk_json_write_named_object_begin(w, "host"); - spdk_json_write_named_string(w, "nqn", opts->hostnqn); - spdk_json_write_named_string(w, "addr", opts->src_addr); - spdk_json_write_named_string(w, "svcid", opts->src_svcid); - spdk_json_write_object_end(w); + opts = spdk_nvme_ctrlr_get_opts(nvme_ctrlr->ctrlr); + spdk_json_write_named_object_begin(w, "host"); + spdk_json_write_named_string(w, "nqn", opts->hostnqn); + spdk_json_write_named_string(w, "addr", opts->src_addr); + spdk_json_write_named_string(w, "svcid", opts->src_svcid); + spdk_json_write_object_end(w); + spdk_json_write_object_end(w); + } + spdk_json_write_array_end(w); spdk_json_write_object_end(w); } @@ -555,7 +559,7 @@ rpc_bdev_nvme_get_controllers(struct spdk_jsonrpc_request *request, { struct rpc_bdev_nvme_get_controllers req = {}; struct spdk_json_write_ctx *w; - struct nvme_ctrlr *ctrlr = NULL; + struct nvme_bdev_ctrlr *nbdev_ctrlr = NULL; if (params && spdk_json_decode_object(params, rpc_bdev_nvme_get_controllers_decoders, SPDK_COUNTOF(rpc_bdev_nvme_get_controllers_decoders), @@ -567,8 +571,8 @@ rpc_bdev_nvme_get_controllers(struct spdk_jsonrpc_request *request, } if (req.name) { - ctrlr = nvme_ctrlr_get_by_name(req.name); - if (ctrlr == NULL) { + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name(req.name); + if (nbdev_ctrlr == NULL) { SPDK_ERRLOG("ctrlr '%s' does not exist\n", req.name); spdk_jsonrpc_send_error_response_fmt(request, EINVAL, "Controller %s does not exist", req.name); goto cleanup; @@ -578,10 +582,10 @@ rpc_bdev_nvme_get_controllers(struct spdk_jsonrpc_request *request, w = spdk_jsonrpc_begin_result(request); spdk_json_write_array_begin(w); - if (ctrlr != NULL) { - rpc_dump_nvme_controller_info(ctrlr, w); + if (nbdev_ctrlr != NULL) { + rpc_dump_nvme_bdev_controller_info(nbdev_ctrlr, w); } else { - nvme_ctrlr_for_each(rpc_dump_nvme_controller_info, w); + nvme_bdev_ctrlr_for_each(rpc_dump_nvme_bdev_controller_info, w); } spdk_json_write_array_end(w); diff --git a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c index b3c6d1c10..2531e6baf 100644 --- a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c +++ b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c @@ -2897,7 +2897,7 @@ test_create_bdev_ctrlr(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid) != NULL); @@ -2943,7 +2943,7 @@ test_create_bdev_ctrlr(void) rc = bdev_nvme_delete("nvme0", &g_any_path); CU_ASSERT(rc == 0); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == nbdev_ctrlr); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == nbdev_ctrlr); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid) != NULL); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path2.trid) != NULL); @@ -2951,7 +2951,7 @@ test_create_bdev_ctrlr(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); /* Add two ctrlrs and delete one by one. */ ctrlr1 = ut_attach_ctrlr(&path1.trid, 0, true, true); @@ -2980,13 +2980,13 @@ test_create_bdev_ctrlr(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); rc = bdev_nvme_delete("nvme0", &path1); CU_ASSERT(rc == 0); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == nbdev_ctrlr); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == nbdev_ctrlr); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid) != NULL); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path2.trid) != NULL); @@ -2994,14 +2994,14 @@ test_create_bdev_ctrlr(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == nbdev_ctrlr); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == nbdev_ctrlr); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid) == NULL); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path2.trid) != NULL); rc = bdev_nvme_delete("nvme0", &path2); CU_ASSERT(rc == 0); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == nbdev_ctrlr); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == nbdev_ctrlr); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid) == NULL); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path2.trid) != NULL); @@ -3009,7 +3009,7 @@ test_create_bdev_ctrlr(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); } static struct nvme_ns * @@ -3096,7 +3096,7 @@ test_add_multi_ns_to_bdev(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); nvme_ctrlr1 = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid); @@ -3140,7 +3140,7 @@ test_add_multi_ns_to_bdev(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == nbdev_ctrlr); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == nbdev_ctrlr); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid) == NULL); CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path2.trid) == nvme_ctrlr2); @@ -3151,7 +3151,7 @@ test_add_multi_ns_to_bdev(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); /* Test if a nvme_bdev which has a shared namespace between two ctrlrs * can be deleted when the bdev subsystem shutdown. @@ -3190,7 +3190,7 @@ test_add_multi_ns_to_bdev(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); bdev1 = nvme_bdev_ctrlr_get_bdev(nbdev_ctrlr, 1); @@ -3233,7 +3233,7 @@ test_add_multi_ns_to_bdev(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); } static void @@ -3291,7 +3291,7 @@ test_add_multi_io_paths_to_nbdev_ch(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); nvme_ctrlr1 = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid); @@ -3379,7 +3379,7 @@ test_add_multi_io_paths_to_nbdev_ch(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); } static void @@ -3433,7 +3433,7 @@ test_admin_path(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); bdev = nvme_bdev_ctrlr_get_bdev(nbdev_ctrlr, 1); @@ -3488,7 +3488,7 @@ test_admin_path(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); } static struct nvme_io_path * @@ -3560,7 +3560,7 @@ test_reset_bdev_ctrlr(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); nvme_ctrlr1 = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid); @@ -3744,7 +3744,7 @@ test_reset_bdev_ctrlr(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); free(first_bdev_io); free(second_bdev_io); @@ -3838,7 +3838,7 @@ test_retry_io_if_ctrlr_is_resetting(void) spdk_delay_us(1000); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); nvme_ctrlr = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path.trid); @@ -3987,7 +3987,7 @@ test_retry_io_if_ctrlr_is_resetting(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); } static void @@ -4036,7 +4036,7 @@ test_retry_io_for_io_path_error(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); nvme_ctrlr1 = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path1.trid); @@ -4193,7 +4193,7 @@ test_retry_io_for_io_path_error(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); g_opts.bdev_retry_count = 0; } @@ -4236,7 +4236,7 @@ test_retry_io_count(void) spdk_delay_us(1000); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); nvme_ctrlr = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path.trid); @@ -4387,7 +4387,7 @@ test_retry_io_count(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); g_opts.bdev_retry_count = 0; } @@ -4520,7 +4520,7 @@ test_retry_io_for_ana_error(void) spdk_delay_us(g_opts.nvme_adminq_poll_period_us); poll_threads(); - nbdev_ctrlr = nvme_bdev_ctrlr_get("nvme0"); + nbdev_ctrlr = nvme_bdev_ctrlr_get_by_name("nvme0"); SPDK_CU_ASSERT_FATAL(nbdev_ctrlr != NULL); nvme_ctrlr = nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &path.trid); @@ -4620,7 +4620,7 @@ test_retry_io_for_ana_error(void) spdk_delay_us(1000); poll_threads(); - CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == NULL); + CU_ASSERT(nvme_bdev_ctrlr_get_by_name("nvme0") == NULL); g_opts.bdev_retry_count = 0; }