ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

iscsi: Consolidating checking uniqueness of IG into register/unregister

Browse Source 
Checking uniqueness of initiator group is done without mutex and
before register/unregister. This is not thread-safe.

This patch is a preparation to dynamic addition of initiator
information to existing initiator groups.

Change-Id: I44f48c857210522eee70d14bc3735ec73b0c5c6f
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/397032
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-02-05 13:02:25 +09:00 committed by Jim Harris
parent 2ea005f0a9
commit 13cffa4a0d
4 changed files with 51 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
lib/iscsi/init_grp.c
View File

@ -47,11 +47,6 @@ spdk_iscsi_init_grp_create(int tag)
{ {
struct spdk_iscsi_init_grp *ig; struct spdk_iscsi_init_grp *ig;
if (spdk_iscsi_init_grp_find_by_tag(tag)) {
SPDK_ERRLOG("duplicate initiator group tag (%d)\n", tag);
return NULL;
}
ig = calloc(1, sizeof(*ig)); ig = calloc(1, sizeof(*ig));
if (ig == NULL) { if (ig == NULL) {
SPDK_ERRLOG("calloc() failed for initiator group\n"); SPDK_ERRLOG("calloc() failed for initiator group\n");
@ -396,14 +391,23 @@ cleanup:
return rc; return rc;
} }
static void int
spdk_iscsi_init_grp_register(struct spdk_iscsi_init_grp *ig) spdk_iscsi_init_grp_register(struct spdk_iscsi_init_grp *ig)
{ {
struct spdk_iscsi_init_grp *tmp;
int rc = -1;
assert(ig != NULL); assert(ig != NULL);
pthread_mutex_lock(&g_spdk_iscsi.mutex); pthread_mutex_lock(&g_spdk_iscsi.mutex);
TAILQ_INSERT_TAIL(&g_spdk_iscsi.ig_head, ig, tailq); tmp = spdk_iscsi_init_grp_find_by_tag(ig->tag);
if (tmp == NULL) {
TAILQ_INSERT_TAIL(&g_spdk_iscsi.ig_head, ig, tailq);
rc = 0;
}
pthread_mutex_unlock(&g_spdk_iscsi.mutex); pthread_mutex_unlock(&g_spdk_iscsi.mutex);
return rc;
} }
/* /*
@ -442,19 +446,22 @@ spdk_iscsi_init_grp_create_from_initiator_list(int tag,
initiator_masks); initiator_masks);
if (rc < 0) { if (rc < 0) {
SPDK_ERRLOG("add initiator netmask error\n"); SPDK_ERRLOG("add initiator netmask error\n");
spdk_iscsi_init_grp_delete_all_initiators(ig);
goto cleanup; goto cleanup;
} }
spdk_iscsi_init_grp_register(ig); rc = spdk_iscsi_init_grp_register(ig);
if (rc < 0) {
SPDK_ERRLOG("initiator group register error (%d)\n", tag);
goto cleanup;
}
return 0; return 0;
cleanup: cleanup:
free(ig); spdk_iscsi_init_grp_destroy(ig);
return rc; return rc;
} }
static void void
spdk_iscsi_init_grp_destroy(struct spdk_iscsi_init_grp *ig) spdk_iscsi_init_grp_destroy(struct spdk_iscsi_init_grp *ig)
{ {
if (!ig) { if (!ig) {
@ -518,28 +525,20 @@ spdk_iscsi_init_grp_array_destroy(void)
} }
pthread_mutex_unlock(&g_spdk_iscsi.mutex); pthread_mutex_unlock(&g_spdk_iscsi.mutex);
} }
static inline void
spdk_initiator_group_unregister(struct spdk_iscsi_init_grp *ig)
{
struct spdk_iscsi_init_grp *initiator_group;
struct spdk_iscsi_init_grp *initiator_group_tmp;
assert(ig != NULL); struct spdk_iscsi_init_grp *
spdk_iscsi_init_grp_unregister(int tag)
{
struct spdk_iscsi_init_grp *ig;
pthread_mutex_lock(&g_spdk_iscsi.mutex); pthread_mutex_lock(&g_spdk_iscsi.mutex);
TAILQ_FOREACH_SAFE(initiator_group, &g_spdk_iscsi.ig_head, tailq, initiator_group_tmp) { TAILQ_FOREACH(ig, &g_spdk_iscsi.ig_head, tailq) {
if (ig->tag == initiator_group->tag) { if (ig->tag == tag) {
TAILQ_REMOVE(&g_spdk_iscsi.ig_head, initiator_group, tailq); TAILQ_REMOVE(&g_spdk_iscsi.ig_head, ig, tailq);
pthread_mutex_unlock(&g_spdk_iscsi.mutex);
return ig;
} }
} }
pthread_mutex_unlock(&g_spdk_iscsi.mutex); pthread_mutex_unlock(&g_spdk_iscsi.mutex);
} return NULL;
void
spdk_iscsi_init_grp_release(struct spdk_iscsi_init_grp *ig)
{
spdk_initiator_group_unregister(ig);
pthread_mutex_lock(&g_spdk_iscsi.mutex);
spdk_iscsi_init_grp_destroy(ig);
pthread_mutex_unlock(&g_spdk_iscsi.mutex);
} }

4
lib/iscsi/init_grp.h
View File

@ -61,8 +61,10 @@ struct spdk_iscsi_init_grp {
int spdk_iscsi_init_grp_create_from_initiator_list(int tag, int spdk_iscsi_init_grp_create_from_initiator_list(int tag,
int num_initiator_names, char **initiator_names, int num_initiator_names, char **initiator_names,
int num_initiator_masks, char **initiator_masks); int num_initiator_masks, char **initiator_masks);
void spdk_iscsi_init_grp_release(struct spdk_iscsi_init_grp *ig); int spdk_iscsi_init_grp_register(struct spdk_iscsi_init_grp *ig);
struct spdk_iscsi_init_grp *spdk_iscsi_init_grp_unregister(int tag);
struct spdk_iscsi_init_grp *spdk_iscsi_init_grp_find_by_tag(int tag); struct spdk_iscsi_init_grp *spdk_iscsi_init_grp_find_by_tag(int tag);
void spdk_iscsi_init_grp_destroy(struct spdk_iscsi_init_grp *ig);
int spdk_iscsi_init_grp_array_create(void); int spdk_iscsi_init_grp_array_create(void);
void spdk_iscsi_init_grp_array_destroy(void); void spdk_iscsi_init_grp_array_destroy(void);

4
lib/iscsi/iscsi_rpc.c
View File

@ -229,12 +229,12 @@ spdk_rpc_delete_initiator_group(struct spdk_jsonrpc_request *request,
goto invalid; goto invalid;
} }
ig = spdk_iscsi_init_grp_find_by_tag(req.tag); ig = spdk_iscsi_init_grp_unregister(req.tag);
if (!ig) { if (!ig) {
goto invalid; goto invalid;
} }
spdk_iscsi_tgt_node_delete_map(NULL, ig); spdk_iscsi_tgt_node_delete_map(NULL, ig);
spdk_iscsi_init_grp_release(ig); spdk_iscsi_init_grp_destroy(ig);
w = spdk_jsonrpc_begin_result(request); w = spdk_jsonrpc_begin_result(request);
if (w == NULL) { if (w == NULL) {

29
test/unit/lib/iscsi/init_grp.c/init_grp_ut.c
View File

@ -115,17 +115,20 @@ create_initiator_group_success_case(void)
static void static void
find_initiator_group_success_case(void) find_initiator_group_success_case(void)
{ {
struct spdk_iscsi_init_grp *ig; struct spdk_iscsi_init_grp *ig, *tmp;
int rc;
ig = spdk_iscsi_init_grp_create(1); ig = spdk_iscsi_init_grp_create(1);
CU_ASSERT(ig != NULL); CU_ASSERT(ig != NULL);
spdk_iscsi_init_grp_register(ig); rc = spdk_iscsi_init_grp_register(ig);
CU_ASSERT(rc == 0);
ig = spdk_iscsi_init_grp_find_by_tag(1); ig = spdk_iscsi_init_grp_find_by_tag(1);
CU_ASSERT(ig != NULL); CU_ASSERT(ig != NULL);
spdk_initiator_group_unregister(ig); tmp = spdk_iscsi_init_grp_unregister(1);
CU_ASSERT(ig == tmp);
spdk_iscsi_init_grp_destroy(ig); spdk_iscsi_init_grp_destroy(ig);
ig = spdk_iscsi_init_grp_find_by_tag(1); ig = spdk_iscsi_init_grp_find_by_tag(1);
@ -133,22 +136,25 @@ find_initiator_group_success_case(void)
} }
static void static void
create_initiator_group_fail_case(void) register_initiator_group_twice_case(void)
{ {
struct spdk_iscsi_init_grp *ig; struct spdk_iscsi_init_grp *ig, *tmp;
int rc;
ig = spdk_iscsi_init_grp_create(1); ig = spdk_iscsi_init_grp_create(1);
CU_ASSERT(ig != NULL); CU_ASSERT(ig != NULL);
spdk_iscsi_init_grp_register(ig); rc = spdk_iscsi_init_grp_register(ig);
CU_ASSERT(rc == 0);
ig = spdk_iscsi_init_grp_create(1); rc = spdk_iscsi_init_grp_register(ig);
CU_ASSERT(ig == NULL); CU_ASSERT(rc != 0);
ig = spdk_iscsi_init_grp_find_by_tag(1); ig = spdk_iscsi_init_grp_find_by_tag(1);
CU_ASSERT(ig != NULL); CU_ASSERT(ig != NULL);
spdk_initiator_group_unregister(ig); tmp = spdk_iscsi_init_grp_unregister(1);
CU_ASSERT(tmp == ig);
spdk_iscsi_init_grp_destroy(ig); spdk_iscsi_init_grp_destroy(ig);
ig = spdk_iscsi_init_grp_find_by_tag(1); ig = spdk_iscsi_init_grp_find_by_tag(1);
@ -158,6 +164,7 @@ create_initiator_group_fail_case(void)
static void static void
add_initiator_name_success_case(void) add_initiator_name_success_case(void)
{ {
int rc; int rc;
struct spdk_iscsi_init_grp *ig; struct spdk_iscsi_init_grp *ig;
struct spdk_iscsi_initiator_name *iname; struct spdk_iscsi_initiator_name *iname;
@ -475,8 +482,8 @@ main(int argc, char **argv)
create_initiator_group_success_case) == NULL create_initiator_group_success_case) == NULL
|| CU_add_test(suite, "find initiator group success case", || CU_add_test(suite, "find initiator group success case",
find_initiator_group_success_case) == NULL find_initiator_group_success_case) == NULL
|| CU_add_test(suite, "create initiator group fail case", || CU_add_test(suite, "register initiator group twice case",
create_initiator_group_fail_case) == NULL register_initiator_group_twice_case) == NULL
|| CU_add_test(suite, "add initiator name success case", || CU_add_test(suite, "add initiator name success case",
add_initiator_name_success_case) == NULL add_initiator_name_success_case) == NULL
|| CU_add_test(suite, "add initiator name fail case", || CU_add_test(suite, "add initiator name fail case",