From 0bd7ace836267ddc2bfc18fe1d5c6a2d38269f06 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Fri, 25 Mar 2022 19:08:00 +0000 Subject: [PATCH] bdev/nvme: add wait_for_attach param to discovery RPC Setting this optional parameter to true makes the RPC completion wait until the attach for all discovered NVM subsystems have completed. This is especially useful for fio or bdevperf, to ensure that all of the namespaces are actually available before testing. Signed-off-by: Jim Harris Change-Id: Icf04a122052f72e263a26b3c7582c81eac32a487 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/12044 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Tomasz Zawadzki --- doc/jsonrpc.md | 1 + module/bdev/nvme/bdev_nvme.c | 19 ++++++++++++++++++- module/bdev/nvme/bdev_nvme.h | 4 +++- module/bdev/nvme/bdev_nvme_rpc.c | 23 +++++++++++++++++++---- scripts/rpc.py | 3 +++ scripts/rpc/bdev.py | 8 ++++++-- 6 files changed, 50 insertions(+), 8 deletions(-) diff --git a/doc/jsonrpc.md b/doc/jsonrpc.md index 79a6a22d4..e39682de2 100644 --- a/doc/jsonrpc.md +++ b/doc/jsonrpc.md @@ -3272,6 +3272,7 @@ traddr | Required | string | NVMe-oF target address: ip adrfam | Optional | string | NVMe-oF target adrfam: ipv4, ipv6 trsvcid | Optional | string | NVMe-oF target trsvcid: port number hostnqn | Optional | string | NVMe-oF target hostnqn +wait_for_attach | Optional | bool | Wait to complete until all discovered NVM subsystems are attached ctrlr_loss_timeout_sec | Optional | number | Time to wait until ctrlr is reconnected before deleting ctrlr. -1 means infinite reconnects. 0 means no reconnect. reconnect_delay_sec | Optional | number | Time to delay a reconnect trial. 0 means no reconnect. fast_io_fail_timeout_sec | Optional | number | Time to wait until ctrlr is reconnected before failing I/O to ctrlr. 0 means no such timeout. diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index 7b70c8dda..9e555a06d 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -4327,6 +4327,7 @@ struct discovery_entry_ctx { struct discovery_ctx { char *name; + spdk_bdev_nvme_start_discovery_fn start_cb_fn; spdk_bdev_nvme_stop_discovery_fn stop_cb_fn; void *cb_ctx; struct spdk_nvme_probe_ctx *probe_ctx; @@ -4342,6 +4343,7 @@ struct discovery_ctx { TAILQ_HEAD(, discovery_entry_ctx) nvm_entry_ctxs; TAILQ_HEAD(, discovery_entry_ctx) discovery_entry_ctxs; int rc; + bool wait_for_attach; /* Denotes if a discovery is currently in progress for this context. * That includes connecting to newly discovered subsystems. Used to * ensure we do not start a new discovery until an existing one is @@ -4471,6 +4473,11 @@ discovery_attach_controller_done(void *cb_ctx, size_t bdev_count, int rc) DISCOVERY_INFOLOG(ctx, "attach %s done\n", entry_ctx->name); ctx->attach_in_progress--; if (ctx->attach_in_progress == 0) { + if (ctx->start_cb_fn) { + ctx->start_cb_fn(ctx->cb_ctx); + ctx->start_cb_fn = NULL; + ctx->cb_ctx = NULL; + } discovery_remove_controllers(ctx); } } @@ -4722,7 +4729,8 @@ int bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid, const char *base_name, struct spdk_nvme_ctrlr_opts *drv_opts, - struct nvme_ctrlr_opts *bdev_opts) + struct nvme_ctrlr_opts *bdev_opts, + spdk_bdev_nvme_start_discovery_fn cb_fn, void *cb_ctx) { struct discovery_ctx *ctx; struct discovery_entry_ctx *discovery_entry_ctx; @@ -4741,6 +4749,14 @@ bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid, memcpy(&ctx->bdev_opts, bdev_opts, sizeof(*bdev_opts)); ctx->bdev_opts.from_discovery_service = true; ctx->calling_thread = spdk_get_thread(); + if (ctx->start_cb_fn) { + /* We can use this when dumping json to denote if this RPC parameter + * was specified or not. + */ + ctx->wait_for_attach = true; + } + ctx->start_cb_fn = cb_fn; + ctx->cb_ctx = cb_ctx; TAILQ_INIT(&ctx->nvm_entry_ctxs); TAILQ_INIT(&ctx->discovery_entry_ctxs); snprintf(trid->subnqn, sizeof(trid->subnqn), "%s", SPDK_NVMF_DISCOVERY_NQN); @@ -5923,6 +5939,7 @@ bdev_nvme_discovery_config_json(struct spdk_json_write_ctx *w, struct discovery_ memset(trid.subnqn, 0, sizeof(trid.subnqn)); nvme_bdev_dump_trid_json(&trid, w); + spdk_json_write_named_bool(w, "wait_for_attach", ctx->wait_for_attach); spdk_json_write_named_int32(w, "ctrlr_loss_timeout_sec", ctx->bdev_opts.ctrlr_loss_timeout_sec); spdk_json_write_named_uint32(w, "reconnect_delay_sec", ctx->bdev_opts.reconnect_delay_sec); spdk_json_write_named_uint32(w, "fast_io_fail_timeout_sec", diff --git a/module/bdev/nvme/bdev_nvme.h b/module/bdev/nvme/bdev_nvme.h index b7681cace..412150320 100644 --- a/module/bdev/nvme/bdev_nvme.h +++ b/module/bdev/nvme/bdev_nvme.h @@ -49,6 +49,7 @@ extern bool g_bdev_nvme_module_finish; #define NVME_MAX_CONTROLLERS 1024 typedef void (*spdk_bdev_create_nvme_fn)(void *ctx, size_t bdev_count, int rc); +typedef void (*spdk_bdev_nvme_start_discovery_fn)(void *ctx); typedef void (*spdk_bdev_nvme_stop_discovery_fn)(void *ctx); struct nvme_ctrlr_opts { @@ -283,7 +284,8 @@ int bdev_nvme_create(struct spdk_nvme_transport_id *trid, bool multipath); int bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid, const char *base_name, - struct spdk_nvme_ctrlr_opts *drv_opts, struct nvme_ctrlr_opts *bdev_opts); + struct spdk_nvme_ctrlr_opts *drv_opts, struct nvme_ctrlr_opts *bdev_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 8fda23144..73a4dc30c 100644 --- a/module/bdev/nvme/bdev_nvme_rpc.c +++ b/module/bdev/nvme/bdev_nvme_rpc.c @@ -1608,6 +1608,7 @@ struct rpc_bdev_nvme_start_discovery { char *traddr; char *trsvcid; char *hostnqn; + bool wait_for_attach; struct spdk_nvme_ctrlr_opts opts; struct nvme_ctrlr_opts bdev_opts; }; @@ -1630,6 +1631,7 @@ static const struct spdk_json_object_decoder rpc_bdev_nvme_start_discovery_decod {"adrfam", offsetof(struct rpc_bdev_nvme_start_discovery, adrfam), spdk_json_decode_string, true}, {"trsvcid", offsetof(struct rpc_bdev_nvme_start_discovery, trsvcid), spdk_json_decode_string, true}, {"hostnqn", offsetof(struct rpc_bdev_nvme_start_discovery, hostnqn), spdk_json_decode_string, true}, + {"wait_for_attach", offsetof(struct rpc_bdev_nvme_start_discovery, wait_for_attach), spdk_json_decode_bool, true}, {"ctrlr_loss_timeout_sec", offsetof(struct rpc_bdev_nvme_start_discovery, bdev_opts.ctrlr_loss_timeout_sec), spdk_json_decode_int32, true}, {"reconnect_delay_sec", offsetof(struct rpc_bdev_nvme_start_discovery, bdev_opts.reconnect_delay_sec), spdk_json_decode_uint32, true}, {"fast_io_fail_timeout_sec", offsetof(struct rpc_bdev_nvme_start_discovery, bdev_opts.fast_io_fail_timeout_sec), spdk_json_decode_uint32, true}, @@ -1640,6 +1642,14 @@ struct rpc_bdev_nvme_start_discovery_ctx { struct spdk_jsonrpc_request *request; }; +static void +rpc_bdev_nvme_start_discovery_done(void *ctx) +{ + struct spdk_jsonrpc_request *request = ctx; + + spdk_jsonrpc_send_bool_response(request, true); +} + static void rpc_bdev_nvme_start_discovery(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params) @@ -1648,6 +1658,8 @@ rpc_bdev_nvme_start_discovery(struct spdk_jsonrpc_request *request, struct spdk_nvme_transport_id trid = {}; size_t len, maxlen; int rc; + spdk_bdev_nvme_start_discovery_fn cb_fn; + void *cb_ctx; ctx = calloc(1, sizeof(*ctx)); if (!ctx) { @@ -1718,11 +1730,14 @@ rpc_bdev_nvme_start_discovery(struct spdk_jsonrpc_request *request, } ctx->request = request; - rc = bdev_nvme_start_discovery(&trid, ctx->req.name, &ctx->req.opts, &ctx->req.bdev_opts); - if (rc == 0) { - spdk_jsonrpc_send_bool_response(ctx->request, true); - } else { + cb_fn = ctx->req.wait_for_attach ? rpc_bdev_nvme_start_discovery_done : NULL; + cb_ctx = ctx->req.wait_for_attach ? request : NULL; + rc = bdev_nvme_start_discovery(&trid, ctx->req.name, &ctx->req.opts, &ctx->req.bdev_opts, + cb_fn, cb_ctx); + if (rc) { spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc)); + } else if (!ctx->req.wait_for_attach) { + rpc_bdev_nvme_start_discovery_done(request); } cleanup: diff --git a/scripts/rpc.py b/scripts/rpc.py index 9d09c5f18..05a8268f7 100755 --- a/scripts/rpc.py +++ b/scripts/rpc.py @@ -711,6 +711,7 @@ if __name__ == "__main__": adrfam=args.adrfam, trsvcid=args.trsvcid, hostnqn=args.hostnqn, + wait_for_attach=args.wait_for_attach, ctrlr_loss_timeout_sec=args.ctrlr_loss_timeout_sec, reconnect_delay_sec=args.reconnect_delay_sec, fast_io_fail_timeout_sec=args.fast_io_fail_timeout_sec) @@ -726,6 +727,8 @@ if __name__ == "__main__": p.add_argument('-s', '--trsvcid', help='NVMe-oF target trsvcid: e.g., a port number') p.add_argument('-q', '--hostnqn', help='NVMe-oF host subnqn') + p.add_argument('-w', '--wait-for-attach', action='store_true', + help='Do not complete RPC until all discovered NVM subsystems are attached') p.add_argument('-l', '--ctrlr-loss-timeout-sec', help="""Time to wait until ctrlr is reconnected before deleting ctrlr. -1 means infinite reconnect retries. 0 means no reconnect retry. diff --git a/scripts/rpc/bdev.py b/scripts/rpc/bdev.py index 7f600cc96..c2f03a47c 100644 --- a/scripts/rpc/bdev.py +++ b/scripts/rpc/bdev.py @@ -745,8 +745,8 @@ def bdev_nvme_reset_controller(client, name): def bdev_nvme_start_discovery(client, name, trtype, traddr, adrfam=None, trsvcid=None, - hostnqn=None, ctrlr_loss_timeout_sec=None, reconnect_delay_sec=None, - fast_io_fail_timeout_sec=None): + hostnqn=None, wait_for_attach=None, ctrlr_loss_timeout_sec=None, + reconnect_delay_sec=None, fast_io_fail_timeout_sec=None): """Start discovery with the specified discovery subsystem Args: @@ -756,6 +756,7 @@ def bdev_nvme_start_discovery(client, name, trtype, traddr, adrfam=None, trsvcid adrfam: address family ("IPv4", "IPv6", "IB", or "FC") trsvcid: transport service ID (port number for IP-based addresses) hostnqn: NQN to connect from (optional) + wait_for_attach: Wait to complete RPC until all discovered NVM subsystems have attached (optional) ctrlr_loss_timeout_sec: Time to wait until ctrlr is reconnected before deleting ctrlr. -1 means infinite reconnect retries. 0 means no reconnect retry. If reconnect_delay_sec is zero, ctrlr_loss_timeout_sec has to be zero. @@ -784,6 +785,9 @@ def bdev_nvme_start_discovery(client, name, trtype, traddr, adrfam=None, trsvcid if trsvcid: params['trsvcid'] = trsvcid + if wait_for_attach: + params['wait_for_attach'] = True + if ctrlr_loss_timeout_sec is not None: params['ctrlr_loss_timeout_sec'] = ctrlr_loss_timeout_sec