[Libstoragemgmt-devel] [PATCH 0/5] NAME_CONFLICT, EXISTS_INITIATOR errors of access_group

Discussion:

[Libstoragemgmt-devel] [PATCH 0/5] NAME_CONFLICT, EXISTS_INITIATOR errors of access_group_create()

Gris Ge

2014-11-24 10:04:49 UTC

* This patch set is based on:
[PATCH 0/6] Raise NO_STATE_CHANGE error in volume mask and unmask.

* This patch set ensure all plugins in current code tree follow the
API error definition of access_group_create():
1. Raise NAME_CONFLICT if requested access group is already used by
other access group.
2. Raise EXISTS_INITIATOR if requested initiator ID is already used
by other access group.

* Plugin test has been updated to include test for these two errors.

Gris Ge (5):
SMI-S Plugin: Raise error for duplicate call of access_group_create()
ONTAP Plugin: Improve error handling in access_group_create()
Simulator Plugin: Fix incorrect error handling in
access_group_create()
Simulator C Plugin: Raise EXISTS_INITIATOR in access_group_create()
Plugin Test: Add NAME_CONFLICT and EXISTS_INITIATOR tests of
access_group_create()

plugin/ontap/ontap.py | 24 ++++++++++----
plugin/sim/simarray.py | 21 ++----------
plugin/simc/simc_lsmplugin.c | 77 ++++++++++++++++++++++++++++++++------------
plugin/smispy/smis.py | 28 ++++------------
test/plugin_test.py | 32 ++++++++++++++++++
5 files changed, 117 insertions(+), 65 deletions(-)

--
1.8.3.1

Gris Ge

2014-11-24 10:04:50 UTC

Permalink

* Raise EXISTS_INITIATOR when requested initiator is already used by other
access group on access_group_create().

* Tested on EMC VMAX for EXISTS_INITIATOR error.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index e7789c6..63ffa2f 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -1953,42 +1953,28 @@ def access_group_create(self, name, init_id, init_type, system,
except (LsmError, CIMError):
# Check possible failure
# 1. Initiator already exist in other group.
- # If that group hold the same name as requested.
- # We consider as a duplicate call, return the exist one.
- exist_cim_init_mgs = self._c.Associators(
+ exist_cim_init_mg_paths = self._c.AssociatorNames(
cim_init_path,
AssocClass='CIM_MemberOfCollection',
- ResultClass='CIM_InitiatorMaskingGroup',
- PropertyList=cim_init_mg_pros)
+ ResultClass='CIM_InitiatorMaskingGroup')

- if len(exist_cim_init_mgs) != 0:
- for exist_cim_init_mg in exist_cim_init_mgs:
- if exist_cim_init_mg['ElementName'] == name:
- return smis_ag.cim_init_mg_to_lsm_ag(
- self._c, exist_cim_init_mg, system.id)
-
- # Name does not match.
+ if len(exist_cim_init_mg_paths) != 0:
raise LsmError(ErrorNumber.EXISTS_INITIATOR,
"Initiator %s " % org_init_id +
- "already exist in other access group "
- "with name %s and ID: %s" %
- (exist_cim_init_mgs[0]['ElementName'],
- md5(exist_cim_init_mgs[0]['InstanceID'])))
+ "already exist in other access group")
+
# 2. Requested name used by other group.
- # Since 1) already checked whether any group containing
- # requested init_id, now, it's surly a conflict.
exist_cim_init_mgs = self._cim_init_mg_of(
system.id, property_list=['ElementName'])
for exist_cim_init_mg in exist_cim_init_mgs:
if exist_cim_init_mg['ElementName'] == name:
raise LsmError(ErrorNumber.NAME_CONFLICT,
"Requested name %s is used by " % name +
- "another access group, but not containing "
- "requested initiator %s" % org_init_id)
+ "another access group")
raise

