From d40292d05a2e0cbde449fa9083f875cb576f7dab Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Tue, 8 Mar 2022 17:56:35 +0900 Subject: [PATCH] bdev/nvme: Add prefix "drv_" to instance or pointer of spdk_nvme_ctrlr_opts The following patches will add options per struct nvme_ctrlr in the NVMe bdev module. bdev_opts will be used for it. Additionally, fabrics_connect_timeout_us is set directly to spdk_nvme_ctrlr_opts. So remove it from the RPC request structure. Signed-off-by: Shuhei Matsumoto Change-Id: I981cda5e69375edc43a8581cd3b43497c38a3d56 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11827 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- module/bdev/nvme/bdev_nvme.c | 52 ++++++++++++++++---------------- module/bdev/nvme/bdev_nvme.h | 6 ++-- module/bdev/nvme/bdev_nvme_rpc.c | 39 ++++++++++++------------ 3 files changed, 48 insertions(+), 49 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 8b48e2553..fceaa7658 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -3412,8 +3412,8 @@ nvme_ctrlr_create(struct spdk_nvme_ctrlr *ctrlr, path_id->trid = *trid; if (ctx != NULL) { - memcpy(path_id->hostid.hostaddr, ctx->opts.src_addr, sizeof(path_id->hostid.hostaddr)); - memcpy(path_id->hostid.hostsvcid, ctx->opts.src_svcid, sizeof(path_id->hostid.hostsvcid)); + memcpy(path_id->hostid.hostaddr, ctx->drv_opts.src_addr, sizeof(path_id->hostid.hostaddr)); + memcpy(path_id->hostid.hostsvcid, ctx->drv_opts.src_svcid, sizeof(path_id->hostid.hostsvcid)); } nvme_ctrlr->active_path_id = path_id; TAILQ_INSERT_HEAD(&nvme_ctrlr->trids, path_id, link); @@ -3479,7 +3479,7 @@ err: static void attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, - struct spdk_nvme_ctrlr *ctrlr, const struct spdk_nvme_ctrlr_opts *opts) + struct spdk_nvme_ctrlr *ctrlr, const struct spdk_nvme_ctrlr_opts *drv_opts) { char *name; @@ -3837,7 +3837,7 @@ connect_attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, struct nvme_async_probe_ctx *ctx; int rc; - ctx = SPDK_CONTAINEROF(user_opts, struct nvme_async_probe_ctx, opts); + ctx = SPDK_CONTAINEROF(user_opts, struct nvme_async_probe_ctx, drv_opts); ctx->ctrlr_attached = true; rc = nvme_ctrlr_create(ctrlr, ctx->base_name, &ctx->trid, ctx); @@ -3856,7 +3856,7 @@ connect_set_failover_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, struct nvme_async_probe_ctx *ctx; int rc; - ctx = SPDK_CONTAINEROF(user_opts, struct nvme_async_probe_ctx, opts); + ctx = SPDK_CONTAINEROF(user_opts, struct nvme_async_probe_ctx, drv_opts); ctx->ctrlr_attached = true; nvme_ctrlr = nvme_ctrlr_get_by_name(ctx->base_name); @@ -3947,7 +3947,7 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, uint32_t prchk_flags, spdk_bdev_create_nvme_fn cb_fn, void *cb_ctx, - struct spdk_nvme_ctrlr_opts *opts, + struct spdk_nvme_ctrlr_opts *drv_opts, bool multipath, int32_t ctrlr_loss_timeout_sec, uint32_t reconnect_delay_sec, @@ -3995,16 +3995,16 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, } } - if (opts) { - memcpy(&ctx->opts, opts, sizeof(*opts)); + if (drv_opts) { + memcpy(&ctx->drv_opts, drv_opts, sizeof(*drv_opts)); } else { - spdk_nvme_ctrlr_get_default_ctrlr_opts(&ctx->opts, sizeof(ctx->opts)); + spdk_nvme_ctrlr_get_default_ctrlr_opts(&ctx->drv_opts, sizeof(ctx->drv_opts)); } - ctx->opts.transport_retry_count = g_opts.transport_retry_count; - ctx->opts.transport_ack_timeout = g_opts.transport_ack_timeout; - ctx->opts.keep_alive_timeout_ms = g_opts.keep_alive_timeout_ms; - ctx->opts.disable_read_ana_log_page = true; + ctx->drv_opts.transport_retry_count = g_opts.transport_retry_count; + ctx->drv_opts.transport_ack_timeout = g_opts.transport_ack_timeout; + ctx->drv_opts.keep_alive_timeout_ms = g_opts.keep_alive_timeout_ms; + ctx->drv_opts.disable_read_ana_log_page = true; if (nvme_bdev_ctrlr_get_by_name(base_name) == NULL || multipath) { attach_cb = connect_attach_cb; @@ -4012,7 +4012,7 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, attach_cb = connect_set_failover_cb; } - ctx->probe_ctx = spdk_nvme_connect_async(trid, &ctx->opts, attach_cb); + ctx->probe_ctx = spdk_nvme_connect_async(trid, &ctx->drv_opts, attach_cb); if (ctx->probe_ctx == NULL) { SPDK_ERRLOG("No controller was found with provided trid (traddr: %s)\n", trid->traddr); free(ctx); @@ -4130,7 +4130,7 @@ bdev_nvme_delete(const char *name, const struct nvme_path_id *path_id) struct discovery_entry_ctx { char name[128]; struct spdk_nvme_transport_id trid; - struct spdk_nvme_ctrlr_opts opts; + struct spdk_nvme_ctrlr_opts drv_opts; struct spdk_nvmf_discovery_log_page_entry entry; TAILQ_ENTRY(discovery_entry_ctx) tailq; struct discovery_ctx *ctx; @@ -4146,7 +4146,7 @@ struct discovery_ctx { struct spdk_nvme_ctrlr *ctrlr; struct spdk_nvme_transport_id trid; struct spdk_poller *poller; - struct spdk_nvme_ctrlr_opts opts; + struct spdk_nvme_ctrlr_opts drv_opts; struct spdk_nvmf_discovery_log_page *log_page; TAILQ_ENTRY(discovery_ctx) tailq; TAILQ_HEAD(, discovery_entry_ctx) nvm_entry_ctxs; @@ -4321,8 +4321,8 @@ discovery_log_page_cb(void *cb_arg, int rc, const struct spdk_nvme_cpl *cpl, new_ctx->ctx = ctx; memcpy(&new_ctx->entry, new_entry, sizeof(*new_entry)); build_trid_from_log_page_entry(&new_ctx->trid, new_entry); - spdk_nvme_ctrlr_get_default_ctrlr_opts(&new_ctx->opts, sizeof(new_ctx->opts)); - snprintf(new_ctx->opts.hostnqn, sizeof(new_ctx->opts.hostnqn), "%s", ctx->hostnqn); + spdk_nvme_ctrlr_get_default_ctrlr_opts(&new_ctx->drv_opts, sizeof(new_ctx->drv_opts)); + snprintf(new_ctx->drv_opts.hostnqn, sizeof(new_ctx->drv_opts.hostnqn), "%s", ctx->hostnqn); TAILQ_INSERT_TAIL(&ctx->discovery_entry_ctxs, new_ctx, tailq); continue; } @@ -4363,11 +4363,11 @@ discovery_log_page_cb(void *cb_arg, int rc, const struct spdk_nvme_cpl *cpl, new_ctx->trid.subnqn, new_ctx->trid.traddr, new_ctx->trid.trsvcid, new_ctx->name); } - spdk_nvme_ctrlr_get_default_ctrlr_opts(&new_ctx->opts, sizeof(new_ctx->opts)); - snprintf(new_ctx->opts.hostnqn, sizeof(new_ctx->opts.hostnqn), "%s", ctx->hostnqn); + spdk_nvme_ctrlr_get_default_ctrlr_opts(&new_ctx->drv_opts, sizeof(new_ctx->drv_opts)); + snprintf(new_ctx->drv_opts.hostnqn, sizeof(new_ctx->drv_opts.hostnqn), "%s", ctx->hostnqn); rc = bdev_nvme_create(&new_ctx->trid, new_ctx->name, NULL, 0, 0, discovery_attach_controller_done, new_ctx, - &new_ctx->opts, true, 0, 0, 0); + &new_ctx->drv_opts, true, 0, 0, 0); if (rc == 0) { TAILQ_INSERT_TAIL(&ctx->nvm_entry_ctxs, new_ctx, tailq); ctx->attach_in_progress++; @@ -4442,7 +4442,7 @@ discovery_attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid, struct spdk_nvme_ctrlr_opts *user_opts = cb_ctx; struct discovery_ctx *ctx; - ctx = SPDK_CONTAINEROF(user_opts, struct discovery_ctx, opts); + ctx = SPDK_CONTAINEROF(user_opts, struct discovery_ctx, drv_opts); DISCOVERY_DEBUGLOG(ctx, "discovery ctrlr attached\n"); ctx->probe_ctx = NULL; @@ -4506,7 +4506,7 @@ start_discovery_poller(void *arg) int bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid, const char *base_name, - struct spdk_nvme_ctrlr_opts *opts, + struct spdk_nvme_ctrlr_opts *drv_opts, spdk_bdev_nvme_start_discovery_fn cb_fn, void *cb_ctx) { @@ -4524,19 +4524,19 @@ bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid, } ctx->start_cb_fn = cb_fn; ctx->cb_ctx = cb_ctx; - memcpy(&ctx->opts, opts, sizeof(*opts)); + memcpy(&ctx->drv_opts, drv_opts, sizeof(*drv_opts)); ctx->calling_thread = spdk_get_thread(); TAILQ_INIT(&ctx->nvm_entry_ctxs); TAILQ_INIT(&ctx->discovery_entry_ctxs); snprintf(trid->subnqn, sizeof(trid->subnqn), "%s", SPDK_NVMF_DISCOVERY_NQN); memcpy(&ctx->trid, trid, sizeof(*trid)); /* Even if user did not specify hostnqn, we can still strdup("\0"); */ - ctx->hostnqn = strdup(ctx->opts.hostnqn); + ctx->hostnqn = strdup(ctx->drv_opts.hostnqn); if (ctx->hostnqn == NULL) { free_discovery_ctx(ctx); return -ENOMEM; } - ctx->probe_ctx = spdk_nvme_connect_async(&ctx->trid, &ctx->opts, discovery_attach_cb); + ctx->probe_ctx = spdk_nvme_connect_async(&ctx->trid, &ctx->drv_opts, discovery_attach_cb); if (ctx->probe_ctx == NULL) { DISCOVERY_ERRLOG(ctx, "could not start discovery connect\n"); free_discovery_ctx(ctx); diff --git a/module/bdev/nvme/bdev_nvme.h b/module/bdev/nvme/bdev_nvme.h index 0ea4c1e61..47e313758 100644 --- a/module/bdev/nvme/bdev_nvme.h +++ b/module/bdev/nvme/bdev_nvme.h @@ -63,7 +63,7 @@ struct nvme_async_probe_ctx { uint32_t fast_io_fail_timeout_sec; struct spdk_poller *poller; struct spdk_nvme_transport_id trid; - struct spdk_nvme_ctrlr_opts opts; + struct spdk_nvme_ctrlr_opts drv_opts; spdk_bdev_create_nvme_fn cb_fn; void *cb_ctx; uint32_t populates_in_progress; @@ -268,14 +268,14 @@ int bdev_nvme_create(struct spdk_nvme_transport_id *trid, uint32_t prchk_flags, spdk_bdev_create_nvme_fn cb_fn, void *cb_ctx, - struct spdk_nvme_ctrlr_opts *opts, + struct spdk_nvme_ctrlr_opts *drv_opts, bool multipath, int32_t ctrlr_loss_timeout_sec, uint32_t reconnect_delay_sec, uint32_t fast_io_fail_timeout_sec); int bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid, const char *base_name, - struct spdk_nvme_ctrlr_opts *opts, + struct spdk_nvme_ctrlr_opts *drv_opts, spdk_bdev_nvme_start_discovery_fn cb_fn, void *cb_ctx); int bdev_nvme_stop_discovery(const char *name, spdk_bdev_nvme_stop_discovery_fn cb_fn, void *cb_ctx); diff --git a/module/bdev/nvme/bdev_nvme_rpc.c b/module/bdev/nvme/bdev_nvme_rpc.c index eac9e5249..2f011280a 100644 --- a/module/bdev/nvme/bdev_nvme_rpc.c +++ b/module/bdev/nvme/bdev_nvme_rpc.c @@ -183,12 +183,11 @@ struct rpc_bdev_nvme_attach_controller { char *hostsvcid; bool prchk_reftag; bool prchk_guard; - uint64_t fabrics_connect_timeout_us; char *multipath; int32_t ctrlr_loss_timeout_sec; uint32_t reconnect_delay_sec; uint32_t fast_io_fail_timeout_sec; - struct spdk_nvme_ctrlr_opts opts; + struct spdk_nvme_ctrlr_opts drv_opts; }; static void @@ -222,11 +221,11 @@ static const struct spdk_json_object_decoder rpc_bdev_nvme_attach_controller_dec {"prchk_reftag", offsetof(struct rpc_bdev_nvme_attach_controller, prchk_reftag), spdk_json_decode_bool, true}, {"prchk_guard", offsetof(struct rpc_bdev_nvme_attach_controller, prchk_guard), spdk_json_decode_bool, true}, - {"hdgst", offsetof(struct rpc_bdev_nvme_attach_controller, opts.header_digest), spdk_json_decode_bool, true}, - {"ddgst", offsetof(struct rpc_bdev_nvme_attach_controller, opts.data_digest), spdk_json_decode_bool, true}, - {"fabrics_connect_timeout_us", offsetof(struct rpc_bdev_nvme_attach_controller, opts.fabrics_connect_timeout_us), spdk_json_decode_uint64, true}, + {"hdgst", offsetof(struct rpc_bdev_nvme_attach_controller, drv_opts.header_digest), spdk_json_decode_bool, true}, + {"ddgst", offsetof(struct rpc_bdev_nvme_attach_controller, drv_opts.data_digest), spdk_json_decode_bool, true}, + {"fabrics_connect_timeout_us", offsetof(struct rpc_bdev_nvme_attach_controller, drv_opts.fabrics_connect_timeout_us), spdk_json_decode_uint64, true}, {"multipath", offsetof(struct rpc_bdev_nvme_attach_controller, multipath), spdk_json_decode_string, true}, - {"num_io_queues", offsetof(struct rpc_bdev_nvme_attach_controller, opts.num_io_queues), spdk_json_decode_uint32, true}, + {"num_io_queues", offsetof(struct rpc_bdev_nvme_attach_controller, drv_opts.num_io_queues), spdk_json_decode_uint32, true}, {"ctrlr_loss_timeout_sec", offsetof(struct rpc_bdev_nvme_attach_controller, ctrlr_loss_timeout_sec), spdk_json_decode_int32, true}, {"reconnect_delay_sec", offsetof(struct rpc_bdev_nvme_attach_controller, reconnect_delay_sec), spdk_json_decode_uint32, true}, {"fast_io_fail_timeout_sec", offsetof(struct rpc_bdev_nvme_attach_controller, fast_io_fail_timeout_sec), spdk_json_decode_uint32, true}, @@ -285,7 +284,7 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, { struct rpc_bdev_nvme_attach_controller_ctx *ctx; struct spdk_nvme_transport_id trid = {}; - const struct spdk_nvme_ctrlr_opts *opts; + const struct spdk_nvme_ctrlr_opts *drv_opts; const struct spdk_nvme_transport_id *ctrlr_trid; uint32_t prchk_flags = 0; struct nvme_ctrlr *ctrlr = NULL; @@ -299,7 +298,7 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, return; } - spdk_nvme_ctrlr_get_default_ctrlr_opts(&ctx->req.opts, sizeof(ctx->req.opts)); + spdk_nvme_ctrlr_get_default_ctrlr_opts(&ctx->req.drv_opts, sizeof(ctx->req.drv_opts)); if (spdk_json_decode_object(params, rpc_bdev_nvme_attach_controller_decoders, SPDK_COUNTOF(rpc_bdev_nvme_attach_controller_decoders), @@ -374,30 +373,30 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, } if (ctx->req.hostnqn) { - snprintf(ctx->req.opts.hostnqn, sizeof(ctx->req.opts.hostnqn), "%s", + snprintf(ctx->req.drv_opts.hostnqn, sizeof(ctx->req.drv_opts.hostnqn), "%s", ctx->req.hostnqn); } if (ctx->req.hostaddr) { - maxlen = sizeof(ctx->req.opts.src_addr); + maxlen = sizeof(ctx->req.drv_opts.src_addr); len = strnlen(ctx->req.hostaddr, maxlen); if (len == maxlen) { spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, "hostaddr too long: %s", ctx->req.hostaddr); goto cleanup; } - snprintf(ctx->req.opts.src_addr, maxlen, "%s", ctx->req.hostaddr); + snprintf(ctx->req.drv_opts.src_addr, maxlen, "%s", ctx->req.hostaddr); } if (ctx->req.hostsvcid) { - maxlen = sizeof(ctx->req.opts.src_svcid); + maxlen = sizeof(ctx->req.drv_opts.src_svcid); len = strnlen(ctx->req.hostsvcid, maxlen); if (len == maxlen) { spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, "hostsvcid too long: %s", ctx->req.hostsvcid); goto cleanup; } - snprintf(ctx->req.opts.src_svcid, maxlen, "%s", ctx->req.hostsvcid); + snprintf(ctx->req.drv_opts.src_svcid, maxlen, "%s", ctx->req.hostsvcid); } ctrlr = nvme_ctrlr_get_by_name(ctx->req.name); @@ -416,7 +415,7 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, } } - opts = spdk_nvme_ctrlr_get_opts(ctrlr->ctrlr); + drv_opts = spdk_nvme_ctrlr_get_opts(ctrlr->ctrlr); ctrlr_trid = spdk_nvme_ctrlr_get_transport_id(ctrlr->ctrlr); /* This controller already exists. Check what the user wants to do. */ @@ -432,8 +431,8 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, if (strncmp(trid.traddr, ctrlr_trid->traddr, sizeof(trid.traddr)) == 0 && strncmp(trid.trsvcid, ctrlr_trid->trsvcid, sizeof(trid.trsvcid)) == 0 && - strncmp(ctx->req.opts.src_addr, opts->src_addr, sizeof(opts->src_addr)) == 0 && - strncmp(ctx->req.opts.src_svcid, opts->src_svcid, sizeof(opts->src_svcid)) == 0) { + strncmp(ctx->req.drv_opts.src_addr, drv_opts->src_addr, sizeof(drv_opts->src_addr)) == 0 && + strncmp(ctx->req.drv_opts.src_svcid, drv_opts->src_svcid, sizeof(drv_opts->src_svcid)) == 0) { /* Exactly same network path can't be added a second time */ spdk_jsonrpc_send_error_response_fmt(request, -EALREADY, "A controller named %s already exists with the specified network path\n", @@ -460,11 +459,11 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, - if (strncmp(ctx->req.opts.hostnqn, opts->hostnqn, SPDK_NVMF_NQN_MAX_LEN) != 0) { + if (strncmp(ctx->req.drv_opts.hostnqn, drv_opts->hostnqn, SPDK_NVMF_NQN_MAX_LEN) != 0) { /* Different HOSTNQN is not allowed when specifying the same controller name. */ spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, "A controller named %s already exists, but uses a different hostnqn (%s)\n", - ctx->req.name, opts->hostnqn); + ctx->req.name, drv_opts->hostnqn); goto cleanup; } @@ -488,7 +487,7 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, multipath = true; } - if (ctx->req.opts.num_io_queues == 0 || ctx->req.opts.num_io_queues > UINT16_MAX + 1) { + if (ctx->req.drv_opts.num_io_queues == 0 || ctx->req.drv_opts.num_io_queues > UINT16_MAX + 1) { spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, "num_io_queues out of bounds, min: %u max: %u\n", 1, UINT16_MAX + 1); @@ -498,7 +497,7 @@ rpc_bdev_nvme_attach_controller(struct spdk_jsonrpc_request *request, ctx->request = request; ctx->count = NVME_MAX_BDEVS_PER_RPC; rc = bdev_nvme_create(&trid, ctx->req.name, ctx->names, ctx->count, prchk_flags, - rpc_bdev_nvme_attach_controller_done, ctx, &ctx->req.opts, + rpc_bdev_nvme_attach_controller_done, ctx, &ctx->req.drv_opts, multipath, ctx->req.ctrlr_loss_timeout_sec, ctx->req.reconnect_delay_sec, ctx->req.fast_io_fail_timeout_sec); if (rc) {