From 1e481d0438e60bc6668656943945fe54d81559b4 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Fri, 25 May 2018 14:51:51 -0700 Subject: [PATCH] nvmf: Do not allow NN to change while connections present Per the NVMe specification, NN cannot change while there are connections present. There was originally a check for this that was removed in commit 763ab88 to match the behavior in the Linux kernel. However, after a discussion with the NVMe specification committee, SPDK was originally correct. Change-Id: I42414d1ee0c8c83f3335d8790edbf65d813c5c74 Signed-off-by: Ben Walker Reviewed-on: https://review.gerrithub.io/412544 Tested-by: SPDK Automated Test System Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris --- lib/nvmf/ctrlr.c | 6 +++- lib/nvmf/nvmf_internal.h | 2 +- lib/nvmf/subsystem.c | 61 ++++++++++++++++++---------------------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index b361dee0b..d7c812876 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -1271,7 +1271,11 @@ 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; - cdata->nn = subsystem->max_nsid; + if (subsystem->max_allowed_nsid != 0) { + cdata->nn = subsystem->max_allowed_nsid; + } else { + cdata->nn = subsystem->max_nsid; + } cdata->vwc.present = 1; cdata->nvmf_specific.ioccsz = sizeof(struct spdk_nvme_cmd) / 16; diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index 1df65f406..d9dee9032 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -44,6 +44,7 @@ #include "spdk/util.h" #define SPDK_NVMF_DEFAULT_NUM_CTRLRS_PER_LCORE 1 +#define SPDK_NVMF_DEFAULT_MAX_NSID 128 enum spdk_nvmf_subsystem_state { SPDK_NVMF_SUBSYSTEM_INACTIVE = 0, @@ -218,7 +219,6 @@ struct spdk_nvmf_subsystem { uint32_t max_nsid; /* This is the maximum allowed nsid to a subsystem */ uint32_t max_allowed_nsid; - uint32_t num_allocated_nsid; TAILQ_HEAD(, spdk_nvmf_ctrlr) ctrlrs; diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 7a050e05e..3697c40d3 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -273,12 +273,7 @@ spdk_nvmf_subsystem_create(struct spdk_nvmf_tgt *tgt, subsystem->id = sid; subsystem->subtype = type; subsystem->max_nsid = num_ns; - /* - * Initially max_nsid and max_allowed_nsid will be same. If max_allowed_nsid is zero nsid range can grow dynamically - * but if it is more than 1 nsid range cannot be extended beyond max_allowed_nsid - */ subsystem->max_allowed_nsid = num_ns; - subsystem->num_allocated_nsid = 0; subsystem->next_cntlid = 0; snprintf(subsystem->subnqn, sizeof(subsystem->subnqn), "%s", nqn); TAILQ_INIT(&subsystem->listeners); @@ -901,7 +896,6 @@ _spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t n spdk_bdev_close(ns->desc); free(ns); - subsystem->num_allocated_nsid--; spdk_nvmf_subsystem_ns_changed(subsystem, nsid); @@ -1006,29 +1000,6 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd return 0; } - if (opts.nsid > subsystem->max_nsid || - (opts.nsid == 0 && subsystem->num_allocated_nsid == subsystem->max_nsid)) { - struct spdk_nvmf_ns **new_ns_array; - uint32_t new_max_nsid; - - if (opts.nsid > subsystem->max_nsid) { - new_max_nsid = opts.nsid; - } else { - new_max_nsid = subsystem->max_nsid + 1; - } - - new_ns_array = realloc(subsystem->ns, sizeof(struct spdk_nvmf_ns *) * new_max_nsid); - if (new_ns_array == NULL) { - SPDK_ERRLOG("Memory allocation error while resizing namespace array.\n"); - return 0; - } - - memset(new_ns_array + subsystem->max_nsid, 0, - sizeof(struct spdk_nvmf_ns *) * (new_max_nsid - subsystem->max_nsid)); - subsystem->ns = new_ns_array; - subsystem->max_nsid = new_max_nsid; - } - if (opts.nsid == 0) { /* NSID not specified - find a free index */ for (i = 0; i < subsystem->max_nsid; i++) { @@ -1038,8 +1009,18 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd } } if (opts.nsid == 0) { - SPDK_ERRLOG("All available NSIDs in use\n"); - return 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 */ @@ -1049,6 +1030,21 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd } } + if (opts.nsid > subsystem->max_nsid) { + struct spdk_nvmf_ns **new_ns_array; + + 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"); + return 0; + } + + memset(new_ns_array + subsystem->max_nsid, 0, + sizeof(struct spdk_nvmf_ns *) * (opts.nsid - subsystem->max_nsid)); + subsystem->ns = new_ns_array; + subsystem->max_nsid = opts.nsid; + } + ns = calloc(1, sizeof(*ns)); if (ns == NULL) { SPDK_ERRLOG("Namespace allocation failed\n"); @@ -1072,9 +1068,6 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd spdk_bdev_get_name(bdev), opts.nsid); - subsystem->max_nsid = spdk_max(subsystem->max_nsid, opts.nsid); - subsystem->num_allocated_nsid++; - spdk_nvmf_subsystem_ns_changed(subsystem, opts.nsid); return opts.nsid;