cim_init_mg = self._c.GetInstance(
- cim_init_mg_path, PropertyList=cim_init_mg_pros, LocalOnly=False)
+ cim_init_mg_path, PropertyList=cim_init_mg_pros)
return smis_ag.cim_init_mg_to_lsm_ag(self._c, cim_init_mg, system.id)

@handle_cim_errors

--
1.8.3.1

Gris Ge

2014-11-24 10:04:52 UTC

Permalink

* Change simulator plugin error handling in access_group_create() to follow
the definition of API design:
1. Raise NAME_CONFLICT if requested access group name is used by other
access group on access_group_create().
2. Raise EXISTS_INITIATOR if requested initiator is used by other
access group on access_group_create().

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/sim/simarray.py | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/plugin/sim/simarray.py b/plugin/sim/simarray.py
index d733e50..c31cd3e 100644
--- a/plugin/sim/simarray.py
+++ b/plugin/sim/simarray.py
@@ -1142,24 +1142,9 @@ def access_group_create(self, name, init_id, init_type, sys_id, flags=0):

exist_sim_ag = self._sim_ag_of_init(init_id)
if exist_sim_ag:
- if exist_sim_ag['name'] == name:
- return exist_sim_ag
- else:
- raise LsmError(ErrorNumber.EXISTS_INITIATOR,
- "Initiator %s already exist in other " %
- init_id + "access group %s(%s)" %
- (exist_sim_ag['name'], exist_sim_ag['ag_id']))
-
- exist_sim_ag = self._sim_ag_of_name(name)
- if exist_sim_ag:
- if init_id in exist_sim_ag['init_ids']:
- return exist_sim_ag
- else:
- raise LsmError(ErrorNumber.NAME_CONFLICT,
- "Another access group %s(%s) is using " %
- (exist_sim_ag['name'], exist_sim_ag['ag_id']) +
- "requested name %s but not contain init_id %s" %
- (exist_sim_ag['name'], init_id))
+ raise LsmError(
+ ErrorNumber.EXISTS_INITIATOR,
+ "Requested initiator is used by other access group")

sim_ag = dict()
sim_ag['init_ids'] = [init_id]

--
1.8.3.1

Gris Ge

2014-11-24 10:04:51 UTC

Permalink

* Raise EXISTS_INITIATOR error if requested initiator is used by other
access group on access_group_create().

* Due to some unknown bug of NetApp, their array sometimes does not
raise error on initiator confliction. Hence we cannot use
'try then expect' way.
(Found problem on real hardware 8.0.2 and simulator 8.1.1)

* When plugin failed to find out the newly created access group,
previous code will raise NOT_FOUND_ACCESS_GROUP.
This patch changed it to PLUGIN_BUG, as NOT_FOUND_ACCESS_GROUP is only
for user input parameter.

