bdev/nvme: always defer start of discovery service

We used to wait until the discovery service could
connect to the discovery subsystem before calling
the callback function provided by the caller (mainly
the start_discovery RPC).

Moving forward, we will be handling the case where
the discovery subsystem is unavailable temporarily.
For now, let's not fail the bdev_nvme_start_discovery
call if we cannot connect to the discovery subsystem.
This will keep the initial service start path the same
as the path where the discovery subsystem is temporarily
unavailable.  In the future, we can consider adding
functionality to the start_discovery RPC that waits
up to X number of seconds to see if we were able to
connect and fail otherwise.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Icb05523b9d59f508bfbc0233595c8bf58c10488f
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11768
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>

module/bdev/nvme/bdev_nvme.c

@@ -4323,7 +4323,6 @@ 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;
@@ -4606,20 +4605,6 @@ discovery_aer_cb(void *arg, const struct spdk_nvme_cpl *cpl)
 	get_discovery_log_page(ctx);
 }
 
-static void
-start_discovery_done(void *cb_ctx)
-{
-	struct discovery_ctx *ctx = cb_ctx;
-
-	DISCOVERY_INFOLOG(ctx, "start discovery done\n");
-	ctx->start_cb_fn(ctx->cb_ctx, ctx->rc);
-	if (ctx->rc != 0) {
-		DISCOVERY_ERRLOG(ctx, "could not connect to discovery ctrlr\n");
-		TAILQ_REMOVE(&g_discovery_ctxs, ctx, tailq);
-		free_discovery_ctx(ctx);
-	}
-}
-
 static void
 discovery_attach_cb(void *cb_ctx, const struct spdk_nvme_transport_id *trid,
 		    struct spdk_nvme_ctrlr *ctrlr, const struct spdk_nvme_ctrlr_opts *opts)
@@ -4644,7 +4629,9 @@ discovery_poller(void *arg)
 	if (ctx->detach) {
 		bool detach_done = false;
 
-		if (ctx->detach_ctx == NULL) {
+		if (ctx->ctrlr == NULL) {
+			detach_done = true;
+		} else if (ctx->detach_ctx == NULL) {
 			rc = spdk_nvme_detach_async(ctx->ctrlr, &ctx->detach_ctx);
 			if (rc != 0) {
 				DISCOVERY_ERRLOG(ctx, "could not detach discovery ctrlr\n");
@@ -4662,12 +4649,16 @@ discovery_poller(void *arg)
 			ctx->stop_cb_fn(ctx->cb_ctx);
 			free_discovery_ctx(ctx);
 		}
+	} else if (ctx->probe_ctx == NULL && ctx->ctrlr == NULL) {
+		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");
+		}
 	} else if (ctx->probe_ctx) {
 		rc = spdk_nvme_probe_poll_async(ctx->probe_ctx);
 		if (rc != -EAGAIN) {
 			DISCOVERY_INFOLOG(ctx, "discovery ctrlr connected\n");
 			ctx->rc = rc;
-			spdk_thread_send_msg(ctx->calling_thread, start_discovery_done, ctx);
 			if (rc == 0) {
 				get_discovery_log_page(ctx);
 			}
@@ -4691,9 +4682,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 *drv_opts,
+			  struct spdk_nvme_ctrlr_opts *drv_opts)
-			  spdk_bdev_nvme_start_discovery_fn cb_fn,
-			  void *cb_ctx)
 {
 	struct discovery_ctx *ctx;
 
@@ -4707,8 +4696,6 @@ bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid,
 		free_discovery_ctx(ctx);
 		return -ENOMEM;
 	}
-	ctx->start_cb_fn = cb_fn;
-	ctx->cb_ctx = cb_ctx;
 	memcpy(&ctx->drv_opts, drv_opts, sizeof(*drv_opts));
 	ctx->calling_thread = spdk_get_thread();
 	TAILQ_INIT(&ctx->nvm_entry_ctxs);
@@ -4721,13 +4708,6 @@ bdev_nvme_start_discovery(struct spdk_nvme_transport_id *trid,
 		free_discovery_ctx(ctx);
 		return -ENOMEM;
 	}
-	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);
-		return -EIO;
-	}
-
 	spdk_thread_send_msg(g_bdev_nvme_init_thread, start_discovery_poller, ctx);
 	return 0;
 }

module/bdev/nvme/bdev_nvme.h

@@ -49,7 +49,6 @@ 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, int rc);
 typedef void (*spdk_bdev_nvme_stop_discovery_fn)(void *ctx);
 
 struct nvme_ctrlr_opts {
@@ -283,8 +282,7 @@ 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 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);
 

module/bdev/nvme/bdev_nvme_rpc.c

@@ -1634,25 +1634,6 @@ struct rpc_bdev_nvme_start_discovery_ctx {
 	struct spdk_jsonrpc_request *request;
 };
 
-static void
-rpc_bdev_nvme_start_discovery_done(void *cb_ctx, int rc)
-{
-	struct rpc_bdev_nvme_start_discovery_ctx *ctx = cb_ctx;
-	struct spdk_jsonrpc_request *request = ctx->request;
-
-	if (rc < 0) {
-		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters");
-		free_rpc_bdev_nvme_start_discovery(&ctx->req);
-		free(ctx);
-		return;
-	}
-
-	spdk_jsonrpc_send_bool_response(ctx->request, rc == 0);
-
-	free_rpc_bdev_nvme_start_discovery(&ctx->req);
-	free(ctx);
-}
-
 static void
 rpc_bdev_nvme_start_discovery(struct spdk_jsonrpc_request *request,
 			      const struct spdk_json_val *params)
@@ -1731,15 +1712,13 @@ 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,
+	rc = bdev_nvme_start_discovery(&trid, ctx->req.name, &ctx->req.opts);
-				       rpc_bdev_nvme_start_discovery_done, ctx);
+	if (rc == 0) {
-	if (rc) {
+		spdk_jsonrpc_send_bool_response(ctx->request, true);
+	} else {
 		spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc));
-		goto cleanup;
 	}
 
-	return;
-
 cleanup:
 	free_rpc_bdev_nvme_start_discovery(&ctx->req);
 	free(ctx);