From 5375dfbb381910b5e13a1539eaa2b02d103b6b25 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Mon, 30 Aug 2021 13:19:31 -0700 Subject: [PATCH] bdev/nvme: Improve error reporting when adding additional paths to a controller Change-Id: I53ac0c6f8879bf80bc1345ef620a215d434f536f Signed-off-by: Ben Walker Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9340 Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot Community-CI: Broadcom CI Reviewed-by: Aleksey Marchuk Reviewed-by: Shuhei Matsumoto --- module/bdev/nvme/bdev_nvme_rpc.c | 25 +++++++++++++++++++------ test/nvmf/host/multicontroller.sh | 9 +++++---- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme_rpc.c b/module/bdev/nvme/bdev_nvme_rpc.c index ae964fa97..3541b2e96 100644 --- a/module/bdev/nvme/bdev_nvme_rpc.c +++ b/module/bdev/nvme/bdev_nvme_rpc.c @@ -271,6 +271,7 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, { struct rpc_bdev_nvme_attach_controller_ctx *ctx; struct spdk_nvme_transport_id trid = {}; + const struct spdk_nvme_ctrlr_opts *opts; uint32_t prchk_flags = 0; struct nvme_ctrlr *ctrlr = NULL; size_t len, maxlen; @@ -385,9 +386,24 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, ctrlr = nvme_ctrlr_get_by_name(ctx->req.name); - if (ctrlr && (ctx->req.hostaddr || ctx->req.hostnqn || ctx->req.hostsvcid || ctx->req.prchk_guard || - ctx->req.prchk_reftag)) { - goto conflicting_arguments; + if (ctrlr) { + /* This controller already exists. Verify the parameters match sufficiently. */ + opts = spdk_nvme_ctrlr_get_opts(ctrlr->ctrlr); + + if (strncmp(ctx->req.opts.hostnqn, opts->hostnqn, SPDK_NVMF_NQN_MAX_LEN) != 0) { + /* Different HOSTNQN is not allowed when specifying the same controller name. */ + spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, + "A controller named %s already exists, but uses a different hostnqn (%s)\n", + ctx->req.name, opts->hostnqn); + goto cleanup; + } + + if (ctx->req.prchk_guard || ctx->req.prchk_reftag) { + spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, + "A controller named %s already exists. To add a path, do not specify PI options.\n", + ctx->req.name); + goto cleanup; + } } if (ctx->req.prchk_reftag) { @@ -410,9 +426,6 @@ 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); diff --git a/test/nvmf/host/multicontroller.sh b/test/nvmf/host/multicontroller.sh index a2e3c6c06..86c077341 100755 --- a/test/nvmf/host/multicontroller.sh +++ b/test/nvmf/host/multicontroller.sh @@ -46,9 +46,10 @@ while ! $rpc_py -s $bdevperf_rpc_sock bdev_nvme_get_controllers | grep -c NVMe; sleep 1s done -# try to attach to the second port with a different hostsvcid (this should fail). +# try to attach with same controller name but different hostnqn. Should fail. NOT $rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP \ - -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -i $NVMF_FIRST_TARGET_IP -c $NVMF_HOST_SECOND_PORT + -s $NVMF_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -i $NVMF_FIRST_TARGET_IP -c $NVMF_HOST_FIRST_PORT \ + -q nqn.2021-09-7.io.spdk:00001 # Add a second path without specifying the host information. Should pass. $rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP \ @@ -58,9 +59,9 @@ $rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe0 -t $TEST_TRAN $rpc_py -s $bdevperf_rpc_sock bdev_nvme_detach_controller NVMe0 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP \ -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -# Add a second controller by attaching to the same subsystem from a different hostid. +# Add a second controller by attaching to the same subsystem with a different controller name $rpc_py -s $bdevperf_rpc_sock bdev_nvme_attach_controller -b NVMe1 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP \ - -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -i $NVMF_FIRST_TARGET_IP -c $NVMF_HOST_SECOND_PORT + -s $NVMF_SECOND_PORT -f ipv4 -n nqn.2016-06.io.spdk:cnode1 -i $NVMF_FIRST_TARGET_IP -c $NVMF_HOST_FIRST_PORT if [ "$($rpc_py -s $bdevperf_rpc_sock bdev_nvme_get_controllers | grep -c NVMe)" != "2" ]; then echo "actual number of controllers is not equal to expected count."