From 14739d6e136ffe99c445b3ad779a08a033e3bb72 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Wed, 8 Sep 2021 11:39:23 -0700 Subject: [PATCH] bdev/nvme: bdev_nvme_detach_controller is now much more flexible It can match by any provided parameter to remove paths. Change-Id: I5e7a87342bbb90943dc97fb52f142814fcf0acfa Signed-off-by: Ben Walker Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9453 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Aleksey Marchuk --- module/bdev/nvme/bdev_nvme.c | 12 +----- module/bdev/nvme/bdev_nvme_rpc.c | 38 ++++++++++--------- test/nvmf/host/multipath.sh | 3 ++ .../lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c | 33 ++++++++-------- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 651aa14216..d5818fc770 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -3101,7 +3101,7 @@ bdev_nvme_delete(const char *name, const struct spdk_nvme_transport_id *trid) struct nvme_ctrlr_trid *ctrlr_trid, *tmp_trid; int rc = -ENXIO; - if (name == NULL) { + if (name == NULL || trid == NULL) { return -EINVAL; } @@ -3116,16 +3116,6 @@ bdev_nvme_delete(const char *name, const struct spdk_nvme_transport_id *trid) */ TAILQ_FOREACH_SAFE(nvme_ctrlr, &nbdev_ctrlr->ctrlrs, tailq, tmp_nvme_ctrlr) { - if (trid == NULL) { - /* Remove all nvme_ctrlrs of the nvme_bdev_ctrlr. */ - rc = _bdev_nvme_delete(nvme_ctrlr, false); - if (rc != 0) { - return rc; - } - - continue; - } - TAILQ_FOREACH_REVERSE_SAFE(ctrlr_trid, &nvme_ctrlr->trids, nvme_paths, link, tmp_trid) { if (trid->trtype != 0) { if (trid->trtype == SPDK_NVME_TRANSPORT_CUSTOM) { diff --git a/module/bdev/nvme/bdev_nvme_rpc.c b/module/bdev/nvme/bdev_nvme_rpc.c index 4709187b30..ae964fa976 100644 --- a/module/bdev/nvme/bdev_nvme_rpc.c +++ b/module/bdev/nvme/bdev_nvme_rpc.c @@ -553,7 +553,6 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, struct spdk_nvme_transport_id trid = {}; size_t len, maxlen; int rc = 0; - bool all_trid_entries, one_trid_entry; if (spdk_json_decode_object(params, rpc_bdev_nvme_detach_controller_decoders, SPDK_COUNTOF(rpc_bdev_nvme_detach_controller_decoders), @@ -563,18 +562,8 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, goto cleanup; } - all_trid_entries = req.trtype && req.traddr && req.adrfam && req.trsvcid && req.subnqn; - one_trid_entry = req.trtype || req.traddr || req.adrfam || req.trsvcid || req.subnqn; - - if (all_trid_entries ^ one_trid_entry) { - spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, - "trtype, traddr, adrfam, trsvcid, subnqn must all be provided together or not at all."); - goto cleanup; - } - - if (all_trid_entries) { - /* Parse trtype */ - rc = spdk_nvme_transport_id_parse_trtype(&trid.trtype, req.trtype); + if (req.trtype != NULL) { + rc = spdk_nvme_transport_id_populate_trstring(&trid, req.trtype); if (rc < 0) { SPDK_ERRLOG("Failed to parse trtype: %s\n", req.trtype); spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, "Failed to parse trtype: %s", @@ -582,7 +571,16 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, goto cleanup; } - /* Parse traddr */ + rc = spdk_nvme_transport_id_parse_trtype(&trid.trtype, req.trtype); + if (rc < 0) { + SPDK_ERRLOG("Failed to parse trtype: %s\n", req.trtype); + spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, "Failed to parse trtype: %s", + req.trtype); + goto cleanup; + } + } + + if (req.traddr != NULL) { maxlen = sizeof(trid.traddr); len = strnlen(req.traddr, maxlen); if (len == maxlen) { @@ -591,7 +589,9 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, goto cleanup; } memcpy(trid.traddr, req.traddr, len + 1); + } + if (req.adrfam != NULL) { rc = spdk_nvme_transport_id_parse_adrfam(&trid.adrfam, req.adrfam); if (rc < 0) { SPDK_ERRLOG("Failed to parse adrfam: %s\n", req.adrfam); @@ -599,7 +599,9 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, req.adrfam); goto cleanup; } + } + if (req.trsvcid != NULL) { maxlen = sizeof(trid.trsvcid); len = strnlen(req.trsvcid, maxlen); if (len == maxlen) { @@ -608,7 +610,10 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, goto cleanup; } memcpy(trid.trsvcid, req.trsvcid, len + 1); + } + /* Parse subnqn */ + if (req.subnqn != NULL) { maxlen = sizeof(trid.subnqn); len = strnlen(req.subnqn, maxlen); if (len == maxlen) { @@ -617,11 +622,10 @@ rpc_bdev_nvme_detach_controller(struct spdk_jsonrpc_request *request, goto cleanup; } memcpy(trid.subnqn, req.subnqn, len + 1); - rc = bdev_nvme_delete(req.name, &trid); - } else { - rc = bdev_nvme_delete(req.name, NULL); } + rc = bdev_nvme_delete(req.name, &trid); + if (rc != 0) { spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc)); goto cleanup; diff --git a/test/nvmf/host/multipath.sh b/test/nvmf/host/multipath.sh index 4acd9e599c..df7b661e34 100755 --- a/test/nvmf/host/multipath.sh +++ b/test/nvmf/host/multipath.sh @@ -94,6 +94,9 @@ rpc_pid=$! wait $rpc_pid +cat $testdir/try.txt +$rpc_py -s $bdevperf_rpc_sock bdev_nvme_get_controllers | grep -q NVMe0 + # No need to wait here since we are deleting a TRID we aren't connected to. $rpc_py -s $bdevperf_rpc_sock bdev_nvme_detach_controller NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_THIRD_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 $rpc_py -s $bdevperf_rpc_sock bdev_nvme_get_controllers | grep -q NVMe0 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 b11cfeeb16..04daa4be16 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 @@ -301,6 +301,7 @@ static int g_ut_attach_ctrlr_status; static size_t g_ut_attach_bdev_count; static int g_ut_register_bdev_status; static uint16_t g_ut_cntlid; +static struct spdk_nvme_transport_id g_any_trid = {}; static void ut_init_trid(struct spdk_nvme_transport_id *trid) @@ -1180,7 +1181,7 @@ test_create_ctrlr(void) CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") != NULL); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") != NULL); @@ -1290,7 +1291,7 @@ test_reset_ctrlr(void) poll_threads(); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -1336,7 +1337,7 @@ test_race_between_reset_and_destruct_ctrlr(void) /* Try destructing ctrlr while ctrlr is being reset, but it will be deferred. */ set_thread(0); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == nvme_ctrlr); CU_ASSERT(nvme_ctrlr->destruct == true); @@ -1504,7 +1505,7 @@ test_failover_ctrlr(void) poll_threads(); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -1633,7 +1634,7 @@ test_pending_reset(void) set_thread(0); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -1697,7 +1698,7 @@ test_attach_ctrlr(void) CU_ASSERT(nvme_ctrlr->ctrlr == ctrlr); CU_ASSERT(nvme_ctrlr->num_ns == 0); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -1731,7 +1732,7 @@ test_attach_ctrlr(void) SPDK_CU_ASSERT_FATAL(nbdev != NULL); CU_ASSERT(bdev_nvme_get_ctrlr(&nbdev->disk) == ctrlr); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -1761,7 +1762,7 @@ test_attach_ctrlr(void) CU_ASSERT(attached_names[0] == NULL); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -1860,7 +1861,7 @@ test_aer_cb(void) CU_ASSERT(nvme_ctrlr_get_ns(nvme_ctrlr, 2)->ana_state == SPDK_NVME_ANA_INACCESSIBLE_STATE); CU_ASSERT(nvme_ctrlr_get_ns(nvme_ctrlr, 4)->ana_state == SPDK_NVME_ANA_CHANGE_STATE); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -2054,7 +2055,7 @@ test_submit_nvme_cmd(void) set_thread(1); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -2207,7 +2208,7 @@ test_add_remove_trid(void) CU_ASSERT(ctrid != NULL); /* If trid is not specified, nvme_ctrlr itself is removed. */ - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == nvme_ctrlr); @@ -2384,7 +2385,7 @@ test_abort(void) set_thread(1); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -2424,7 +2425,7 @@ test_get_io_qpair(void) spdk_put_io_channel(ch); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -2602,7 +2603,7 @@ test_init_ana_log_page(void) CU_ASSERT(nvme_ctrlr_get_ns(nvme_ctrlr, 4)->bdev != NULL); CU_ASSERT(nvme_ctrlr_get_ns(nvme_ctrlr, 5)->bdev != NULL); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -2763,7 +2764,7 @@ test_reconnect_qpair(void) poll_threads(); - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); poll_threads(); @@ -2843,7 +2844,7 @@ test_create_bdev_ctrlr(void) CU_ASSERT(nvme_bdev_ctrlr_get_ctrlr(nbdev_ctrlr, &trid2) != NULL); /* Delete two ctrlrs at once. */ - rc = bdev_nvme_delete("nvme0", NULL); + rc = bdev_nvme_delete("nvme0", &g_any_trid); CU_ASSERT(rc == 0); CU_ASSERT(nvme_bdev_ctrlr_get("nvme0") == nbdev_ctrlr);