nvmf: allow caller to pick NSID when adding ns

Change-Id: I1ea22fd3f56a3c048e25dead986992c848cf37b1
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/365508
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Sriram Popuri <sgpopuri@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>

app/nvmf_tgt/conf.c

@@ -590,7 +590,7 @@ spdk_nvmf_construct_subsystem(const char *name,
 				SPDK_ERRLOG("Could not find namespace bdev '%s'\n", namespace);
 				goto error;
 			}
-			if (spdk_nvmf_subsystem_add_ns(subsystem, bdev)) {
+			if (spdk_nvmf_subsystem_add_ns(subsystem, bdev, 0) == 0) {
 				goto error;
 			}
 

include/spdk/nvmf.h

@@ -169,7 +169,18 @@ int nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem,
 
 void spdk_nvmf_subsystem_poll(struct spdk_nvmf_subsystem *subsystem);
 
-int spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bdev *bdev);
+/**
+ * Add a namespace to a subsytem.
+ *
+ * \param subsystem Subsystem to add namespace to.
+ * \param bdev Block device to add as a namespace.
+ * \param nsid Namespace ID to assign to the new namespace, or 0 to automatically use an available
+ *             NSID.
+ *
+ * \return Newly added NSID on success or 0 on failure.
+ */
+uint32_t spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bdev *bdev,
+				    uint32_t nsid);
 
 const char *spdk_nvmf_subsystem_get_sn(const struct spdk_nvmf_subsystem *subsystem);
 

lib/nvmf/subsystem.c

@@ -384,29 +384,54 @@ static void spdk_nvmf_ctrlr_hot_remove(void *remove_ctx)
 	subsystem->is_removed = true;
 }
 
-int
+uint32_t
-spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bdev *bdev)
+spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bdev *bdev,
+			   uint32_t nsid)
 {
-	int i = 0;
+	uint32_t i;
 
 	assert(subsystem->mode == NVMF_SUBSYSTEM_MODE_VIRTUAL);
-	while (i < MAX_VIRTUAL_NAMESPACE && subsystem->dev.virt.ns_list[i]) {
+
-		i++;
+	if (nsid == 0) {
-	}
+		/* NSID not specified - find a free index */
-	if (i == MAX_VIRTUAL_NAMESPACE) {
+		for (i = 0; i < MAX_VIRTUAL_NAMESPACE; i++) {
-		SPDK_ERRLOG("spdk_nvmf_subsystem_add_ns() failed\n");
+			if (subsystem->dev.virt.ns_list[i] == NULL) {
-		return -1;
+				nsid = i + 1;
+				break;
+			}
+		}
+		if (nsid == 0) {
+			SPDK_ERRLOG("All available NSIDs in use\n");
+			return 0;
+		}
+	} else {
+		/* Specific NSID requested */
+		i = nsid - 1;
+		if (i >= MAX_VIRTUAL_NAMESPACE) {
+			SPDK_ERRLOG("Requested NSID %" PRIu32 " out of range\n", nsid);
+			return 0;
+		}
+
+		if (subsystem->dev.virt.ns_list[i]) {
+			SPDK_ERRLOG("Requested NSID %" PRIu32 " already in use\n", nsid);
+			return 0;
+		}
 	}
 
 	if (!spdk_bdev_claim(bdev, spdk_nvmf_ctrlr_hot_remove, subsystem)) {
 		SPDK_ERRLOG("Subsystem %s: bdev %s is already claimed\n",
 			    subsystem->subnqn, spdk_bdev_get_name(bdev));
-		return -1;
+		return 0;
 	}
 
+	SPDK_TRACELOG(SPDK_TRACE_NVMF, "Subsystem %s: bdev %s assigned nsid %" PRIu32 "\n",
+		      spdk_nvmf_subsystem_get_nqn(subsystem),
+		      spdk_bdev_get_name(bdev),
+		      nsid);
+
 	subsystem->dev.virt.ns_list[i] = bdev;
 	subsystem->dev.virt.ns_count++;
-	return 0;
+	return nsid;
 }
 
 const char *

test/lib/nvmf/subsystem/subsystem_ut.c

@@ -196,10 +196,27 @@ test_spdk_nvmf_subsystem_add_ns(void)
 		.dev.virt.ns_count = 0,
 		.dev.virt.ns_list = {},
 	};
-	struct spdk_bdev bdev = {};
+	struct spdk_bdev bdev1 = {}, bdev2 = {};
-	spdk_nvmf_subsystem_add_ns(&subsystem, &bdev);
+	uint32_t nsid;
+
+	/* Allow NSID to be assigned automatically */
+	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev1, 0);
+	/* NSID 1 is the first unused ID */
+	CU_ASSERT(nsid == 1);
 	CU_ASSERT(subsystem.dev.virt.ns_count == 1);
-	CU_ASSERT_PTR_EQUAL(subsystem.dev.virt.ns_list[0], &bdev);
+	CU_ASSERT(subsystem.dev.virt.ns_list[nsid - 1] == &bdev1);
+
+	/* Request a specific NSID */
+	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, 5);
+	CU_ASSERT(nsid == 5);
+	CU_ASSERT(subsystem.dev.virt.ns_count == 2);
+	CU_ASSERT(subsystem.dev.virt.ns_list[nsid - 1] == &bdev2);
+
+	/* Request an NSID that is already in use */
+	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, 5);
+	CU_ASSERT(nsid == 0);
+	/* ns_count should be unchanged */
+	CU_ASSERT(subsystem.dev.virt.ns_count == 2);
 }
 
 static void