From eb387189c2eb411a82bc4275fc286a7c0c40e6c3 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Tue, 12 Jun 2018 15:19:09 -0700 Subject: [PATCH] nvmf: don't change NN while ctrlrs exist This was partially fixed in commit 1e481d0438e6 ("nvmf: Do not allow NN to change while connections present"), but we did not handle the case where the user asked to add a NS with a NSID outside the current NN. This patch reworks the logic (again) to be more straightforward and hopefully more obviously correct. Some confusion between max_allowed_nsid and max_nsid is also clarified; if max_allowed_nsid is set, then max_nsid == max_allowed_nsid at all times, so we don't need the extra logic when calculating NN in spdk_nvmf_ctrlr_identify_ctrlr(). Change-Id: If531baf1114e03441ff3e1e1be098071702d9056 Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/414894 Tested-by: SPDK Automated Test System Reviewed-by: Changpeng Liu Reviewed-by: Ben Walker --- lib/nvmf/ctrlr.c | 6 +---- lib/nvmf/subsystem.c | 55 ++++++++++++++++++++----------------------- test/nvmf/host/aer.sh | 4 +++- 3 files changed, 29 insertions(+), 36 deletions(-) diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index d8f596f93b..cd55cca9cb 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -1296,11 +1296,7 @@ spdk_nvmf_ctrlr_identify_ctrlr(struct spdk_nvmf_ctrlr *ctrlr, struct spdk_nvme_c cdata->sqes.max = 6; cdata->cqes.min = 4; cdata->cqes.max = 4; - if (subsystem->max_allowed_nsid != 0) { - cdata->nn = subsystem->max_allowed_nsid; - } else { - cdata->nn = subsystem->max_nsid; - } + cdata->nn = subsystem->max_nsid; cdata->vwc.present = 1; cdata->nvmf_specific.ioccsz = sizeof(struct spdk_nvme_cmd) / 16; diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 2751d268fc..408bad96be 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -973,7 +973,6 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd { struct spdk_nvmf_ns_opts opts; struct spdk_nvmf_ns *ns; - uint32_t i; int rc; if (!(subsystem->state == SPDK_NVMF_SUBSYSTEM_INACTIVE || @@ -995,44 +994,40 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd return 0; } - if ((subsystem->max_allowed_nsid > 0) && (opts.nsid > subsystem->max_allowed_nsid)) { - SPDK_ERRLOG("Can't extend NSID range above MaxNamespaces\n"); - return 0; - } - if (opts.nsid == 0) { - /* NSID not specified - find a free index */ - for (i = 0; i < subsystem->max_nsid; i++) { - if (_spdk_nvmf_subsystem_get_ns(subsystem, i + 1) == NULL) { - opts.nsid = i + 1; + /* + * NSID not specified - find a free index. + * + * If no free slots are found, opts.nsid will be subsystem->max_nsid + 1, which will + * expand max_nsid if possible. + */ + for (opts.nsid = 1; opts.nsid <= subsystem->max_nsid; opts.nsid++) { + if (_spdk_nvmf_subsystem_get_ns(subsystem, opts.nsid) == NULL) { break; } } - if (opts.nsid == 0) { - /* All of the current slots are full */ - if (TAILQ_EMPTY(&subsystem->ctrlrs) && - (subsystem->max_allowed_nsid == 0 || - subsystem->max_nsid < subsystem->max_allowed_nsid)) { - /* No controllers are connected and the subsystem is - * not at its maximum NSID limit, so the array - * can be expanded. */ - opts.nsid = subsystem->max_nsid + 1; - } else { - SPDK_ERRLOG("Can't extend NSID range above MaxNamespaces\n"); - return 0; - } - } - } else { - /* Specific NSID requested */ - if (_spdk_nvmf_subsystem_get_ns(subsystem, opts.nsid)) { - SPDK_ERRLOG("Requested NSID %" PRIu32 " already in use\n", opts.nsid); - return 0; - } + } + + if (_spdk_nvmf_subsystem_get_ns(subsystem, opts.nsid)) { + SPDK_ERRLOG("Requested NSID %" PRIu32 " already in use\n", opts.nsid); + return 0; } if (opts.nsid > subsystem->max_nsid) { struct spdk_nvmf_ns **new_ns_array; + /* If MaxNamespaces was specified, we can't extend max_nsid beyond it. */ + if (subsystem->max_allowed_nsid > 0 && opts.nsid > subsystem->max_allowed_nsid) { + SPDK_ERRLOG("Can't extend NSID range above MaxNamespaces\n"); + return 0; + } + + /* If a controller is connected, we can't change NN. */ + if (!TAILQ_EMPTY(&subsystem->ctrlrs)) { + SPDK_ERRLOG("Can't extend NSID range while controllers are connected\n"); + return 0; + } + new_ns_array = realloc(subsystem->ns, sizeof(struct spdk_nvmf_ns *) * opts.nsid); if (new_ns_array == NULL) { SPDK_ERRLOG("Memory allocation error while resizing namespace array.\n"); diff --git a/test/nvmf/host/aer.sh b/test/nvmf/host/aer.sh index 28644fcdc1..03ae5f6689 100755 --- a/test/nvmf/host/aer.sh +++ b/test/nvmf/host/aer.sh @@ -30,7 +30,8 @@ timing_exit start_nvmf_tgt modprobe -v nvme-rdma $rpc_py construct_malloc_bdev 64 512 --name Malloc0 -$rpc_py construct_nvmf_subsystem nqn.2016-06.io.spdk:cnode1 "trtype:RDMA traddr:$NVMF_FIRST_TARGET_IP trsvcid:$NVMF_PORT" '' -a -s SPDK00000000000001 -n Malloc0 +$rpc_py construct_nvmf_subsystem nqn.2016-06.io.spdk:cnode1 "trtype:RDMA traddr:$NVMF_FIRST_TARGET_IP trsvcid:$NVMF_PORT" '' -a -s SPDK00000000000001 -n Malloc0 -m 2 +$rpc_py get_nvmf_subsystems # TODO: this aer test tries to invoke an AER completion by setting the temperature #threshold to a very low value. This does not work with emulated controllers @@ -58,6 +59,7 @@ sleep 5 # Add a new namespace $rpc_py construct_malloc_bdev 64 4096 --name Malloc1 $rpc_py nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode1 Malloc1 -n 2 +$rpc_py get_nvmf_subsystems wait $aerpid