iscsi: Unify the ordering of PG create_op between conf-file and JSON-RPC

Orderings of portal group create operation are diferrent between
config file and JSON-RPC. Unification is necessary for correct
concurrency control and the global accept poller like NVMf-tgt.
Hence unify the ordering of operations in this patch.

Common ordering of portal group create operation between configuration
file  and JSON-RPC after this patch is the following:
 - create a portal group
 - create portals
 - add the portals to the portal group
 - open the portals of the portal group
 - add the portal group to the global portal group list

After this patch, the gap between listening socket and accepting socket
will be increased a little when portals groups are creted by config file.
However this will cause no issue because of the TCP backlog and resend
mechanism.

Besides, necessary concurrency control is added and minor refactoring
is done.

About portal group delete operation, orderings of application shutdown
and JSON-RPC are already unified.

Change-Id: I1db7ef4400388127134d7734c68e138a4573b734
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/396848
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
This commit is contained in:
Shuhei Matsumoto 2018-01-29 13:36:03 +09:00 committed by Jim Harris
parent 04c6347b4e
commit 2ea005f0a9
5 changed files with 94 additions and 129 deletions

View File

@ -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);

View File

@ -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;
}

View File

@ -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)
{

View File

@ -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

View File

@ -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();