nvmf: Make spdk_nvmf_tgt_listen synchronous again

This was recently made asynchronous to support virtualized transports.
However, we're moving to add a new call to associated a listener with a
subsystem to transports and the operation that needed to be asynchronous
will actually be performed there. For simplicity, make this synchronous
again.

Change-Id: Ie98136a19c58f0f9bba0d140476de3bbb38e12d7
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/881
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>

include/spdk/nvmf.h

@@ -122,14 +122,6 @@ struct spdk_nvmf_transport_poll_group_stat {
  */
 typedef void (*new_qpair_fn)(struct spdk_nvmf_qpair *qpair, void *cb_arg);
 
-/**
- * Function to be called once the target is listening.
- *
- * \param ctx Context argument passed to this function.
- * \param status 0 if it completed successfully, or negative errno if it failed.
- */
-typedef void (*spdk_nvmf_tgt_listen_done_fn)(void *ctx, int status);
-
 /**
  * Construct an NVMe-oF target.
  *
@@ -211,16 +203,11 @@ void spdk_nvmf_tgt_write_config_json(struct spdk_json_write_ctx *w, struct spdk_
  *
  * \param tgt The target associated with this listen address.
  * \param trid The address to listen at.
- * \param cb_fn A callback that will be called once the target is listening
- * \param cb_arg A context argument passed to cb_fn.
  *
- * \return void. The callback status argument will be 0 on success
+ * \return 0 on success or a negated errno on failure.
- *	   or a negated errno on failure.
  */
-void spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
+int spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
-			  struct spdk_nvme_transport_id *trid,
+			 struct spdk_nvme_transport_id *trid);
-			  spdk_nvmf_tgt_listen_done_fn cb_fn,
-			  void *cb_arg);
 
 /**
  * Stop accepting new connections at the provided address.
@@ -984,16 +971,12 @@ void spdk_nvmf_tgt_add_transport(struct spdk_nvmf_tgt *tgt,
  *
  * \param transport The transport to add listener to
  * \param trid Address to listen at
- * \param cb_fn A callback that will be called once the listener is created
- * \param cb_arg A context argument passed to cb_fn.
  *
  * \return int. 0 if it completed successfully, or negative errno if it failed.
  */
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid,
+			   const struct spdk_nvme_transport_id *trid);
-			   spdk_nvmf_tgt_listen_done_fn cb_fn,
-			   void *cb_arg);
 
 /**
  * Remove listener from transport and stop accepting new connections.

include/spdk/nvmf_transport.h

@@ -209,9 +209,7 @@ struct spdk_nvmf_transport_ops {
 	  * provided. This may be called multiple times.
 	  */
 	int (*listen)(struct spdk_nvmf_transport *transport,
-		      const struct spdk_nvme_transport_id *trid,
+		      const struct spdk_nvme_transport_id *trid);
-		      spdk_nvmf_tgt_listen_done_fn cb_fn,
-		      void *cb_arg);
 
 	/**
 	  * Stop accepting new connections at the given address.

lib/nvmf/fc.c

@@ -1921,12 +1921,8 @@ nvmf_fc_destroy(struct spdk_nvmf_transport *transport)
 
 static int
 nvmf_fc_listen(struct spdk_nvmf_transport *transport,
-	       const struct spdk_nvme_transport_id *trid,
+	       const struct spdk_nvme_transport_id *trid)
-	       spdk_nvmf_tgt_listen_done_fn cb_fn,
-	       void *cb_arg)
 {
-	cb_fn(cb_arg, 0);
-
 	return 0;
 }
 

lib/nvmf/nvmf.c

@@ -541,11 +541,9 @@ spdk_nvmf_tgt_write_config_json(struct spdk_json_write_ctx *w, struct spdk_nvmf_
 	}
 }
 
-void
+int
 spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
-		     struct spdk_nvme_transport_id *trid,
+		     struct spdk_nvme_transport_id *trid)
-		     spdk_nvmf_tgt_listen_done_fn cb_fn,
-		     void *cb_arg)
 {
 	struct spdk_nvmf_transport *transport;
 	const char *trtype;
@@ -560,16 +558,16 @@ spdk_nvmf_tgt_listen(struct spdk_nvmf_tgt *tgt,
 			SPDK_ERRLOG("The specified trtype %d is unknown. Please make sure that it is properly registered.\n",
 				    trid->trtype);
 		}
-		cb_fn(cb_arg, -EINVAL);
+
-		return;
+		return -EINVAL;
 	}
 
-	rc = spdk_nvmf_transport_listen(transport, trid, cb_fn, cb_arg);
+	rc = spdk_nvmf_transport_listen(transport, trid);
 	if (rc < 0) {
 		SPDK_ERRLOG("Unable to listen on address '%s'\n", trid->traddr);
-		cb_fn(cb_arg, rc);
-		return;
 	}
+
+	return rc;
 }
 
 int

lib/nvmf/nvmf_rpc.c

@@ -633,42 +633,27 @@ nvmf_rpc_listen_resumed(struct spdk_nvmf_subsystem *subsystem,
 	spdk_jsonrpc_end_result(request, w);
 }
 
-static void
-nvmf_rpc_tgt_listen(void *cb_arg, int status)
-{
-	struct nvmf_rpc_listener_ctx *ctx = cb_arg;
-
-	if (status) {
-		spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,
-						 "Invalid parameters");
-		ctx->response_sent = true;
-	} else {
-		if (spdk_nvmf_subsystem_add_listener(ctx->subsystem, &ctx->trid)) {
-			spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,
-							 "Invalid parameters");
-			ctx->response_sent = true;
-		}
-	}
-
-	if (spdk_nvmf_subsystem_resume(ctx->subsystem, nvmf_rpc_listen_resumed, ctx)) {
-		if (!ctx->response_sent) {
-			spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, "Internal error");
-		}
-		nvmf_rpc_listener_ctx_free(ctx);
-		/* Can't really do anything to recover here - subsystem will remain paused. */
-	}
-}
-
 static void
 nvmf_rpc_listen_paused(struct spdk_nvmf_subsystem *subsystem,
 		       void *cb_arg, int status)
 {
 	struct nvmf_rpc_listener_ctx *ctx = cb_arg;
+	int rc;
 
 	if (ctx->op == NVMF_RPC_LISTEN_ADD) {
 		if (!spdk_nvmf_subsystem_find_listener(subsystem, &ctx->trid)) {
-			spdk_nvmf_tgt_listen(ctx->tgt, &ctx->trid, nvmf_rpc_tgt_listen, ctx);
+			rc = spdk_nvmf_tgt_listen(ctx->tgt, &ctx->trid);
-			return;
+			if (rc == 0) {
+				if (spdk_nvmf_subsystem_add_listener(ctx->subsystem, &ctx->trid)) {
+					spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,
+									 "Invalid parameters");
+					ctx->response_sent = true;
+				}
+			} else {
+				spdk_jsonrpc_send_error_response(ctx->request, SPDK_JSONRPC_ERROR_INVALID_PARAMS,
+								 "Invalid parameters");
+				ctx->response_sent = true;
+			}
 		}
 	} else if (ctx->op == NVMF_RPC_LISTEN_REMOVE) {
 		if (spdk_nvmf_subsystem_remove_listener(subsystem, &ctx->trid)) {

lib/nvmf/rdma.c

@@ -2643,9 +2643,7 @@ spdk_nvmf_rdma_trid_from_cm_id(struct rdma_cm_id *id,
 
 static int
 spdk_nvmf_rdma_listen(struct spdk_nvmf_transport *transport,
-		      const struct spdk_nvme_transport_id *trid,
+		      const struct spdk_nvme_transport_id *trid)
-		      spdk_nvmf_tgt_listen_done_fn cb_fn,
-		      void *cb_arg)
 {
 	struct spdk_nvmf_rdma_transport	*rtransport;
 	struct spdk_nvmf_rdma_device	*device;
@@ -2753,11 +2751,6 @@ spdk_nvmf_rdma_listen(struct spdk_nvmf_transport *transport,
 
 	TAILQ_INSERT_TAIL(&rtransport->ports, port, link);
 	pthread_mutex_unlock(&rtransport->lock);
-
-	if (cb_fn != NULL) {
-		cb_fn(cb_arg, 0);
-	}
-
 	return 0;
 }
 
@@ -2997,7 +2990,7 @@ nvmf_rdma_handle_cm_event_addr_change(struct spdk_nvmf_transport *transport,
 		nvmf_rdma_disconnect_qpairs_on_port(rtransport, port);
 
 		spdk_nvmf_rdma_stop_listen(transport, trid);
-		spdk_nvmf_rdma_listen(transport, trid, NULL, NULL);
+		spdk_nvmf_rdma_listen(transport, trid);
 	}
 
 	return event_acked;

lib/nvmf/tcp.c

@@ -597,9 +597,7 @@ _spdk_nvmf_tcp_find_port(struct spdk_nvmf_tcp_transport *ttransport,
 
 static int
 spdk_nvmf_tcp_listen(struct spdk_nvmf_transport *transport,
-		     const struct spdk_nvme_transport_id *trid,
+		     const struct spdk_nvme_transport_id *trid)
-		     spdk_nvmf_tgt_listen_done_fn cb_fn,
-		     void *cb_arg)
 {
 	struct spdk_nvmf_tcp_transport *ttransport;
 	struct spdk_nvmf_tcp_port *port;
@@ -655,11 +653,6 @@ spdk_nvmf_tcp_listen(struct spdk_nvmf_transport *transport,
 
 	TAILQ_INSERT_TAIL(&ttransport->ports, port, link);
 	pthread_mutex_unlock(&ttransport->lock);
-
-	if (cb_fn != NULL) {
-		cb_fn(cb_arg, 0);
-	}
-
 	return 0;
 }
 

lib/nvmf/transport.c

@@ -199,9 +199,7 @@ spdk_nvmf_transport_find_listener(struct spdk_nvmf_transport *transport,
 
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid,
+			   const struct spdk_nvme_transport_id *trid)
-			   spdk_nvmf_tgt_listen_done_fn cb_fn,
-			   void *cb_arg)
 {
 	struct spdk_nvmf_listener *listener;
 
@@ -215,12 +213,11 @@ spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
 		listener->ref = 1;
 		listener->trid = *trid;
 		TAILQ_INSERT_TAIL(&transport->listeners, listener, link);
-		return transport->ops->listen(transport, &listener->trid, cb_fn, cb_arg);
+		return transport->ops->listen(transport, &listener->trid);
 	}
 
 	++listener->ref;
 
-	cb_fn(cb_arg, 0);
 	return 0;
 }
 

module/event/subsystems/nvmf/conf.c

@@ -234,16 +234,6 @@ spdk_nvmf_parse_nvmf_tgt(void)
 	return 0;
 }
 
-static void
-spdk_nvmf_tgt_listen_done(void *cb_arg, int status)
-{
-	/* TODO: Config parsing should wait for this operation to finish. */
-
-	if (status) {
-		SPDK_ERRLOG("Failed to listen on transport address\n");
-	}
-}
-
 static int
 spdk_nvmf_tgt_parse_listen_ip_addr(char *address,
 				   struct spdk_nvme_transport_id *trid)
@@ -482,7 +472,9 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp)
 			continue;
 		}
 
-		spdk_nvmf_tgt_listen(g_spdk_nvmf_tgt, &trid, spdk_nvmf_tgt_listen_done, NULL);
+		if (spdk_nvmf_tgt_listen(g_spdk_nvmf_tgt, &trid)) {
+			SPDK_ERRLOG("Failed to listen on transport address\n");
+		}
 
 		spdk_nvmf_subsystem_add_listener(subsystem, &trid);
 		allow_any_listener = false;

test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c

@@ -79,9 +79,7 @@ spdk_bdev_get_uuid(const struct spdk_bdev *bdev)
 
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid,
+			   const struct spdk_nvme_transport_id *trid)
-			   spdk_nvmf_tgt_listen_done_fn cb_fn,
-			   void *cb_arg)
 {
 	return 0;
 }

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

@@ -66,9 +66,7 @@ DEFINE_STUB(spdk_nvmf_transport_stop_listen,
 
 int
 spdk_nvmf_transport_listen(struct spdk_nvmf_transport *transport,
-			   const struct spdk_nvme_transport_id *trid,
+			   const struct spdk_nvme_transport_id *trid)
-			   spdk_nvmf_tgt_listen_done_fn cb_fn,
-			   void *cb_arg)
 {
 	return 0;
 }