nvmf: pass an options struct for ns creation

This will allow more parameters to be added to
spdk_nvmf_subsystem_add_ns() without breaking API/ABI compatibility
later.

Change-Id: I6b2f58f1a2d5fcd4c754830cbd4713dc461a31fc
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/399519
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>

app/nvmf_tgt/conf.c

@@ -393,6 +393,7 @@ struct spdk_nvmf_subsystem *
 
 	for (j = 0; j < num_ns; j++) {
 		struct spdk_nvmf_ns_params *ns_params = &ns_list[j];
+		struct spdk_nvmf_ns_opts ns_opts;
 
 		if (!ns_params->bdev_name) {
 			SPDK_ERRLOG("Namespace missing bdev name\n");
@@ -405,7 +406,10 @@ struct spdk_nvmf_subsystem *
 			goto error;
 		}
 
-		if (spdk_nvmf_subsystem_add_ns(subsystem, bdev, ns_params->nsid) == 0) {
+		spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
+		ns_opts.nsid = ns_params->nsid;
+
+		if (spdk_nvmf_subsystem_add_ns(subsystem, bdev, &ns_opts, sizeof(ns_opts)) == 0) {
 			goto error;
 		}
 

app/nvmf_tgt/nvmf_rpc.c

@@ -728,6 +728,7 @@ nvmf_rpc_ns_paused(struct spdk_nvmf_subsystem *subsystem,
 		   void *cb_arg, int status)
 {
 	struct nvmf_rpc_ns_ctx *ctx = cb_arg;
+	struct spdk_nvmf_ns_opts ns_opts;
 	struct spdk_bdev *bdev;
 
 	bdev = spdk_bdev_get_by_name(ctx->ns_params.bdev_name);
@@ -739,7 +740,10 @@ nvmf_rpc_ns_paused(struct spdk_nvmf_subsystem *subsystem,
 		goto resume;
 	}
 
-	ctx->ns_params.nsid = spdk_nvmf_subsystem_add_ns(subsystem, bdev, ctx->ns_params.nsid);
+	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
+	ns_opts.nsid = ctx->ns_params.nsid;
+
+	ctx->ns_params.nsid = spdk_nvmf_subsystem_add_ns(subsystem, bdev, &ns_opts, sizeof(ns_opts));
 	if (ctx->ns_params.nsid == 0) {
 		SPDK_ERRLOG("Unable to add namespace\n");
 		spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,

include/spdk/nvmf.h

@@ -368,6 +368,24 @@ struct spdk_nvmf_listener *spdk_nvmf_subsystem_get_next_listener(
 const struct spdk_nvme_transport_id *spdk_nvmf_listener_get_trid(
 	struct spdk_nvmf_listener *listener);
 
+/** NVMe-oF target namespace creation options */
+struct spdk_nvmf_ns_opts {
+	/**
+	 * Namespace ID
+	 *
+	 * Set to 0 to automatically assign a free NSID.
+	 */
+	uint32_t nsid;
+};
+
+/**
+ * Get default namespace creation options.
+ *
+ * \param opts Namespace options to fill with defaults.
+ * \param opts_size sizeof(struct spdk_nvmf_ns_opts)
+ */
+void spdk_nvmf_ns_opts_get_defaults(struct spdk_nvmf_ns_opts *opts, size_t opts_size);
+
 /**
  * Add a namespace to a subsytem.
  *
@@ -375,13 +393,13 @@ const struct spdk_nvme_transport_id *spdk_nvmf_listener_get_trid(
  *
  * \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
+ * \param opts Namespace options, or NULL to use defaults.
- *             NSID.
+ * \param opts_size sizeof(*opts)
  *
  * \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 struct spdk_nvmf_ns_opts *opts, size_t opts_size);
 
 /**
  * Remove a namespace from a subsytem.
@@ -439,6 +457,16 @@ uint32_t spdk_nvmf_ns_get_id(const struct spdk_nvmf_ns *ns);
  */
 struct spdk_bdev *spdk_nvmf_ns_get_bdev(struct spdk_nvmf_ns *ns);
 
+/**
+ * Get the options specified for a namespace.
+ *
+ * \param ns Namespace to query.
+ * \param opts Output parameter for options.
+ * \param opts_size sizeof(*opts)
+ */
+void spdk_nvmf_ns_get_opts(const struct spdk_nvmf_ns *ns, struct spdk_nvmf_ns_opts *opts,
+			   size_t opts_size);
+
 const char *spdk_nvmf_subsystem_get_sn(const struct spdk_nvmf_subsystem *subsystem);
 
 int spdk_nvmf_subsystem_set_sn(struct spdk_nvmf_subsystem *subsystem, const char *sn);

lib/nvmf/ctrlr.c

@@ -963,11 +963,11 @@ spdk_nvmf_ctrlr_identify_active_ns_list(struct spdk_nvmf_subsystem *subsystem,
 
 	for (ns = spdk_nvmf_subsystem_get_first_ns(subsystem); ns != NULL;
 	     ns = spdk_nvmf_subsystem_get_next_ns(subsystem, ns)) {
-		if (ns->id <= cmd->nsid) {
+		if (ns->opts.nsid <= cmd->nsid) {
 			continue;
 		}
 
-		ns_list->ns_list[count++] = ns->id;
+		ns_list->ns_list[count++] = ns->opts.nsid;
 		if (count == SPDK_COUNTOF(ns_list->ns_list)) {
 			break;
 		}

lib/nvmf/ctrlr_bdev.c

@@ -64,7 +64,7 @@ spdk_nvmf_subsystem_bdev_io_type_supported(struct spdk_nvmf_subsystem *subsystem
 			SPDK_DEBUGLOG(SPDK_LOG_NVMF,
 				      "Subsystem %s namespace %u (%s) does not support io_type %d\n",
 				      spdk_nvmf_subsystem_get_nqn(subsystem),
-				      ns->id, spdk_bdev_get_name(ns->bdev), (int)io_type);
+				      ns->opts.nsid, spdk_bdev_get_name(ns->bdev), (int)io_type);
 			return false;
 		}
 	}

lib/nvmf/nvmf_internal.h

@@ -138,7 +138,7 @@ struct spdk_nvmf_ns {
 	struct spdk_nvmf_subsystem *subsystem;
 	struct spdk_bdev *bdev;
 	struct spdk_bdev_desc *desc;
-	uint32_t id;
+	struct spdk_nvmf_ns_opts opts;
 };
 
 struct spdk_nvmf_qpair {

lib/nvmf/subsystem.c

@@ -317,7 +317,7 @@ spdk_nvmf_subsystem_destroy(struct spdk_nvmf_subsystem *subsystem)
 	while (ns != NULL) {
 		struct spdk_nvmf_ns *next_ns = spdk_nvmf_subsystem_get_next_ns(subsystem, ns);
 
-		spdk_nvmf_subsystem_remove_ns(subsystem, ns->id);
+		spdk_nvmf_subsystem_remove_ns(subsystem, ns->opts.nsid);
 		ns = next_ns;
 	}
 
@@ -801,7 +801,7 @@ _spdk_nvmf_ns_hot_remove(struct spdk_nvmf_subsystem *subsystem,
 {
 	struct spdk_nvmf_ns *ns = cb_arg;
 
-	spdk_nvmf_subsystem_remove_ns(ns->subsystem, ns->id);
+	spdk_nvmf_subsystem_remove_ns(subsystem, ns->opts.nsid);
 
 	spdk_nvmf_subsystem_resume(subsystem, NULL, NULL);
 }
@@ -818,10 +818,18 @@ spdk_nvmf_ns_hot_remove(void *remove_ctx)
 	}
 }
 
+void
+spdk_nvmf_ns_opts_get_defaults(struct spdk_nvmf_ns_opts *opts, size_t opts_size)
+{
+	/* All current fields are set to 0 by default. */
+	memset(opts, 0, opts_size);
+}
+
 uint32_t
 spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bdev *bdev,
-			   uint32_t nsid)
+			   const struct spdk_nvmf_ns_opts *user_opts, size_t opts_size)
 {
+	struct spdk_nvmf_ns_opts opts;
 	struct spdk_nvmf_ns *ns;
 	uint32_t i;
 	int rc;
@@ -831,18 +839,23 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd
 		return 0;
 	}
 
-	if (nsid == SPDK_NVME_GLOBAL_NS_TAG) {
+	spdk_nvmf_ns_opts_get_defaults(&opts, sizeof(opts));
-		SPDK_ERRLOG("Invalid NSID %" PRIu32 "\n", nsid);
+	if (user_opts) {
+		memcpy(&opts, user_opts, spdk_min(sizeof(opts), opts_size));
+	}
+
+	if (opts.nsid == SPDK_NVME_GLOBAL_NS_TAG) {
+		SPDK_ERRLOG("Invalid NSID %" PRIu32 "\n", opts.nsid);
 		return 0;
 	}
 
-	if (nsid > subsystem->max_nsid ||
+	if (opts.nsid > subsystem->max_nsid ||
-	    (nsid == 0 && subsystem->num_allocated_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 (nsid > subsystem->max_nsid) {
+		if (opts.nsid > subsystem->max_nsid) {
-			new_max_nsid = nsid;
+			new_max_nsid = opts.nsid;
 		} else {
 			new_max_nsid = subsystem->max_nsid + 1;
 		}
@@ -864,22 +877,22 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd
 		subsystem->max_nsid = new_max_nsid;
 	}
 
-	if (nsid == 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) {
-				nsid = i + 1;
+				opts.nsid = i + 1;
 				break;
 			}
 		}
-		if (nsid == 0) {
+		if (opts.nsid == 0) {
 			SPDK_ERRLOG("All available NSIDs in use\n");
 			return 0;
 		}
 	} else {
 		/* Specific NSID requested */
-		if (_spdk_nvmf_subsystem_get_ns(subsystem, nsid)) {
+		if (_spdk_nvmf_subsystem_get_ns(subsystem, opts.nsid)) {
-			SPDK_ERRLOG("Requested NSID %" PRIu32 " already in use\n", nsid);
+			SPDK_ERRLOG("Requested NSID %" PRIu32 " already in use\n", opts.nsid);
 			return 0;
 		}
 	}
@@ -891,7 +904,7 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd
 	}
 
 	ns->bdev = bdev;
-	ns->id = nsid;
+	ns->opts = opts;
 	ns->subsystem = subsystem;
 	rc = spdk_bdev_open(bdev, true, spdk_nvmf_ns_hot_remove, ns, &ns->desc);
 	if (rc != 0) {
@@ -900,17 +913,17 @@ spdk_nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem, struct spdk_bd
 		free(ns);
 		return 0;
 	}
-	subsystem->ns[nsid - 1] = ns;
+	subsystem->ns[opts.nsid - 1] = ns;
 
 	SPDK_DEBUGLOG(SPDK_LOG_NVMF, "Subsystem %s: bdev %s assigned nsid %" PRIu32 "\n",
 		      spdk_nvmf_subsystem_get_nqn(subsystem),
 		      spdk_bdev_get_name(bdev),
-		      nsid);
+		      opts.nsid);
 
-	subsystem->max_nsid = spdk_max(subsystem->max_nsid, nsid);
+	subsystem->max_nsid = spdk_max(subsystem->max_nsid, opts.nsid);
 	subsystem->num_allocated_nsid++;
 
-	return nsid;
+	return opts.nsid;
 }
 
 static uint32_t
@@ -947,7 +960,7 @@ spdk_nvmf_subsystem_get_next_ns(struct spdk_nvmf_subsystem *subsystem,
 {
 	uint32_t next_nsid;
 
-	next_nsid = spdk_nvmf_subsystem_get_next_allocated_nsid(subsystem, prev_ns->id);
+	next_nsid = spdk_nvmf_subsystem_get_next_allocated_nsid(subsystem, prev_ns->opts.nsid);
 	return _spdk_nvmf_subsystem_get_ns(subsystem, next_nsid);
 }
 
@@ -960,7 +973,7 @@ spdk_nvmf_subsystem_get_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid)
 uint32_t
 spdk_nvmf_ns_get_id(const struct spdk_nvmf_ns *ns)
 {
-	return ns->id;
+	return ns->opts.nsid;
 }
 
 struct spdk_bdev *
@@ -969,6 +982,14 @@ spdk_nvmf_ns_get_bdev(struct spdk_nvmf_ns *ns)
 	return ns->bdev;
 }
 
+void
+spdk_nvmf_ns_get_opts(const struct spdk_nvmf_ns *ns, struct spdk_nvmf_ns_opts *opts,
+		      size_t opts_size)
+{
+	memset(opts, 0, opts_size);
+	memcpy(opts, &ns->opts, spdk_min(sizeof(ns->opts), opts_size));
+}
+
 const char *
 spdk_nvmf_subsystem_get_sn(const struct spdk_nvmf_subsystem *subsystem)
 {

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

@@ -203,10 +203,12 @@ test_spdk_nvmf_subsystem_add_ns(void)
 		.ns = NULL,
 	};
 	struct spdk_bdev bdev1 = {}, bdev2 = {};
+	struct spdk_nvmf_ns_opts ns_opts;
 	uint32_t nsid;
 
 	/* Allow NSID to be assigned automatically */
-	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev1, 0);
+	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
+	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev1, &ns_opts, sizeof(ns_opts));
 	/* NSID 1 is the first unused ID */
 	CU_ASSERT(nsid == 1);
 	CU_ASSERT(subsystem.max_nsid == 1);
@@ -215,19 +217,25 @@ test_spdk_nvmf_subsystem_add_ns(void)
 	CU_ASSERT(subsystem.ns[nsid - 1]->bdev == &bdev1);
 
 	/* Request a specific NSID */
-	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, 5);
+	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
+	ns_opts.nsid = 5;
+	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, &ns_opts, sizeof(ns_opts));
 	CU_ASSERT(nsid == 5);
 	CU_ASSERT(subsystem.max_nsid == 5);
 	SPDK_CU_ASSERT_FATAL(subsystem.ns[nsid - 1] != NULL);
 	CU_ASSERT(subsystem.ns[nsid - 1]->bdev == &bdev2);
 
 	/* Request an NSID that is already in use */
-	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, 5);
+	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
+	ns_opts.nsid = 5;
+	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, &ns_opts, sizeof(ns_opts));
 	CU_ASSERT(nsid == 0);
 	CU_ASSERT(subsystem.max_nsid == 5);
 
 	/* Request 0xFFFFFFFF (invalid NSID, reserved for broadcast) */
-	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, 0xFFFFFFFF);
+	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));
+	ns_opts.nsid = 0xFFFFFFFF;
+	nsid = spdk_nvmf_subsystem_add_ns(&subsystem, &bdev2, &ns_opts, sizeof(ns_opts));
 	CU_ASSERT(nsid == 0);
 	CU_ASSERT(subsystem.max_nsid == 5);