nvmf: The maximum number of namespaces a subsystem may contain must now

always be specified

Previously the parameter was optional and the size could be increased as
necesary. Now, it is required and a hard maximum. Later another function
could be added to dynamically increase or reduce this number if
necessary.

Change-Id: I3524ac737a6b592b4f6ce14ea48d3742a352c70f
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/4996
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>

include/spdk/nvmf.h

@@ -345,7 +345,7 @@ int spdk_nvmf_qpair_get_listen_trid(struct spdk_nvmf_qpair *qpair,
  * \param tgt The NVMe-oF target that will own this subsystem.
  * \param nqn The NVMe qualified name of this subsystem.
  * \param type Whether this subsystem is an I/O subsystem or a Discovery subsystem.
- * \param num_ns The number of namespaces this subsystem contains.
+ * \param num_ns The maximum number of namespaces this subsystem may contain.
  *
  * \return a pointer to a NVMe-oF subsystem on success, or NULL on failure.
  */

lib/nvmf/nvmf_internal.h

@@ -270,8 +270,6 @@ struct spdk_nvmf_subsystem {
 	/* Array of pointers to namespaces of size max_nsid indexed by nsid - 1 */
 	struct spdk_nvmf_ns				**ns;
 	uint32_t					max_nsid;
-	/* This is the maximum allowed nsid to a subsystem */
-	uint32_t					max_allowed_nsid;
 
 	TAILQ_HEAD(, spdk_nvmf_ctrlr)			ctrlrs;
 

lib/nvmf/subsystem.c

@@ -49,6 +49,7 @@
 #include "spdk_internal/utf.h"
 
 #define MODEL_NUMBER_DEFAULT "SPDK bdev Controller"
+#define NVMF_SUBSYSTEM_DEFAULT_NAMESPACES 32
 
 /*
  * States for parsing valid domains in NQNs according to RFC 1034
@@ -252,10 +253,14 @@ spdk_nvmf_subsystem_create(struct spdk_nvmf_tgt *tgt,
 		return NULL;
 	}
 
-	if (type == SPDK_NVMF_SUBTYPE_DISCOVERY && num_ns != 0) {
+	if (type == SPDK_NVMF_SUBTYPE_DISCOVERY) {
+		if (num_ns != 0) {
 			SPDK_ERRLOG("Discovery subsystem cannot have namespaces.\n");
 			return NULL;
 		}
+	} else if (num_ns == 0) {
+		num_ns = NVMF_SUBSYSTEM_DEFAULT_NAMESPACES;
+	}
 
 	/* Find a free subsystem id (sid) */
 	for (sid = 0; sid < tgt->max_subsystems; sid++) {
@@ -278,7 +283,6 @@ spdk_nvmf_subsystem_create(struct spdk_nvmf_tgt *tgt,
 	subsystem->id = sid;
 	subsystem->subtype = type;
 	subsystem->max_nsid = num_ns;
-	subsystem->max_allowed_nsid = num_ns;
 	subsystem->next_cntlid = 0;
 	snprintf(subsystem->subnqn, sizeof(subsystem->subnqn), "%s", nqn);
 	pthread_mutex_init(&subsystem->mutex, NULL);
@@ -1381,32 +1385,10 @@ spdk_nvmf_subsystem_add_ns_ext(struct spdk_nvmf_subsystem *subsystem, const char
 	}
 
 	if (opts.nsid > subsystem->max_nsid) {
-		struct spdk_nvmf_ns **new_ns_array;
+		SPDK_ERRLOG("NSID greater than maximum not allowed\n");
-
-		/* 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");
-			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");
@@ -1693,7 +1675,7 @@ nvmf_subsystem_get_ctrlr(struct spdk_nvmf_subsystem *subsystem, uint16_t cntlid)
 uint32_t
 spdk_nvmf_subsystem_get_max_namespaces(const struct spdk_nvmf_subsystem *subsystem)
 {
-	return subsystem->max_allowed_nsid;
+	return subsystem->max_nsid;
 }
 
 struct _nvmf_ns_registrant {

test/nvmf/target/identify_passthru.sh

@@ -38,7 +38,7 @@ $rpc_py nvmf_create_transport $NVMF_TRANSPORT_OPTS -u 8192
 timing_exit start_nvmf_tgt
 
 $rpc_py bdev_nvme_attach_controller -b Nvme0 -t PCIe -a ${bdf}
-$rpc_py nvmf_create_subsystem nqn.2016-06.io.spdk:cnode1 -a -s SPDK00000000000001
+$rpc_py nvmf_create_subsystem nqn.2016-06.io.spdk:cnode1 -a -s SPDK00000000000001 -m 1
 $rpc_py nvmf_subsystem_add_ns nqn.2016-06.io.spdk:cnode1 Nvme0n1
 $rpc_py nvmf_subsystem_add_listener nqn.2016-06.io.spdk:cnode1 -t $TEST_TRANSPORT -a $NVMF_FIRST_TARGET_IP -s $NVMF_PORT
 

test/unit/lib/nvmf/subsystem.c/subsystem_ut.c

@@ -285,7 +285,7 @@ test_spdk_nvmf_subsystem_add_ns(void)
 {
 	struct spdk_nvmf_tgt tgt = {};
 	struct spdk_nvmf_subsystem subsystem = {
-		.max_nsid = 0,
+		.max_nsid = 1024,
 		.ns = NULL,
 		.tgt = &tgt
 	};
@@ -293,26 +293,19 @@ test_spdk_nvmf_subsystem_add_ns(void)
 	uint32_t nsid;
 	int rc;
 
+	subsystem.ns = calloc(subsystem.max_nsid, sizeof(struct spdk_nvmf_subsystem_ns *));
+	SPDK_CU_ASSERT_FATAL(subsystem.ns != NULL);
+
 	tgt.max_subsystems = 1024;
 	tgt.subsystems = calloc(tgt.max_subsystems, sizeof(struct spdk_nvmf_subsystem *));
 	SPDK_CU_ASSERT_FATAL(tgt.subsystems != NULL);
 
-	/* Allow NSID to be assigned automatically */
-	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
-	nsid = spdk_nvmf_subsystem_add_ns_ext(&subsystem, "bdev1", &ns_opts, sizeof(ns_opts), NULL);
-	/* NSID 1 is the first unused ID */
-	CU_ASSERT(nsid == 1);
-	CU_ASSERT(subsystem.max_nsid == 1);
-	SPDK_CU_ASSERT_FATAL(subsystem.ns != NULL);
-	SPDK_CU_ASSERT_FATAL(subsystem.ns[nsid - 1] != NULL);
-	CU_ASSERT(subsystem.ns[nsid - 1]->bdev == &g_bdevs[nsid - 1]);
-
 	/* Request a specific NSID */
 	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
 	ns_opts.nsid = 5;
 	nsid = spdk_nvmf_subsystem_add_ns_ext(&subsystem, "bdev2", &ns_opts, sizeof(ns_opts), NULL);
 	CU_ASSERT(nsid == 5);
-	CU_ASSERT(subsystem.max_nsid == 5);
+	CU_ASSERT(subsystem.max_nsid == 1024);
 	SPDK_CU_ASSERT_FATAL(subsystem.ns[nsid - 1] != NULL);
 	CU_ASSERT(subsystem.ns[nsid - 1]->bdev == &g_bdevs[1]);
 
@@ -321,17 +314,15 @@ test_spdk_nvmf_subsystem_add_ns(void)
 	ns_opts.nsid = 5;
 	nsid = spdk_nvmf_subsystem_add_ns_ext(&subsystem, "bdev2", &ns_opts, sizeof(ns_opts), NULL);
 	CU_ASSERT(nsid == 0);
-	CU_ASSERT(subsystem.max_nsid == 5);
+	CU_ASSERT(subsystem.max_nsid == 1024);
 
 	/* Request 0xFFFFFFFF (invalid NSID, reserved for broadcast) */
 	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
 	ns_opts.nsid = 0xFFFFFFFF;
 	nsid = spdk_nvmf_subsystem_add_ns_ext(&subsystem, "bdev2", &ns_opts, sizeof(ns_opts), NULL);
 	CU_ASSERT(nsid == 0);
-	CU_ASSERT(subsystem.max_nsid == 5);
+	CU_ASSERT(subsystem.max_nsid == 1024);
 
-	rc = spdk_nvmf_subsystem_remove_ns(&subsystem, 1);
-	CU_ASSERT(rc == 0);
 	rc = spdk_nvmf_subsystem_remove_ns(&subsystem, 5);
 	CU_ASSERT(rc == 0);
 
@@ -1301,7 +1292,7 @@ test_spdk_nvmf_ns_event(void)
 {
 	struct spdk_nvmf_tgt tgt = {};
 	struct spdk_nvmf_subsystem subsystem = {
-		.max_nsid = 0,
+		.max_nsid = 1024,
 		.ns = NULL,
 		.tgt = &tgt
 	};
@@ -1312,6 +1303,9 @@ test_spdk_nvmf_ns_event(void)
 	uint32_t nsid;
 	struct spdk_bdev *bdev;
 
+	subsystem.ns = calloc(subsystem.max_nsid, sizeof(struct spdk_nvmf_subsystem_ns *));
+	SPDK_CU_ASSERT_FATAL(subsystem.ns != NULL);
+
 	tgt.max_subsystems = 1024;
 	tgt.subsystems = calloc(tgt.max_subsystems, sizeof(struct spdk_nvmf_subsystem *));
 	SPDK_CU_ASSERT_FATAL(tgt.subsystems != NULL);