lib/event: remove app.c dependency from subsystem initialization

This change adds return code to spdk_subsystem_init().
Making it's caller responsible for handling application
state - such as calling spdk_app_stop().

This change implies that start_subsystem_init RPC does not
stop the application on failure, only reports back the error.

Renamed g_app_start/stop variables to now more relevant
g_subsystem_start/stop.

Change-Id: I66a7da6ecfb234a569c65279cc4b210ddac53d2a
Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/464412
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>

CHANGELOG.md

@@ -53,6 +53,11 @@ Metadata support has been added to Null bdev module.
 
 Protection information support has been added to Null bdev module.
 
+### event
+
+start_subsystem_init RPC no longer stops the application on error during
+initialization.
+
 ### rpc
 
 Added optional parameter '--md-size'to 'construct_null_bdev' RPC method.

include/spdk_internal/event.h

@@ -86,13 +86,14 @@ extern struct spdk_subsystem_depend_list g_subsystems_deps;
 void spdk_add_subsystem(struct spdk_subsystem *subsystem);
 void spdk_add_subsystem_depend(struct spdk_subsystem_depend *depend);
 
-void spdk_subsystem_init(spdk_msg_fn cb_fn, void *cb_arg);
+typedef void (*spdk_subsystem_init_fn)(int rc, void *ctx);
+void spdk_subsystem_init(spdk_subsystem_init_fn cb_fn, void *cb_arg);
 void spdk_subsystem_fini(spdk_msg_fn cb_fn, void *cb_arg);
 void spdk_subsystem_init_next(int rc);
 void spdk_subsystem_fini_next(void);
 void spdk_subsystem_config(FILE *fp);
 void spdk_app_json_config_load(const char *json_config_file, const char *rpc_addr,
-			       spdk_msg_fn cb_fn, void *cb_arg);
+			       spdk_subsystem_init_fn cb_fn, void *cb_arg);
 
 /**
  * Save pointed \c subsystem configuration to the JSON write context \c w. In case of

lib/event/app.c

@@ -344,8 +344,13 @@ spdk_app_start_application(void)
 }
 
 static void
-spdk_app_start_rpc(void *arg1)
+spdk_app_start_rpc(int rc, void *arg1)
 {
+	if (rc) {
+		spdk_app_stop(rc);
+		return;
+	}
+
 	spdk_rpc_initialize(g_spdk_app.rpc_addr);
 	if (!g_delay_subsystem_init) {
 		spdk_rpc_set_state(SPDK_RPC_RUNTIME);
@@ -1051,13 +1056,19 @@ spdk_app_usage(void)
 }
 
 static void
-spdk_rpc_start_subsystem_init_cpl(void *arg1)
+spdk_rpc_start_subsystem_init_cpl(int rc, void *arg1)
 {
 	struct spdk_jsonrpc_request *request = arg1;
 	struct spdk_json_write_ctx *w;
 
 	assert(spdk_get_thread() == g_app_thread);
 
+	if (rc) {
+		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,
+						 "subsystem_initialization failed");
+		return;
+	}
+
 	spdk_rpc_set_state(SPDK_RPC_RUNTIME);
 	/* If we're loading JSON config file, we're still operating on a fake,
 	 * temporary RPC server. We'll have to defer calling the app start callback

lib/event/json_config.c

@@ -86,7 +86,7 @@ typedef void (*client_resp_handler)(struct load_json_config_ctx *,
 struct load_json_config_ctx {
 	/* Thread used during configuration. */
 	struct spdk_thread *thread;
-	spdk_msg_fn cb_fn;
+	spdk_subsystem_init_fn cb_fn;
 	void *cb_arg;
 
 	/* Current subsystem */
@@ -132,14 +132,9 @@ spdk_app_json_config_load_done(struct load_json_config_ctx *ctx, int rc)
 
 	spdk_rpc_finish();
 
-	if (rc) {
+	SPDK_DEBUG_APP_CFG("Config load finished with rc %d\n", rc);
-		SPDK_ERRLOG("Config load failed. Stopping SPDK application.\n");
+	ctx->cb_fn(rc, ctx->cb_arg);
-		spdk_app_stop(rc);
-	} else {
-		ctx->cb_fn(ctx->cb_arg);
-	}
 
-	SPDK_DEBUG_APP_CFG("Config load finished\n");
 	free(ctx->json_data);
 	free(ctx->values);
 	free(ctx);
