[Libstoragemgmt-devel] [PATCH] C & PY API: access_group_initiator_[add|delete] V2

[Tony Asleson]

For the initiator add we use both initiator id (string) and an
enumerated initiator type.

For the initiator delete we use just initiator id.

This patch makes the delete match the add. I also added a
validation check for the initiator id in the delete.

V2: Update plugin_test.py

Signed-off-by: Tony Asleson <***@redhat.com>

plugin_test.py: Correct for signature change

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/include/libstoragemgmt/libstoragemgmt.h | 2 ++
.../libstoragemgmt/libstoragemgmt_plug_interface.h | 2 ++
c_binding/lsm_mgmt.cpp | 14 +++++++++++---
c_binding/lsm_plugin_ipc.cpp | 8 ++++++--
plugin/nstor/nstor.py | 3 ++-
plugin/ontap/ontap.py | 3 ++-
plugin/sim/simarray.py | 9 ++++++---
plugin/sim/simulator.py | 6 +++---
plugin/simc/simc_lsmplugin.c | 5 +++--
plugin/smispy/eseries.py | 3 ++-
plugin/smispy/smis.py | 3 ++-
python_binding/lsm/_client.py | 4 +++-
python_binding/lsm/_iplugin.py | 2 +-
test/plugin_test.py | 11 +++++++----
test/tester.c | 22 +++++-----------------
tools/lsmcli/cmdline.py | 3 ++-
16 files changed, 59 insertions(+), 41 deletions(-)

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt.h b/c_binding/include/libstoragemgmt/libstoragemgmt.h
index 1587f0d..e87a305 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt.h
@@ -474,6 +474,7 @@ extern "C" {
* @param[in] conn Valid connection @see lsm_connect_password
* @param[in] access_group Group to modify
* @param[in] initiator_id Initiator to delete from group
+ * @param[in] init_type Type of initiator, enumerated type
* @param[out] updated_access_group Updated access group
* @param[in] flags Reserved for future use, must be zero.
* @return[in] LSM_ERR_OK on success, else error reason.
@@ -481,6 +482,7 @@ extern "C" {
int LSM_DLL_EXPORT lsm_access_group_initiator_delete(lsm_connect *conn,
lsm_access_group *access_group,
const char *initiator_id,
+ lsm_access_group_init_type init_type,
lsm_access_group **updated_access_group,
lsm_flag flags);

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h b/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
index 1da1aa3..1fd1593 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
@@ -438,6 +438,7 @@ typedef int (*lsm_plug_access_group_initiator_add)(lsm_plugin_ptr c,
* @param[in] c Valid lsm plug-in pointer
* @param[in] access_group Group to remove initiator from
* @param[in] initiator_id Initiator to remove
+ * @param[in] id_type Initiator type
* @param[out] updated_access_group Updated access group
* @param[in] flags Reserved
* @return LSM_ERR_OK, else error reason
@@ -445,6 +446,7 @@ typedef int (*lsm_plug_access_group_initiator_add)(lsm_plugin_ptr c,
typedef int (*lsm_plug_access_group_initiator_delete)(lsm_plugin_ptr c,
lsm_access_group *access_group,
const char *initiator_id,
+ lsm_access_group_init_type id_type,
lsm_access_group **updated_access_group,
lsm_flag flags);

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 8b27b58..d48802d 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1341,20 +1341,28 @@ int lsm_access_group_initiator_add(lsm_connect *c,

int lsm_access_group_initiator_delete(lsm_connect *c,
lsm_access_group *access_group,
- const char* initiator_id,
+ const char* init_id,
+ lsm_access_group_init_type init_type,
lsm_access_group **updated_access_group,
lsm_flag flags)
{
CONN_SETUP(c);

- if( !LSM_IS_ACCESS_GROUP(access_group) || CHECK_STR(initiator_id) ||
+ if( !LSM_IS_ACCESS_GROUP(access_group) || CHECK_STR(init_id) ||
LSM_FLAG_UNUSED_CHECK(flags) || CHECK_RP(updated_access_group)) {
return LSM_ERR_INVALID_ARGUMENT;
}

+ Value id;
+
+ if( LSM_ERR_OK != verify_initiator_id(init_id, init_type, id) ) {
+ return LSM_ERR_INVALID_ARGUMENT;
+ }
+
std::map<std::string, Value> p;
p["access_group"] = access_group_to_value(access_group);
- p["init_id"] = Value(initiator_id);
+ p["init_id"] = id;
+ p["init_type"] = Value((int32_t)init_type);
p["flags"] = Value(flags);

Value parameters(p);
diff --git a/c_binding/lsm_plugin_ipc.cpp b/c_binding/lsm_plugin_ipc.cpp
index eef07db..fb002f9 100644
--- a/c_binding/lsm_plugin_ipc.cpp
+++ b/c_binding/lsm_plugin_ipc.cpp
@@ -1106,17 +1106,21 @@ static int ag_initiator_del(lsm_plugin_ptr p, Value &params, Value &response)

Value v_group = params["access_group"];
Value v_init_id = params["init_id"];
+ Value v_init_type = params["init_type"];

if( Value::object_t == v_group.valueType() &&
Value::string_t == v_init_id.valueType() &&
+ Value::numeric_t == v_init_type.valueType() &&
LSM_FLAG_EXPECTED_TYPE(params) ) {

lsm_access_group *ag = value_to_access_group(v_group);

if( ag ) {
lsm_access_group *updated_access_group = NULL;
- const char *init = v_init_id.asC_str();
- rc = p->san_ops->ag_del_initiator(p, ag, init,
+ const char *id = v_init_id.asC_str();
+ lsm_access_group_init_type id_type =
+ (lsm_access_group_init_type) v_init_type.asInt32_t();
+ rc = p->san_ops->ag_del_initiator(p, ag, id, id_type,
&updated_access_group,
LSM_FLAG_GET_VALUE(params));

diff --git a/plugin/nstor/nstor.py b/plugin/nstor/nstor.py
index b47b8b6..b23a452 100644
--- a/plugin/nstor/nstor.py
+++ b/plugin/nstor/nstor.py
@@ -764,7 +764,8 @@ class NexentaStor(INfs, IStorageAreaNetwork):
access_group.id)

@handle_nstor_errors
- def access_group_initiator_delete(self, access_group, init_id, flags=0):
+ def access_group_initiator_delete(self, access_group, init_id, init_type,
+ flags=0):
"""
Deletes an initiator from an access group
"""
diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index 88c3ddd..c30cc18 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -763,7 +763,8 @@ class Ontap(IStorageAreaNetwork, INfs):
return self._access_group(na_ags[0])

@handle_ontap_errors
- def access_group_initiator_delete(self, access_group, init_id, flags=0):
+ def access_group_initiator_delete(self, access_group, init_id, init_type,
+ flags=0):
try:
self.f.igroup_del_initiator(access_group.name, init_id)
except na.FilerError as oe:
diff --git a/plugin/sim/simarray.py b/plugin/sim/simarray.py
index d0d291c..d0519ea 100644
--- a/plugin/sim/simarray.py
+++ b/plugin/sim/simarray.py
@@ -393,8 +393,10 @@ class SimArray(object):
ag_id, init_id, init_type, flags)
return SimArray._sim_ag_2_lsm(sim_ag)

- def access_group_initiator_delete(self, ag_id, init_id, flags=0):
- sim_ag = self.data.access_group_initiator_delete(ag_id, init_id, flags)
+ def access_group_initiator_delete(self, ag_id, init_id, init_type,
+ flags=0):
+ sim_ag = self.data.access_group_initiator_delete(ag_id, init_id,
+ init_type, flags)
return SimArray._sim_ag_2_lsm(sim_ag)

def volume_mask(self, ag_id, vol_id, flags=0):
@@ -1148,7 +1150,8 @@ class SimData(object):
self.ag_dict[ag_id]['init_ids'].extend([init_id])
return self.ag_dict[ag_id]

- def access_group_initiator_delete(self, ag_id, init_id, flags=0):
+ def access_group_initiator_delete(self, ag_id, init_id, init_type,
+ flags=0):
if ag_id not in self.ag_dict.keys():
raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
"Access group not found: %s" % ag_id)
diff --git a/plugin/sim/simulator.py b/plugin/sim/simulator.py
index f84f6bd..46fc08b 100644
--- a/plugin/sim/simulator.py
+++ b/plugin/sim/simulator.py
@@ -16,7 +16,7 @@
# Author: tasleson
# Gris Ge <***@redhat.com>

-from lsm import (uri_parse, VERSION, Capabilities, Pool, INfs,
+from lsm import (uri_parse, VERSION, Capabilities, INfs,
IStorageAreaNetwork, search_property)

from simarray import SimArray
@@ -173,10 +173,10 @@ class SimPlugin(INfs, IStorageAreaNetwork):
access_group.id, init_id, init_type, flags)
return SimPlugin._sim_data_2_lsm(sim_ag)

- def access_group_initiator_delete(self, access_group, init_id,
+ def access_group_initiator_delete(self, access_group, init_id, init_type,
flags=0):
sim_ag = self.sim_array.access_group_initiator_delete(
- access_group.id, init_id, flags)
+ access_group.id, init_id, init_type, flags)
return SimPlugin._sim_data_2_lsm(sim_ag)

def volume_mask(self, access_group, volume, flags=0):
diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 12a6edd..62d70e5 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -1120,7 +1120,8 @@ static int access_group_initiator_add( lsm_plugin_ptr c,

static int access_group_initiator_delete( lsm_plugin_ptr c,
lsm_access_group *group,
- const char *init,
+ const char *initiator_id,
+ lsm_access_group_init_type id_type,
lsm_access_group **updated_access_group,
lsm_flag flags)
{
@@ -1136,7 +1137,7 @@ static int access_group_initiator_delete( lsm_plugin_ptr c,
lsm_string_list *inits = lsm_access_group_initiator_id_get(find->ag);

for(i = 0; i < lsm_string_list_size(inits); ++i) {
- if( strcmp(init, lsm_string_list_elem_get(inits, i)) == 0 ) {
+ if( strcmp(initiator_id, lsm_string_list_elem_get(inits, i)) == 0 ) {
lsm_string_list_delete(inits, i);
rc = LSM_ERR_OK;
break;
diff --git a/plugin/smispy/eseries.py b/plugin/smispy/eseries.py
index 2e4b828..c493a47 100644
--- a/plugin/smispy/eseries.py
+++ b/plugin/smispy/eseries.py
@@ -152,7 +152,8 @@ class ESeries(Smis):
pass

@handle_cim_errors
- def access_group_initiator_delete(self, access_group, init_id, flags=0):
+ def access_group_initiator_delete(self, access_group, init_id, init_type,
+ flags=0):
"""
When using HidePaths to remove initiator, the whole SPC will be
removed. Before we find a workaround for this, I would like to have
diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index cc8fa4c..4dcbacf 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -3102,7 +3102,8 @@ class Smis(IStorageAreaNetwork):
cim_init_mg, access_group.system_id)

@handle_cim_errors
- def access_group_initiator_delete(self, access_group, init_id, flags=0):
+ def access_group_initiator_delete(self, access_group, init_id, init_type,
+ flags=0):
init_id = _lsm_init_id_to_snia(init_id)
mask_type = self._mask_type(raise_error=True)

diff --git a/python_binding/lsm/_client.py b/python_binding/lsm/_client.py
index 149ca70..481b368 100644
--- a/python_binding/lsm/_client.py
+++ b/python_binding/lsm/_client.py
@@ -593,10 +593,12 @@ class Client(INetworkAttachedStorage):
# @param self The this pointer
# @param access_group The access group to remove initiator from
# @param init_id The initiator to remove from the group
+ # @param init_type Initiator id type (enumeration)
# @param flags Reserved for future use, must be zero.
# @returns None on success, throws LsmError on errors.
@_return_requires(AccessGroup)
- def access_group_initiator_delete(self, access_group, init_id, flags=0):
+ def access_group_initiator_delete(self, access_group, init_id, init_type,
+ flags=0):
"""
Deletes an initiator from an access group
"""
diff --git a/python_binding/lsm/_iplugin.py b/python_binding/lsm/_iplugin.py
index 5973df6..3188405 100644
--- a/python_binding/lsm/_iplugin.py
+++ b/python_binding/lsm/_iplugin.py
@@ -272,7 +272,7 @@ class IStorageAreaNetwork(IPlugin):
"""
raise LsmError(ErrorNumber.NO_SUPPORT, "Not supported")

- def access_group_initiator_delete(self, access_group, init_id,
+ def access_group_initiator_delete(self, access_group, init_id, init_type,
flags=0):
"""
Deletes an initiator from an access group, Raises LsmError on error
diff --git a/test/plugin_test.py b/test/plugin_test.py
index 83f1b3b..2b2f272 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -727,8 +727,8 @@ class TestPlugin(unittest.TestCase):
self.assertTrue(len(match) == 1)
return t_id

- def _ag_init_delete(self, ag, init_id):
- self.c.access_group_initiator_delete(ag, init_id)
+ def _ag_init_delete(self, ag, init_id, init_type):
+ self.c.access_group_initiator_delete(ag, init_id, init_type)
ag_after = self.c.access_groups('id', ag.id)[0]
match = [x for x in ag_after.init_ids if x == init_id]
self.assertTrue(len(match) == 0)
@@ -772,14 +772,17 @@ class TestPlugin(unittest.TestCase):
init_id = self._ag_init_add(ag)
if supported(cap, [lsm.Capabilities.
ACCESS_GROUP_INITIATOR_DELETE]):
- self._ag_init_delete(ag, init_id)
+ self._ag_init_delete(
+ ag, init_id, lsm.AccessGroup.INIT_TYPE_WWPN)

if supported(cap, [lsm.Capabilities.
ACCESS_GROUP_INITIATOR_ADD_ISCSI_IQN]):
init_id = self._ag_init_add(ag)
if supported(cap, [lsm.Capabilities.
ACCESS_GROUP_INITIATOR_DELETE]):
- self._ag_init_delete(ag, init_id)
+ self._ag_init_delete(
+ ag, init_id,
+ lsm.AccessGroup.INIT_TYPE_ISCSI_IQN)

if ag_to_delete is not None:
self._delete_access_group(ag_to_delete)
diff --git a/test/tester.c b/test/tester.c
index 31a4c61..0007952 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -688,7 +688,8 @@ START_TEST(test_access_groups)
lsm_string_list_elem_get(init_list, i));

G(rc, lsm_access_group_initiator_delete, c, groups[0],
- lsm_string_list_elem_get(init_list, i), &updated,
+ lsm_string_list_elem_get(init_list, i),
+ LSM_ACCESS_GROUP_INIT_TYPE_ISCSI_IQN, &updated,
LSM_FLAG_RSVD)

fail_unless(updated != NULL);
@@ -698,20 +699,6 @@ START_TEST(test_access_groups)
init_list = NULL;
}

- /* TODO Need to fix this
- for( i = 0; i < init_list_count; ++i ) {
- printf("Deleting initiator %s from group!\n", lsm_initiator_id_get(inits[i]));
- rc = lsm_access_group_initiator_delete(c, groups[0],
- lsm_initiator_id_get(inits[i]), LSM_FLAG_RSVD);
-
- if( LSM_ERR_JOB_STARTED == rc ) {
- wait_for_job_fs(c, &job);
- } else {
- fail_unless(LSM_ERR_OK == rc);
- }
- }
- */
-
if( group ) {
G(rc, lsm_access_group_record_free, group);
group = NULL;
@@ -1560,10 +1547,11 @@ START_TEST(test_invalid_input)
fail_unless(rc == LSM_ERR_INVALID_ARGUMENT, "rc = %d", rc);


- rc = lsm_access_group_initiator_delete(c, NULL, NULL, NULL, LSM_FLAG_RSVD);
+ rc = lsm_access_group_initiator_delete(c, NULL, NULL, 0, NULL, LSM_FLAG_RSVD);
fail_unless(rc == LSM_ERR_INVALID_ARGUMENT, "rc = %d", rc);

- rc = lsm_access_group_initiator_delete(c, ag, NULL, NULL, LSM_FLAG_RSVD);
+ rc = lsm_access_group_initiator_delete(c, ag, NULL,
+ LSM_ACCESS_GROUP_INIT_TYPE_OTHER, NULL, LSM_FLAG_RSVD);
fail_unless(rc == LSM_ERR_INVALID_ARGUMENT, "rc = %d", rc);


diff --git a/tools/lsmcli/cmdline.py b/tools/lsmcli/cmdline.py
index 50841f9..d520733 100644
--- a/tools/lsmcli/cmdline.py
+++ b/tools/lsmcli/cmdline.py
@@ -920,7 +920,8 @@ class CmdLine:
return self.c.access_group_initiator_add(lsm_ag, init_id,
init_type)
else:
- return self.c.access_group_initiator_delete(lsm_ag, init_id)
+ return self.c.access_group_initiator_delete(lsm_ag, init_id,
+ init_type)

## Adds an initiator from an access group
def access_group_add(self, args):