From 1973d10b59f6ff160b8ec1b5e3a8cffaf2688cc9 Mon Sep 17 00:00:00 2001 From: Seth Howell Date: Fri, 12 Jun 2020 18:06:58 -0700 Subject: [PATCH] bdev/nvme: modify attach_controller rpc to also add multipath trids. This allows us to avoid creating a separate rpc just for multipath TRIDs. Signed-off-by: Seth Howell Change-Id: I4e83167eaf16e50a72efbd513333a4d09c52be61 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2884 Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- doc/jsonrpc.md | 5 ++++- module/bdev/nvme/bdev_nvme.c | 28 +++++++++++++++++++++------- module/bdev/nvme/bdev_nvme_rpc.c | 11 +++++++++++ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/doc/jsonrpc.md b/doc/jsonrpc.md index aae5d8fc7..1f4335e3b 100644 --- a/doc/jsonrpc.md +++ b/doc/jsonrpc.md @@ -1771,7 +1771,10 @@ Example response: ## bdev_nvme_attach_controller {#rpc_bdev_nvme_attach_controller} -Construct @ref bdev_config_nvme +Construct @ref bdev_config_nvme. This RPC can also be used to add additional paths to an existing controller to enable +multipathing. This is done by specifying the `name` parameter as an existing controller. When adding an additional +path, the hostnqn, hostsvcid, hostaddr, prchk_reftag, and prchk_guard_arguments must not be specified and are assumed +to have the same value as the existing path. ### Result diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index e8acdfb34..cc665b71c 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -1686,7 +1686,7 @@ nvme_ctrlr_populate_namespaces_done(struct nvme_async_probe_ctx *ctx) uint32_t i, nsid; size_t j; - nvme_bdev_ctrlr = nvme_bdev_ctrlr_get(&ctx->trid); + nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name(ctx->base_name); assert(nvme_bdev_ctrlr != NULL); /* @@ -1849,17 +1849,26 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, { struct nvme_probe_skip_entry *entry, *tmp; struct nvme_async_probe_ctx *ctx; + struct nvme_bdev_ctrlr *existing_ctrlr; + int rc; - if (nvme_bdev_ctrlr_get(trid) != NULL) { + existing_ctrlr = nvme_bdev_ctrlr_get_by_name(base_name); + if (existing_ctrlr) { + if (trid->trtype == SPDK_NVME_TRANSPORT_PCIE) { + SPDK_ERRLOG("A controller with the provided name (name: %s) already exists with transport type PCIe. PCIe multipath is not supported.\n", + base_name); + return -EEXIST; + } + rc = bdev_nvme_add_trid(existing_ctrlr->name, trid); + if (rc) { + return rc; + } + /* TODO expand this check to include both the host and target TRIDs. Only if both are the same should we fail. */ + } else if (nvme_bdev_ctrlr_get(trid) != NULL) { SPDK_ERRLOG("A controller with the provided trid (traddr: %s) already exists.\n", trid->traddr); return -EEXIST; } - if (nvme_bdev_ctrlr_get_by_name(base_name)) { - SPDK_ERRLOG("A controller with the provided name (%s) already exists.\n", base_name); - return -EEXIST; - } - if (trid->trtype == SPDK_NVME_TRANSPORT_PCIE) { TAILQ_FOREACH_SAFE(entry, &g_skipped_nvme_ctrlrs, tailq, tmp) { if (spdk_nvme_transport_id_compare(trid, &entry->trid) == 0) { @@ -1882,6 +1891,11 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, ctx->prchk_flags = prchk_flags; ctx->trid = *trid; + if (existing_ctrlr) { + nvme_ctrlr_populate_namespaces_done(ctx); + return 0; + } + spdk_nvme_ctrlr_get_default_ctrlr_opts(&ctx->opts, sizeof(ctx->opts)); ctx->opts.transport_retry_count = g_opts.retry_count; diff --git a/module/bdev/nvme/bdev_nvme_rpc.c b/module/bdev/nvme/bdev_nvme_rpc.c index 2d797c95d..c768edafa 100644 --- a/module/bdev/nvme/bdev_nvme_rpc.c +++ b/module/bdev/nvme/bdev_nvme_rpc.c @@ -257,6 +257,7 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, struct spdk_nvme_transport_id trid = {}; struct spdk_nvme_host_id hostid = {}; uint32_t prchk_flags = 0; + struct nvme_bdev_ctrlr *ctrlr = NULL; int rc; ctx = calloc(1, sizeof(*ctx)); @@ -287,6 +288,8 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, rc = spdk_nvme_transport_id_parse_trtype(&trid.trtype, ctx->req.trtype); assert(rc == 0); + ctrlr = nvme_bdev_ctrlr_get_by_name(ctx->req.name); + /* Parse traddr */ snprintf(trid.traddr, sizeof(trid.traddr), "%s", ctx->req.traddr); @@ -316,6 +319,11 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, snprintf(trid.subnqn, sizeof(trid.subnqn), "%s", ctx->req.subnqn); } + if (ctrlr && (ctx->req.hostaddr || ctx->req.hostnqn || ctx->req.hostsvcid || ctx->req.prchk_guard || + ctx->req.prchk_reftag)) { + goto conflicting_arguments; + } + if (ctx->req.hostaddr) { snprintf(hostid.hostaddr, sizeof(hostid.hostaddr), "%s", ctx->req.hostaddr); } @@ -343,6 +351,9 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, return; +conflicting_arguments: + spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, + "Invalid agrgument list. Existing controller name cannot be combined with host information or PI options.\n"); cleanup: free_rpc_bdev_nvme_attach_controller(&ctx->req); free(ctx);