From 250d342bc189262958ada11c40e0df9c2ce80a23 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Tue, 22 Aug 2017 08:54:12 -0700 Subject: [PATCH] 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 Reviewed-on: https://review.gerrithub.io/399519 Reviewed-by: Jim Harris Tested-by: SPDK Automated Test System Reviewed-by: Shuhei Matsumoto --- app/nvmf_tgt/conf.c | 6 +- app/nvmf_tgt/nvmf_rpc.c | 6 +- include/spdk/nvmf.h | 34 +++++++++- lib/nvmf/ctrlr.c | 4 +- lib/nvmf/ctrlr_bdev.c | 2 +- lib/nvmf/nvmf_internal.h | 2 +- lib/nvmf/subsystem.c | 63 ++++++++++++------- test/unit/lib/nvmf/subsystem.c/subsystem_ut.c | 16 +++-- 8 files changed, 99 insertions(+), 34 deletions(-) diff --git a/app/nvmf_tgt/conf.c b/app/nvmf_tgt/conf.c index 865eb39f0..d3779005e 100644 --- a/app/nvmf_tgt/conf.c +++ b/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; } diff --git a/app/nvmf_tgt/nvmf_rpc.c b/app/nvmf_tgt/nvmf_rpc.c index 50b770f04..7c37e793e 100644 --- a/app/nvmf_tgt/nvmf_rpc.c +++ b/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, diff --git a/include/spdk/nvmf.h b/include/spdk/nvmf.h index d3fd8cfe0..48c042169 100644 --- a/include/spdk/nvmf.h +++ b/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 - * NSID. + * \param opts Namespace options, or NULL to use defaults. + * \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); diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index 0501b948b..193f086ae 100644 --- a/lib/nvmf/ctrlr.c +++ b/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; } diff --git a/lib/nvmf/ctrlr_bdev.c b/lib/nvmf/ctrlr_bdev.c index 65bb2e50f..acdaf6325 100644 --- a/lib/nvmf/ctrlr_bdev.c +++ b/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; } } diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index 7e1f4262d..4f5b92684 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/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 { diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 12f6a8527..ab4af74d8 100644 --- a/lib/nvmf/subsystem.c +++ b/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_ERRLOG("Invalid NSID %" PRIu32 "\n", nsid); + spdk_nvmf_ns_opts_get_defaults(&opts, sizeof(opts)); + 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 || - (nsid == 0 && subsystem->num_allocated_nsid == subsystem->max_nsid)) { + 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 (nsid > subsystem->max_nsid) { - new_max_nsid = nsid; + if (opts.nsid > subsystem->max_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)) { - SPDK_ERRLOG("Requested NSID %" PRIu32 " already in use\n", nsid); + if (_spdk_nvmf_subsystem_get_ns(subsystem, opts.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) { diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c index 406d30475..bcf4fcc1e 100644 --- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +++ b/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);