diff --git a/lib/iscsi/iscsi_rpc.c b/lib/iscsi/iscsi_rpc.c index a02d8e98f..236b372ca 100644 --- a/lib/iscsi/iscsi_rpc.c +++ b/lib/iscsi/iscsi_rpc.c @@ -826,7 +826,8 @@ spdk_rpc_add_portal_group(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params) { struct rpc_portal_group req = {}; - struct spdk_iscsi_portal *portal_list[MAX_PORTAL] = {}; + struct spdk_iscsi_portal_grp *pg = NULL; + struct spdk_iscsi_portal *portal; struct spdk_json_write_ctx *w; size_t i = 0; int rc = -1; @@ -838,21 +839,31 @@ spdk_rpc_add_portal_group(struct spdk_jsonrpc_request *request, goto out; } + pg = spdk_iscsi_portal_grp_create(req.tag); + if (pg == NULL) { + SPDK_ERRLOG("portal_grp_create failed\n"); + goto out; + } for (i = 0; i < req.portal_list.num_portals; i++) { - portal_list[i] = spdk_iscsi_portal_create(req.portal_list.portals[i].host, - req.portal_list.portals[i].port, - req.portal_list.portals[i].cpumask); - if (portal_list[i] == NULL) { - SPDK_ERRLOG("portal_list allocation failed\n"); + portal = spdk_iscsi_portal_create(req.portal_list.portals[i].host, + req.portal_list.portals[i].port, + req.portal_list.portals[i].cpumask); + if (portal == NULL) { + SPDK_ERRLOG("portal_create failed\n"); goto out; } + spdk_iscsi_portal_grp_add_portal(pg, portal); } - rc = spdk_iscsi_portal_grp_create_from_portal_list(req.tag, portal_list, - req.portal_list.num_portals); + rc = spdk_iscsi_portal_grp_open(pg); + if (rc != 0) { + SPDK_ERRLOG("portal_grp_open failed\n"); + goto out; + } - if (rc < 0) { - SPDK_ERRLOG("create_from_portal_list failed\n"); + rc = spdk_iscsi_portal_grp_register(pg); + if (rc != 0) { + SPDK_ERRLOG("portal_grp_register failed\n"); } out: @@ -865,8 +876,8 @@ out: } else { spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters"); - for (; i > 0; --i) { - spdk_iscsi_portal_destroy(portal_list[i - 1]); + if (pg != NULL) { + spdk_iscsi_portal_grp_release(pg); } } free_rpc_portal_group(&req); diff --git a/lib/iscsi/iscsi_subsystem.c b/lib/iscsi/iscsi_subsystem.c index c63d75ae0..1155d9aa2 100644 --- a/lib/iscsi/iscsi_subsystem.c +++ b/lib/iscsi/iscsi_subsystem.c @@ -815,19 +815,6 @@ spdk_iscsi_app_read_parameters(void) return 0; } -static void -spdk_iscsi_setup(void *arg1, void *arg2) -{ - int rc; - - /* open portals */ - rc = spdk_iscsi_portal_grp_open_all(); - if (rc < 0) { - SPDK_ERRLOG("spdk_iscsi_portal_grp_open_all() failed\n"); - return; - } -} - int spdk_iscsi_init(void) { @@ -857,11 +844,6 @@ spdk_iscsi_init(void) return -1; } - /* - * Defer creation of listening sockets until the reactor has started. - */ - spdk_event_call(spdk_event_allocate(spdk_env_get_current_core(), spdk_iscsi_setup, NULL, NULL)); - return 0; } diff --git a/lib/iscsi/portal_grp.c b/lib/iscsi/portal_grp.c index 761905598..2fdbf8f63 100644 --- a/lib/iscsi/portal_grp.c +++ b/lib/iscsi/portal_grp.c @@ -48,9 +48,6 @@ #define PORTNUMSTRLEN 32 -static int -spdk_iscsi_portal_grp_open(struct spdk_iscsi_portal_grp *pg); - static struct spdk_iscsi_portal * spdk_iscsi_portal_find_by_addr(const char *host, const char *port) { @@ -195,6 +192,13 @@ spdk_iscsi_portal_open(struct spdk_iscsi_portal *p) p->sock = sock; + /* + * When the portal is created by config file, incoming connection + * requests for the socket are pended to accept until reactors start. + * However the gap between listen() and accept() will be slight and + * the requests will be queued by the nonzero backlog of the socket + * or resend by TCP. + */ spdk_iscsi_acceptor_start(p); return 0; @@ -326,7 +330,7 @@ error_out: return rc; } -static struct spdk_iscsi_portal_grp * +struct spdk_iscsi_portal_grp * spdk_iscsi_portal_grp_create(int tag) { struct spdk_iscsi_portal_grp *pg = malloc(sizeof(*pg)); @@ -344,7 +348,7 @@ spdk_iscsi_portal_grp_create(int tag) return pg; } -static void +void spdk_iscsi_portal_grp_destroy(struct spdk_iscsi_portal_grp *pg) { struct spdk_iscsi_portal *p; @@ -360,14 +364,13 @@ spdk_iscsi_portal_grp_destroy(struct spdk_iscsi_portal_grp *pg) free(pg); } -static int +int spdk_iscsi_portal_grp_register(struct spdk_iscsi_portal_grp *pg) { int rc = -1; struct spdk_iscsi_portal_grp *tmp; assert(pg != NULL); - assert(!TAILQ_EMPTY(&pg->head)); pthread_mutex_lock(&g_spdk_iscsi.mutex); tmp = spdk_iscsi_portal_grp_find_by_tag(pg->tag); @@ -379,7 +382,7 @@ spdk_iscsi_portal_grp_register(struct spdk_iscsi_portal_grp *pg) return rc; } -static void +void spdk_iscsi_portal_grp_add_portal(struct spdk_iscsi_portal_grp *pg, struct spdk_iscsi_portal *p) { @@ -390,71 +393,11 @@ spdk_iscsi_portal_grp_add_portal(struct spdk_iscsi_portal_grp *pg, TAILQ_INSERT_TAIL(&pg->head, p, per_pg_tailq); } -/** - * If all portals are valid, this function will take their ownership. - */ -int -spdk_iscsi_portal_grp_create_from_portal_list(int tag, - struct spdk_iscsi_portal **portal_list, - int num_portals) -{ - int i = 0, rc = 0; - struct spdk_iscsi_portal_grp *pg; - - SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "add portal group (from portal list) %d\n", tag); - - if (num_portals > MAX_PORTAL) { - SPDK_ERRLOG("%d > MAX_PORTAL\n", num_portals); - return -1; - } - - pg = spdk_iscsi_portal_grp_create(tag); - if (!pg) { - SPDK_ERRLOG("portal group creation error (%d)\n", tag); - return -1; - } - - for (i = 0; i < num_portals; i++) { - struct spdk_iscsi_portal *p = portal_list[i]; - - SPDK_DEBUGLOG(SPDK_LOG_ISCSI, - "RIndex=%d, Host=%s, Port=%s, Tag=%d\n", - i, p->host, p->port, tag); - rc = spdk_iscsi_portal_open(p); - if (rc < 0) { - /* if listening failed on any port, do not register the portal group - * and close any previously opened. */ - for (--i; i >= 0; --i) { - spdk_iscsi_portal_close(portal_list[i]); - } - - break; - } - } - - if (rc < 0) { - spdk_iscsi_portal_grp_destroy(pg); - } else { - /* Add portals to portal group */ - for (i = 0; i < num_portals; i++) { - spdk_iscsi_portal_grp_add_portal(pg, portal_list[i]); - } - - /* Add portal group to the end of the pg list */ - rc = spdk_iscsi_portal_grp_register(pg); - if (rc != 0) { - spdk_iscsi_portal_grp_destroy(pg); - } - } - - return rc; -} - static int spdk_iscsi_portal_grp_create_from_configfile(struct spdk_conf_section *sp) { struct spdk_iscsi_portal_grp *pg; - struct spdk_iscsi_portal *p; + struct spdk_iscsi_portal *p; const char *val; char *label, *portal; int portals = 0, i = 0, rc = 0; @@ -502,16 +445,14 @@ spdk_iscsi_portal_grp_create_from_configfile(struct spdk_conf_section *sp) label = spdk_conf_section_get_nmval(sp, "Portal", i, 0); portal = spdk_conf_section_get_nmval(sp, "Portal", i, 1); if (label == NULL || portal == NULL) { - spdk_iscsi_portal_grp_destroy(pg); SPDK_ERRLOG("portal error\n"); - return -1; + goto error; } rc = spdk_iscsi_portal_create_from_configline(portal, &p, 0); if (rc < 0) { - spdk_iscsi_portal_grp_destroy(pg); SPDK_ERRLOG("parse portal error (%s)\n", portal); - return -1; + goto error; } SPDK_DEBUGLOG(SPDK_LOG_ISCSI, @@ -521,15 +462,24 @@ spdk_iscsi_portal_grp_create_from_configfile(struct spdk_conf_section *sp) spdk_iscsi_portal_grp_add_portal(pg, p); } + rc = spdk_iscsi_portal_grp_open(pg); + if (rc != 0) { + SPDK_ERRLOG("portal_grp_open failed\n"); + goto error; + } + /* Add portal group to the end of the pg list */ rc = spdk_iscsi_portal_grp_register(pg); if (rc != 0) { SPDK_ERRLOG("register portal failed\n"); - spdk_iscsi_portal_grp_destroy(pg); - return -1; + goto error; } return 0; + +error: + spdk_iscsi_portal_grp_release(pg); + return -1; } struct spdk_iscsi_portal_grp * @@ -591,7 +541,7 @@ spdk_iscsi_portal_grp_array_destroy(void) pthread_mutex_unlock(&g_spdk_iscsi.mutex); } -static int +int spdk_iscsi_portal_grp_open(struct spdk_iscsi_portal_grp *pg) { struct spdk_iscsi_portal *p; @@ -606,25 +556,6 @@ spdk_iscsi_portal_grp_open(struct spdk_iscsi_portal_grp *pg) return 0; } -int -spdk_iscsi_portal_grp_open_all(void) -{ - struct spdk_iscsi_portal_grp *pg; - int rc; - - SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "spdk_iscsi_portal_grp_open_all\n"); - pthread_mutex_lock(&g_spdk_iscsi.mutex); - TAILQ_FOREACH(pg, &g_spdk_iscsi.pg_head, tailq) { - rc = spdk_iscsi_portal_grp_open(pg); - if (rc < 0) { - pthread_mutex_unlock(&g_spdk_iscsi.mutex); - return -1; - } - } - pthread_mutex_unlock(&g_spdk_iscsi.mutex); - return 0; -} - static void spdk_iscsi_portal_grp_close(struct spdk_iscsi_portal_grp *pg) { diff --git a/lib/iscsi/portal_grp.h b/lib/iscsi/portal_grp.h index ce030d86b..39e72899d 100644 --- a/lib/iscsi/portal_grp.h +++ b/lib/iscsi/portal_grp.h @@ -62,15 +62,17 @@ struct spdk_iscsi_portal *spdk_iscsi_portal_create(const char *host, const char const char *cpumask); void spdk_iscsi_portal_destroy(struct spdk_iscsi_portal *p); -int spdk_iscsi_portal_grp_create_from_portal_list(int tag, - struct spdk_iscsi_portal **portal_list, - int num_portals); +struct spdk_iscsi_portal_grp *spdk_iscsi_portal_grp_create(int tag); +void spdk_iscsi_portal_grp_add_portal(struct spdk_iscsi_portal_grp *pg, + struct spdk_iscsi_portal *p); +void spdk_iscsi_portal_grp_destroy(struct spdk_iscsi_portal_grp *pg); void spdk_iscsi_portal_grp_release(struct spdk_iscsi_portal_grp *pg); int spdk_iscsi_portal_grp_array_create(void); void spdk_iscsi_portal_grp_array_destroy(void); +int spdk_iscsi_portal_grp_register(struct spdk_iscsi_portal_grp *pg); struct spdk_iscsi_portal_grp *spdk_iscsi_portal_grp_unregister(int tag); struct spdk_iscsi_portal_grp *spdk_iscsi_portal_grp_find_by_tag(int tag); +int spdk_iscsi_portal_grp_open(struct spdk_iscsi_portal_grp *pg); -int spdk_iscsi_portal_grp_open_all(void); void spdk_iscsi_portal_grp_close_all(void); #endif // SPDK_PORTAL_GRP_H diff --git a/test/unit/lib/iscsi/portal_grp.c/portal_grp_ut.c b/test/unit/lib/iscsi/portal_grp.c/portal_grp_ut.c index 8e401198d..432f9ec32 100644 --- a/test/unit/lib/iscsi/portal_grp.c/portal_grp_ut.c +++ b/test/unit/lib/iscsi/portal_grp.c/portal_grp_ut.c @@ -371,6 +371,43 @@ portal_grp_register_twice_case(void) CU_ASSERT(TAILQ_EMPTY(&g_spdk_iscsi.portal_head)); } +static void +portal_grp_add_delete_case(void) +{ + struct spdk_iscsi_portal_grp *pg1, *pg2; + struct spdk_iscsi_portal *p; + int rc; + + const char *host = "192.168.2.0"; + const char *port = "3260"; + const char *cpumask = "1"; + + /* internal of add_portal_group */ + pg1 = spdk_iscsi_portal_grp_create(1); + CU_ASSERT(pg1 != NULL); + + p = spdk_iscsi_portal_create(host, port, cpumask); + CU_ASSERT(p != NULL); + + spdk_iscsi_portal_grp_add_portal(pg1, p); + + rc = spdk_iscsi_portal_grp_open(pg1); + CU_ASSERT(rc == 0); + + rc = spdk_iscsi_portal_grp_register(pg1); + CU_ASSERT(rc == 0); + + /* internal of delete_portal_group */ + pg2 = spdk_iscsi_portal_grp_unregister(1); + CU_ASSERT(pg2 != NULL); + CU_ASSERT(pg1 == pg2); + + spdk_iscsi_portal_grp_release(pg2); + + CU_ASSERT(TAILQ_EMPTY(&g_spdk_iscsi.portal_head)); + CU_ASSERT(TAILQ_EMPTY(&g_spdk_iscsi.pg_head)); +} + int main(int argc, char **argv) { @@ -418,6 +455,8 @@ main(int argc, char **argv) portal_grp_register_unregister_case) == NULL || CU_add_test(suite, "portal group register twice case", portal_grp_register_twice_case) == NULL + || CU_add_test(suite, "portal group add/delete case", + portal_grp_add_delete_case) == NULL ) { CU_cleanup_registry(); return CU_get_error();