From f046750c32911098a8ffeeaf3ef0bbde1a87dcce Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Tue, 2 Mar 2021 12:45:59 -0700 Subject: [PATCH] event: Add return code to spdk_rpc_initialize This is an internal API used in several places. The call can fail, so make sure it can report that correctly. Change-Id: Iac0ed2c8299c9dd3d2556070278a2224c3807b7b Signed-off-by: Ben Walker Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6640 Tested-by: SPDK CI Jenkins Reviewed-by: Reviewed-by: Aleksey Marchuk Reviewed-by: Tomasz Zawadzki Reviewed-by: Shuhei Matsumoto --- examples/nvmf/nvmf/nvmf.c | 8 +++++++- include/spdk_internal/event.h | 2 +- lib/event/app.c | 15 +++++++++++++-- lib/event/json_config.c | 7 +++++-- lib/event/rpc.c | 14 +++++++++----- test/unit/lib/event/app.c/app_ut.c | 2 +- 6 files changed, 36 insertions(+), 12 deletions(-) diff --git a/examples/nvmf/nvmf/nvmf.c b/examples/nvmf/nvmf/nvmf.c index 0acc51877c..8f7170980f 100644 --- a/examples/nvmf/nvmf/nvmf.c +++ b/examples/nvmf/nvmf/nvmf.c @@ -711,7 +711,13 @@ static void nvmf_subsystem_init_done(int rc, void *cb_arg) { fprintf(stdout, "bdev subsystem init successfully\n"); - spdk_rpc_initialize(g_rpc_addr); + + rc = spdk_rpc_initialize(g_rpc_addr); + if (rc) { + spdk_app_stop(rc); + return; + } + spdk_rpc_set_state(SPDK_RPC_RUNTIME); g_target_state = NVMF_INIT_TARGET; diff --git a/include/spdk_internal/event.h b/include/spdk_internal/event.h index 0e3f9442e0..fa1a282dcc 100644 --- a/include/spdk_internal/event.h +++ b/include/spdk_internal/event.h @@ -214,7 +214,7 @@ void spdk_app_json_config_load(const char *json_config_file, const char *rpc_add */ void spdk_subsystem_config_json(struct spdk_json_write_ctx *w, struct spdk_subsystem *subsystem); -void spdk_rpc_initialize(const char *listen_addr); +int spdk_rpc_initialize(const char *listen_addr); void spdk_rpc_finish(void); struct spdk_governor_capabilities { diff --git a/lib/event/app.c b/lib/event/app.c index 3a6ce7ca4a..3eb31da30b 100644 --- a/lib/event/app.c +++ b/lib/event/app.c @@ -278,7 +278,12 @@ app_start_rpc(int rc, void *arg1) return; } - spdk_rpc_initialize(g_spdk_app.rpc_addr); + rc = spdk_rpc_initialize(g_spdk_app.rpc_addr); + if (rc) { + spdk_app_stop(rc); + return; + } + if (!g_delay_subsystem_init) { spdk_rpc_set_state(SPDK_RPC_RUNTIME); app_start_application(); @@ -393,6 +398,8 @@ app_setup_trace(struct spdk_app_opts *opts) static void bootstrap_fn(void *arg1) { + int rc; + if (g_spdk_app.json_config_file) { g_delay_subsystem_init = false; spdk_app_json_config_load(g_spdk_app.json_config_file, g_spdk_app.rpc_addr, app_start_rpc, @@ -401,7 +408,11 @@ bootstrap_fn(void *arg1) if (!g_delay_subsystem_init) { spdk_subsystem_init(app_start_rpc, NULL); } else { - spdk_rpc_initialize(g_spdk_app.rpc_addr); + rc = spdk_rpc_initialize(g_spdk_app.rpc_addr); + if (rc) { + spdk_app_stop(rc); + return; + } } } } diff --git a/lib/event/json_config.c b/lib/event/json_config.c index 67890ded4d..da085846a2 100644 --- a/lib/event/json_config.c +++ b/lib/event/json_config.c @@ -614,8 +614,11 @@ spdk_app_json_config_load(const char *json_config_file, const char *rpc_addr, goto fail; } - /* FIXME: spdk_rpc_initialize() function should return error code. */ - spdk_rpc_initialize(ctx->rpc_socket_path_temp); + rc = spdk_rpc_initialize(ctx->rpc_socket_path_temp); + if (rc) { + goto fail; + } + ctx->client_conn = spdk_jsonrpc_client_connect(ctx->rpc_socket_path_temp, AF_UNIX); if (ctx->client_conn == NULL) { SPDK_ERRLOG("Failed to connect to '%s'\n", ctx->rpc_socket_path_temp); diff --git a/lib/event/rpc.c b/lib/event/rpc.c index 8e4b7d6188..62c8d5bb21 100644 --- a/lib/event/rpc.c +++ b/lib/event/rpc.c @@ -51,31 +51,35 @@ rpc_subsystem_poll(void *arg) return SPDK_POLLER_BUSY; } -void +int spdk_rpc_initialize(const char *listen_addr) { int rc; if (listen_addr == NULL) { - return; + /* Not treated as an error */ + return 0; } if (!spdk_rpc_verify_methods()) { - spdk_app_stop(-EINVAL); - return; + return -EINVAL; } /* Listen on the requested address */ rc = spdk_rpc_listen(listen_addr); if (rc != 0) { SPDK_ERRLOG("Unable to start RPC service at %s\n", listen_addr); - return; + /* TODO: Eventually, treat this as an error. But it historically has not + * been and many tests rely on this gracefully failing. */ + return 0; } spdk_rpc_set_state(SPDK_RPC_STARTUP); /* Register a poller to periodically check for RPCs */ g_rpc_poller = SPDK_POLLER_REGISTER(rpc_subsystem_poll, NULL, RPC_SELECT_INTERVAL); + + return 0; } void diff --git a/test/unit/lib/event/app.c/app_ut.c b/test/unit/lib/event/app.c/app_ut.c index 048ece07fa..932eef8f71 100644 --- a/test/unit/lib/event/app.c/app_ut.c +++ b/test/unit/lib/event/app.c/app_ut.c @@ -49,7 +49,7 @@ DEFINE_STUB_V(spdk_rpc_register_method, (const char *method, spdk_rpc_method_han DEFINE_STUB_V(spdk_rpc_register_alias_deprecated, (const char *method, const char *alias)); 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_rpc_initialize, (const char *listen_addr)); +DEFINE_STUB(spdk_rpc_initialize, int, (const char *listen_addr), 0); DEFINE_STUB_V(spdk_rpc_finish, (void)); DEFINE_STUB_V(spdk_app_json_config_load, (const char *json_config_file, const char *rpc_addr, spdk_subsystem_init_fn cb_fn, void *cb_arg, bool stop_on_error));