@@ -544,14 +539,14 @@ err:
 
 void
 spdk_app_json_config_load(const char *json_config_file, const char *rpc_addr,
-			  spdk_msg_fn cb_fn, void *cb_arg)
+			  spdk_subsystem_init_fn cb_fn, void *cb_arg)
 {
 	struct load_json_config_ctx *ctx = calloc(1, sizeof(*ctx));
 	int rc;
 
 	assert(cb_fn);
 	if (!ctx) {
-		spdk_app_stop(-ENOMEM);
+		cb_fn(-ENOMEM, cb_arg);
 		return;
 	}
 

lib/event/subsystem.c

@@ -44,10 +44,10 @@ struct spdk_subsystem_depend_list g_subsystems_deps = TAILQ_HEAD_INITIALIZER(g_s
 static struct spdk_subsystem *g_next_subsystem;
 static bool g_subsystems_initialized = false;
 static bool g_subsystems_init_interrupted = false;
-static spdk_msg_fn g_app_start_fn = NULL;
+static spdk_subsystem_init_fn g_subsystem_start_fn = NULL;
-static void *g_app_start_arg = NULL;
+static void *g_subsystem_start_arg = NULL;
-static spdk_msg_fn g_app_stop_fn = NULL;
+static spdk_msg_fn g_subsystem_stop_fn = NULL;
-static void *g_app_stop_arg = NULL;
+static void *g_subsystem_stop_arg = NULL;
 static struct spdk_thread *g_fini_thread = NULL;
 
 void
@@ -127,7 +127,7 @@ spdk_subsystem_init_next(int rc)
 
 	if (rc) {
 		SPDK_ERRLOG("Init subsystem %s failed\n", g_next_subsystem->name);
-		spdk_app_stop(rc);
+		g_subsystem_start_fn(rc, g_subsystem_start_arg);
 		return;
 	}
 
@@ -139,7 +139,7 @@ spdk_subsystem_init_next(int rc)
 
 	if (!g_next_subsystem) {
 		g_subsystems_initialized = true;
-		g_app_start_fn(g_app_start_arg);
+		g_subsystem_start_fn(0, g_subsystem_start_arg);
 		return;
 	}
 
@@ -151,24 +151,24 @@ spdk_subsystem_init_next(int rc)
 }
 
 void
-spdk_subsystem_init(spdk_msg_fn cb_fn, void *cb_arg)
+spdk_subsystem_init(spdk_subsystem_init_fn cb_fn, void *cb_arg)
 {
 	struct spdk_subsystem_depend *dep;
 
-	g_app_start_fn = cb_fn;
+	g_subsystem_start_fn = cb_fn;
-	g_app_start_arg = cb_arg;
+	g_subsystem_start_arg = cb_arg;
 
 	/* Verify that all dependency name and depends_on subsystems are registered */
 	TAILQ_FOREACH(dep, &g_subsystems_deps, tailq) {
 		if (!spdk_subsystem_find(&g_subsystems, dep->name)) {
 			SPDK_ERRLOG("subsystem %s is missing\n", dep->name);
-			spdk_app_stop(-1);
+			g_subsystem_start_fn(-1, g_subsystem_start_arg);
 			return;
 		}
 		if (!spdk_subsystem_find(&g_subsystems, dep->depends_on)) {
 			SPDK_ERRLOG("subsystem %s dependency %s is missing\n",
 				    dep->name, dep->depends_on);
-			spdk_app_stop(-1);
+			g_subsystem_start_fn(-1, g_subsystem_start_arg);
 			return;
 		}
 	}
@@ -206,7 +206,7 @@ _spdk_subsystem_fini_next(void *arg1)
 		g_next_subsystem = TAILQ_PREV(g_next_subsystem, spdk_subsystem_list, tailq);
 	}
 
-	g_app_stop_fn(g_app_stop_arg);
+	g_subsystem_stop_fn(g_subsystem_stop_arg);
 	return;
 }
 
@@ -223,8 +223,8 @@ spdk_subsystem_fini_next(void)
 void
 spdk_subsystem_fini(spdk_msg_fn cb_fn, void *cb_arg)
 {
-	g_app_stop_fn = cb_fn;
+	g_subsystem_stop_fn = cb_fn;
-	g_app_stop_arg = cb_arg;
+	g_subsystem_stop_arg = cb_arg;
 
 	g_fini_thread = spdk_get_thread();
 

test/unit/lib/event/app.c/app_ut.c

@@ -42,13 +42,13 @@
 DEFINE_STUB_V(spdk_event_call, (struct spdk_event *event));
 DEFINE_STUB(spdk_event_allocate, struct spdk_event *, (uint32_t core, spdk_event_fn fn, void *arg1,
 		void *arg2), NULL);
-DEFINE_STUB_V(spdk_subsystem_init, (spdk_msg_fn cb_fn, void *cb_arg));
+DEFINE_STUB_V(spdk_subsystem_init, (spdk_subsystem_init_fn cb_fn, void *cb_arg));
 DEFINE_STUB_V(spdk_rpc_register_method, (const char *method, spdk_rpc_method_handler func,
 		uint32_t state_mask));
 DEFINE_STUB_V(spdk_rpc_set_state, (uint32_t state));
 DEFINE_STUB(spdk_rpc_get_state, uint32_t, (void), SPDK_RPC_RUNTIME);
 DEFINE_STUB_V(spdk_app_json_config_load, (const char *json_config_file, const char *rpc_addr,
-		spdk_msg_fn cb_fn, void *cb_arg));
+		spdk_subsystem_init_fn cb_fn, void *cb_arg));
 
 static void
 unittest_usage(void)

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

@@ -43,17 +43,12 @@ static struct spdk_subsystem g_ut_subsystems[8];
 static struct spdk_subsystem_depend g_ut_subsystem_deps[8];
 static int global_rc;
 
-void
+static void
-spdk_app_stop(int rc)
+ut_event_fn(int rc, void *arg1)
 {
 	global_rc = rc;
 }
 
-static void
-ut_event_fn(void *arg1)
-{
-}
-
 struct spdk_event *
 spdk_event_allocate(uint32_t core, spdk_event_fn fn, void *arg1, void *arg2)
 {
@@ -119,6 +114,7 @@ subsystem_sort_test_depends_on_single(void)
 
 	global_rc = -1;
 	spdk_subsystem_init(ut_event_fn, NULL);
+	CU_ASSERT(global_rc == 0);
 
 	i = 4;
 	TAILQ_FOREACH(subsystem, &g_subsystems, tailq) {
@@ -164,6 +160,7 @@ subsystem_sort_test_depends_on_multiple(void)
 
 	global_rc = -1;
 	spdk_subsystem_init(ut_event_fn, NULL);
+	CU_ASSERT(global_rc == 0);
 
 	subsystem = TAILQ_FIRST(&g_subsystems);
 	CU_ASSERT(strcmp(subsystem->name, "interface") == 0);