* Tested on NetApp ONTAP simulator 8.1.1.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/ontap/ontap.py | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index ad7e3b3..f51c373 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -816,11 +816,23 @@ def access_group_create(self, name, init_id, init_type, system,
if self.sys_info.id != system.id:
raise LsmError(ErrorNumber.NOT_FOUND_SYSTEM,
"System %s not found" % system.id)
- cur_groups = self.access_groups()
- for cg in cur_groups:
- if cg.name == name:
- raise LsmError(ErrorNumber.NAME_CONFLICT,
- "Access group with the same name exists!")
+
+ # NetApp sometimes(real hardware 8.0.2 and simulator 8.1.1) does not
+ # raise error for initiator conflict.
+ #
+ # Precheck for initiator conflict
+ cur_lsm_groups = self.access_groups()
+ for cur_lsm_group in cur_lsm_groups:
+ if cur_lsm_group.name == name:
+ raise LsmError(
+ ErrorNumber.NAME_CONFLICT,
+ "Requested access group name is already used by other "
+ "access group")
+ if init_id in cur_lsm_group.init_ids:
+ raise LsmError(
+ ErrorNumber.EXISTS_INITIATOR,
+ "Requested initiator is already used by other "
+ "access group")

if init_type == AccessGroup.INIT_TYPE_ISCSI_IQN:
self.f.igroup_create(name, 'iscsi')
@@ -838,7 +850,7 @@ def access_group_create(self, name, init_id, init_type, system,
if g.name == name:
return g

- raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ raise LsmError(ErrorNumber.PLUGIN_BUG,
"access_group_create(): Unable to find access group "
"%s just created!" % name)

--
1.8.3.1

Gris Ge

2014-11-24 10:04:53 UTC

Permalink

* Raise LSM_ERR_EXISTS_INITIATOR if requested initiator is already used
by other access group.

* Added new private method _find_dup_init() to check initiator confliction.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/simc/simc_lsmplugin.c | 77 ++++++++++++++++++++++++++++++++------------
1 file changed, 57 insertions(+), 20 deletions(-)

diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 403eb6e..a54aa5a 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -1015,6 +1015,38 @@ static int access_group_list(lsm_plugin_ptr c,
return rc;
}

+static int _find_dup_init(struct plugin_data *pd, const char *initiator_id)
+{
+ GList *all_aags = g_hash_table_get_values(pd->access_groups);
+ guint y;
+ int rc = 1;
+ for(y = 0; y < g_list_length(all_aags); ++y) {
+ struct allocated_ag *cur_aag =
+ (struct allocated_ag *) g_list_nth_data(all_aags, y);
+ if (cur_aag){
+ lsm_string_list *inits =
+ lsm_access_group_initiator_id_get(cur_aag->ag);
+ int i;
+ for(i = 0; i < lsm_string_list_size(inits); ++i) {
+ const char *cur_init_id = lsm_string_list_elem_get(
+ inits, i);
+ if(strcmp(initiator_id, cur_init_id) == 0 ) {
+ rc = 0;
+ break;
+ }
+ }
+ lsm_string_list_free(inits);
+ if (rc == 0){
+ break;
+ }else{
+ cur_aag = (struct allocated_ag *) g_list_next(all_aags);
+ }
+ }
+ }
+ g_list_free(all_aags);
+ return rc;
+}
+
static int access_group_create(lsm_plugin_ptr c,
const char *name,
const char *initiator_id,
@@ -1033,30 +1065,35 @@ static int access_group_create(lsm_plugin_ptr c,
g_hash_table_lookup(pd->access_groups, id);

if( !find ) {
- lsm_string_list *initiators = lsm_string_list_alloc(1);
- if( initiators && id &&
- (LSM_ERR_OK == lsm_string_list_elem_set(initiators, 0, initiator_id))) {
- ag = lsm_access_group_record_alloc(id, name, initiators, id_type,
- lsm_system_id_get(system),
- NULL);
- aag = alloc_allocated_ag(ag, id_type);
- if( ag && aag ) {
- g_hash_table_insert(pd->access_groups, (gpointer)id,
- (gpointer)aag);
- *access_group = lsm_access_group_record_copy(ag);
+ // check initiator conflict
+ if ( _find_dup_init(pd, initiator_id) == 0 ){
+ rc = lsm_log_error_basic(
+ c, LSM_ERR_EXISTS_INITIATOR,
+ "Requested initiator is used by other access group");
+ }else{
+ lsm_string_list *initiators = lsm_string_list_alloc(1);
+ if( initiators && id &&
+ (LSM_ERR_OK ==
+ lsm_string_list_elem_set(initiators, 0, initiator_id))) {
+ ag = lsm_access_group_record_alloc(
+ id, name, initiators, id_type, lsm_system_id_get(system),
+ NULL);
+ aag = alloc_allocated_ag(ag, id_type);
+ if( ag && aag ) {
+ g_hash_table_insert(pd->access_groups, (gpointer)id,
+ (gpointer)aag);
+ *access_group = lsm_access_group_record_copy(ag);
+ } else {
+ free_allocated_ag(aag);
+ lsm_access_group_record_free(ag);
+ rc = lsm_log_error_basic(c, LSM_ERR_NO_MEMORY, "ENOMEM");
+ }
} else {
- free_allocated_ag(aag);
- lsm_access_group_record_free(ag);
rc = lsm_log_error_basic(c, LSM_ERR_NO_MEMORY, "ENOMEM");
}
- } else {
- rc = lsm_log_error_basic(c, LSM_ERR_NO_MEMORY,
- "ENOMEM");
+ /* Initiators is copied when allocating a group record */
+ lsm_string_list_free(initiators);
}
-
- /* Initiators is copied when allocating a group record */
- lsm_string_list_free(initiators);
-
} else {
rc = lsm_log_error_basic(c, LSM_ERR_NAME_CONFLICT,
"access group with same id found");

--
1.8.3.1

Gris Ge

2014-11-24 10:04:54 UTC

Permalink

* Add new tests for access_group_create():
1. Check NAME_CONFLICT for access group name confliction.
2. Check EXISTS_INITIATOR for initiator confliction.

Signed-off-by: Gris Ge <***@redhat.com>
---
test/plugin_test.py | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 7c1c964..838101d 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -776,6 +776,36 @@ def _delete_access_group(self, ag):
"deleted to not show up in the "
"access group list!")

+ def _test_ag_create_dup(self, lsm_ag, lsm_system):
+ """
+ Test NAME_CONFLICT and EXISTS_INITIATOR of access_group_create().
+ """
+ flag_got_expected_error = False
+ new_init_id = None
+ if lsm_ag.init_type == lsm.AccessGroup.INIT_TYPE_ISCSI_IQN:
+ new_init_id = 'iqn.1994-05.com.domain:01.' + rs(None, 6)
+ else:
+ new_init_id = r_fcpn()
+ try:
+ self.c.access_group_create(
+ lsm_ag.name, new_init_id, lsm_ag.init_type, lsm_system)
+ except LsmError as lsm_error:
+ self.assertTrue(lsm_error.code == ErrorNumber.NAME_CONFLICT)
+ flag_got_expected_error = True
+
+ self.assertTrue(flag_got_expected_error)
+
+ flag_got_expected_error = False
+ try:
+ self.c.access_group_create(
+ rs('ag'), lsm_ag.init_ids[0], lsm_ag.init_type, lsm_system)
+ except LsmError as lsm_error:
+ self.assertTrue(lsm_error.code == ErrorNumber.EXISTS_INITIATOR)
+ flag_got_expected_error = True
+
+ self.assertTrue(flag_got_expected_error)
+
+
def _test_ag_create_delete(self, cap, s):
ag = None
if supported(cap, [lsm.Capabilities.ACCESS_GROUPS,
@@ -784,6 +814,7 @@ def _test_ag_create_delete(self, cap, s):
cap, rs('ag'), s, lsm.AccessGroup.INIT_TYPE_ISCSI_IQN)
if ag is not None and \
supported(cap, [lsm.Capabilities.ACCESS_GROUP_DELETE]):
+ self._test_ag_create_dup(ag, s)
self._delete_access_group(ag)

if supported(cap, [lsm.Capabilities.ACCESS_GROUPS,
@@ -792,6 +823,7 @@ def _test_ag_create_delete(self, cap, s):
cap, rs('ag'), s, lsm.AccessGroup.INIT_TYPE_WWPN)
if ag is not None and \
supported(cap, [lsm.Capabilities.ACCESS_GROUP_DELETE]):
+ self._test_ag_create_dup(ag, s)
self._delete_access_group(ag)

def test_access_group_create_delete(self):

--
1.8.3.1

Tony Asleson

2014-11-24 18:48:41 UTC

Permalink

Hi Gris,

`make check` is failing with this patch set:

98%: Checks: 52, Failures: 1, Errors: 0
tester.c:2508:F:Basic:test_search_access_groups:0:
call:lsm_access_group_list rc = 152 Error msg= UNA - exception (null) -
debug (null) (which 1)

Please take a look.

*Hint*
Look at header documentation for lsm_access_group_initiator_id_get, you
shouldn't be calling lsm_string_list_free on the return value.

Thanks,
Tony

Post by Gris Ge
[PATCH 0/6] Raise NO_STATE_CHANGE error in volume mask and unmask.
* This patch set ensure all plugins in current code tree follow the
1. Raise NAME_CONFLICT if requested access group is already used by
other access group.
2. Raise EXISTS_INITIATOR if requested initiator ID is already used
by other access group.
* Plugin test has been updated to include test for these two errors.
SMI-S Plugin: Raise error for duplicate call of access_group_create()
ONTAP Plugin: Improve error handling in access_group_create()
Simulator Plugin: Fix incorrect error handling in
access_group_create()
Simulator C Plugin: Raise EXISTS_INITIATOR in access_group_create()
Plugin Test: Add NAME_CONFLICT and EXISTS_INITIATOR tests of
access_group_create()
plugin/ontap/ontap.py | 24 ++++++++++----
plugin/sim/simarray.py | 21 ++----------
plugin/simc/simc_lsmplugin.c | 77 ++++++++++++++++++++++++++++++++------------
plugin/smispy/smis.py | 28 ++++------------
test/plugin_test.py | 32 ++++++++++++++++++
5 files changed, 117 insertions(+), 65 deletions(-)

Gris Ge

2014-11-25 08:49:23 UTC

Permalink

* This patch set is based on:
[PATCH 0/6] Raise NO_STATE_CHANGE error in volume mask and unmask.

* This patch set ensure all plugins in current code tree follow the
API error definition of access_group_create():
1. Raise NAME_CONFLICT if requested access group is already used by
other access group.
2. Raise EXISTS_INITIATOR if requested initiator ID is already used
by other access group.

* Plugin test has been updated to include test for these two errors.

Changes in V2:

* Fix simulator C plugin for a incorrect memory free in access_group_create()
introduced by V1 patch. Please check patch 4/5 for detail.

Gris Ge (5):
SMI-S Plugin: Raise error for duplicate call of access_group_create()
ONTAP Plugin: Improve error handling in access_group_create()
Simulator Plugin: Fix incorrect error handling in
access_group_create()
Simulator C Plugin: Raise EXISTS_INITIATOR in access_group_create()
Plugin Test: Add NAME_CONFLICT and EXISTS_INITIATOR tests of
access_group_create()

plugin/ontap/ontap.py | 24 ++++++++++----
plugin/sim/simarray.py | 21 ++----------
plugin/simc/simc_lsmplugin.c | 76 ++++++++++++++++++++++++++++++++------------
plugin/smispy/smis.py | 28 ++++------------
test/plugin_test.py | 32 +++++++++++++++++++
5 files changed, 116 insertions(+), 65 deletions(-)

--
1.8.3.1

Gris Ge

2014-11-25 08:49:24 UTC

Permalink

* Raise EXISTS_INITIATOR when requested initiator is already used by other
access group on access_group_create().

* Tested on EMC VMAX for EXISTS_INITIATOR error.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 77b1e8a..d22a643 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -1959,42 +1959,28 @@ def access_group_create(self, name, init_id, init_type, system,
except (LsmError, CIMError):
# Check possible failure
# 1. Initiator already exist in other group.
- # If that group hold the same name as requested.
- # We consider as a duplicate call, return the exist one.
- exist_cim_init_mgs = self._c.Associators(
+ exist_cim_init_mg_paths = self._c.AssociatorNames(
cim_init_path,
AssocClass='CIM_MemberOfCollection',
- ResultClass='CIM_InitiatorMaskingGroup',
- PropertyList=cim_init_mg_pros)
+ ResultClass='CIM_InitiatorMaskingGroup')

- if len(exist_cim_init_mgs) != 0:
- for exist_cim_init_mg in exist_cim_init_mgs:
- if exist_cim_init_mg['ElementName'] == name:
- return smis_ag.cim_init_mg_to_lsm_ag(
- self._c, exist_cim_init_mg, system.id)
-
- # Name does not match.
+ if len(exist_cim_init_mg_paths) != 0:
raise LsmError(ErrorNumber.EXISTS_INITIATOR,
"Initiator %s " % org_init_id +
- "already exist in other access group "
- "with name %s and ID: %s" %
- (exist_cim_init_mgs[0]['ElementName'],
- md5(exist_cim_init_mgs[0]['InstanceID'])))
+ "already exist in other access group")
+
# 2. Requested name used by other group.
- # Since 1) already checked whether any group containing
- # requested init_id, now, it's surly a conflict.
exist_cim_init_mgs = self._cim_init_mg_of(
system.id, property_list=['ElementName'])
for exist_cim_init_mg in exist_cim_init_mgs:
if exist_cim_init_mg['ElementName'] == name:
raise LsmError(ErrorNumber.NAME_CONFLICT,
"Requested name %s is used by " % name +
- "another access group, but not containing "
- "requested initiator %s" % org_init_id)
+ "another access group")
raise

cim_init_mg = self._c.GetInstance(
- cim_init_mg_path, PropertyList=cim_init_mg_pros, LocalOnly=False)
+ cim_init_mg_path, PropertyList=cim_init_mg_pros)
return smis_ag.cim_init_mg_to_lsm_ag(self._c, cim_init_mg, system.id)

@handle_cim_errors

--
1.8.3.1

Gris Ge

2014-11-25 08:49:25 UTC

Permalink

--
1.8.3.1

Gris Ge

2014-11-25 08:49:26 UTC

Permalink

--
1.8.3.1

Gris Ge

2014-11-25 08:49:27 UTC

Permalink

* Raise LSM_ERR_EXISTS_INITIATOR if requested initiator is already used
by other access group.

* Added new private method _find_dup_init() to check initiator confliction.

Changes in V2:
* Remove incorrect lsm_string_list_free() in _find_dup_init().

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/simc/simc_lsmplugin.c | 76 ++++++++++++++++++++++++++++++++------------
1 file changed, 56 insertions(+), 20 deletions(-)

diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 403eb6e..289f5e7 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -1015,6 +1015,37 @@ static int access_group_list(lsm_plugin_ptr c,
return rc;
}

+static int _find_dup_init(struct plugin_data *pd, const char *initiator_id)
+{
+ GList *all_aags = g_hash_table_get_values(pd->access_groups);
+ guint y;
+ int rc = 1;
+ for(y = 0; y < g_list_length(all_aags); ++y) {
+ struct allocated_ag *cur_aag =
+ (struct allocated_ag *) g_list_nth_data(all_aags, y);
+ if (cur_aag){
+ lsm_string_list *inits =
+ lsm_access_group_initiator_id_get(cur_aag->ag);
+ int i;
+ for(i = 0; i < lsm_string_list_size(inits); ++i) {
+ const char *cur_init_id = lsm_string_list_elem_get(
+ inits, i);
+ if(strcmp(initiator_id, cur_init_id) == 0 ) {
+ rc = 0;
+ break;
+ }
+ }
+ if (rc == 0){
+ break;
+ }else{
+ cur_aag = (struct allocated_ag *) g_list_next(all_aags);
+ }
+ }
+ }
+ g_list_free(all_aags);
+ return rc;
+}
+
static int access_group_create(lsm_plugin_ptr c,
const char *name,
const char *initiator_id,
@@ -1033,30 +1064,35 @@ static int access_group_create(lsm_plugin_ptr c,
g_hash_table_lookup(pd->access_groups, id);

if( !find ) {
- lsm_string_list *initiators = lsm_string_list_alloc(1);
- if( initiators && id &&
- (LSM_ERR_OK == lsm_string_list_elem_set(initiators, 0, initiator_id))) {
- ag = lsm_access_group_record_alloc(id, name, initiators, id_type,
- lsm_system_id_get(system),
- NULL);
- aag = alloc_allocated_ag(ag, id_type);
- if( ag && aag ) {
- g_hash_table_insert(pd->access_groups, (gpointer)id,
- (gpointer)aag);
- *access_group = lsm_access_group_record_copy(ag);
+ // check initiator conflict
+ if ( _find_dup_init(pd, initiator_id) == 0 ){
+ rc = lsm_log_error_basic(
+ c, LSM_ERR_EXISTS_INITIATOR,
+ "Requested initiator is used by other access group");
+ }else{
+ lsm_string_list *initiators = lsm_string_list_alloc(1);
+ if( initiators && id &&
+ (LSM_ERR_OK ==
+ lsm_string_list_elem_set(initiators, 0, initiator_id))) {
+ ag = lsm_access_group_record_alloc(
+ id, name, initiators, id_type, lsm_system_id_get(system),
+ NULL);
+ aag = alloc_allocated_ag(ag, id_type);
+ if( ag && aag ) {
+ g_hash_table_insert(pd->access_groups, (gpointer)id,
+ (gpointer)aag);
+ *access_group = lsm_access_group_record_copy(ag);
+ } else {
+ free_allocated_ag(aag);
+ lsm_access_group_record_free(ag);
+ rc = lsm_log_error_basic(c, LSM_ERR_NO_MEMORY, "ENOMEM");
+ }
} else {
- free_allocated_ag(aag);
- lsm_access_group_record_free(ag);
rc = lsm_log_error_basic(c, LSM_ERR_NO_MEMORY, "ENOMEM");
}
- } else {
- rc = lsm_log_error_basic(c, LSM_ERR_NO_MEMORY,
- "ENOMEM");
+ /* Initiators is copied when allocating a group record */
+ lsm_string_list_free(initiators);
}
-
- /* Initiators is copied when allocating a group record */
- lsm_string_list_free(initiators);
-
} else {
rc = lsm_log_error_basic(c, LSM_ERR_NAME_CONFLICT,
"access group with same id found");

--
1.8.3.1

Gris Ge

2014-11-25 08:49:28 UTC

Permalink

--
1.8.3.1

Tony Asleson

2014-11-25 22:02:42 UTC

Permalink

Patch set committed!

Thanks!
-Tony

Post by Gris Ge
[PATCH 0/6] Raise NO_STATE_CHANGE error in volume mask and unmask.
* This patch set ensure all plugins in current code tree follow the
1. Raise NAME_CONFLICT if requested access group is already used by
other access group.
2. Raise EXISTS_INITIATOR if requested initiator ID is already used
by other access group.
* Plugin test has been updated to include test for these two errors.
* Fix simulator C plugin for a incorrect memory free in access_group_create()
introduced by V1 patch. Please check patch 4/5 for detail.
SMI-S Plugin: Raise error for duplicate call of access_group_create()
ONTAP Plugin: Improve error handling in access_group_create()
Simulator Plugin: Fix incorrect error handling in
access_group_create()
Simulator C Plugin: Raise EXISTS_INITIATOR in access_group_create()
Plugin Test: Add NAME_CONFLICT and EXISTS_INITIATOR tests of
access_group_create()
plugin/ontap/ontap.py | 24 ++++++++++----
plugin/sim/simarray.py | 21 ++----------
plugin/simc/simc_lsmplugin.c | 76 ++++++++++++++++++++++++++++++++------------
plugin/smispy/smis.py | 28 ++++------------
test/plugin_test.py | 32 +++++++++++++++++++
5 files changed, 116 insertions(+), 65 deletions(-)