[Libstoragemgmt-devel] [PATCH 3/3] C API: Access group add/del initiator, return update

[Tony Asleson]

To be consistent we will return an updated access group
when we add or remove an initiator from it. This makes
it consistent with other methods, like re-sizing a volume.

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/include/libstoragemgmt/libstoragemgmt.h | 15 +++++---
.../libstoragemgmt/libstoragemgmt_plug_interface.h | 33 +++++++++--------
c_binding/lsm_mgmt.cpp | 39 +++++++++++++++-----
c_binding/lsm_plugin_ipc.cpp | 15 ++++++++
plugin/simc/simc_lsmplugin.c | 17 +++++++++
test/tester.c | 41 +++++++++++-----------
6 files changed, 111 insertions(+), 49 deletions(-)

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt.h b/c_binding/include/libstoragemgmt/libstoragemgmt.h
index 4569cdb..a49aafd 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt.h
@@ -545,26 +545,31 @@ extern "C" {
* @param[in] access_group Group to modify
* @param[in] init_id Initiator to add to group
* @param[in] init_type Type of initiator
+ * @param[out] updated_access_group Updated access group
* @param[in] flags Reserved for future use, must be zero.
* @return LSM_ERR_OK on success, else error reason.
*/
int LSM_DLL_EXPORT lsm_access_group_initiator_add(lsm_connect *conn,
lsm_access_group *access_group,
const char *init_id,
- lsm_initiator_type init_type, lsm_flag flags);
+ lsm_initiator_type init_type,
+ lsm_access_group **updated_access_group,
+ lsm_flag flags);

/**
* Removes an initiator from an access group.
* @param[in] conn Valid connection @see lsm_connect_password
- * @param[in] group Group to modify
+ * @param[in] access_group Group to modify
* @param[in] initiator_id Initiator to delete from group
+ * @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.
*/
int LSM_DLL_EXPORT lsm_access_group_initiator_delete(lsm_connect *conn,
- lsm_access_group *group,
- const char *initiator_id,
- lsm_flag flags);
+ lsm_access_group *access_group,
+ const char *initiator_id,
+ lsm_access_group **updated_access_group,
+ lsm_flag flags);

/**
* Grants access to a volume for the specified group
diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h b/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
index 9c206ee..789d89a 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt_plug_interface.h
@@ -498,30 +498,35 @@ typedef int (*lsm_plug_access_group_delete)(lsm_plugin_ptr c,

/**
* Add an initiator to an access group, callback function signature
- * @param[in] c Valid lsm plug-in pointer
- * @param[in] group Group to add initiator to
- * @param[in] initiator_id Initiator to add to group
- * @param[in] id_type Initiator type
- * @param[in] flags Reserved
+ * @param[in] c Valid lsm plug-in pointer
+ * @param[in] access_group Group to add initiator to
+ * @param[in] initiator_id Initiator to add to group
+ * @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
*/
typedef int (*lsm_plug_access_group_initiator_add)(lsm_plugin_ptr c,
- lsm_access_group *group,
+ lsm_access_group *access_group,
const char *initiator_id,
- lsm_initiator_type id_type, lsm_flag flags);
+ lsm_initiator_type id_type,
+ lsm_access_group **updated_access_group,
+ lsm_flag flags);

/**
* Remove an initiator from an access group, callback function signature
- * @param[in] c Valid lsm plug-in pointer
- * @param[in] group Group to remove initiator from
- * @param[in] initiator_id Initiator to remove
- * @param[in] flags Reserved
+ * @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[out] updated_access_group Updated access group
+ * @param[in] flags Reserved
* @return LSM_ERR_OK, else error reason
*/
typedef int (*lsm_plug_access_group_initiator_delete)(lsm_plugin_ptr c,
- lsm_access_group *group,
- const char *initiator_id,
- lsm_flag flags);
+ lsm_access_group *access_group,
+ const char *initiator_id,
+ lsm_access_group **updated_access_group,
+ lsm_flag flags);

/**
* Grants access to a volume for the specified access group, callback function signature
diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index af20a02..86f9789 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -1496,6 +1496,7 @@ int lsm_access_group_initiator_add(lsm_connect *c,
lsm_access_group *access_group,
const char *init_id,
lsm_initiator_type init_type,
+ lsm_access_group **updated_access_group,
lsm_flag flags)
{
CONN_SETUP(c);
@@ -1504,8 +1505,8 @@ int lsm_access_group_initiator_add(lsm_connect *c,
return LSM_ERR_INVALID_ACCESS_GROUP;
}

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

@@ -1518,31 +1519,51 @@ int lsm_access_group_initiator_add(lsm_connect *c,
Value parameters(p);
Value response;

- return rpc(c, "access_group_initiator_add", parameters, response);
+ int rc = rpc(c, "access_group_initiator_add", parameters, response);
+ if( LSM_ERR_OK == rc ) {
+ //We should be getting a value back.
+ if( Value::object_t == response.valueType() ) {
+ *updated_access_group = value_to_access_group(response);
+ }
+ }
+
+ return rc;
}

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

- if( !LSM_IS_ACCESS_GROUP(group)) {
+ if( !LSM_IS_ACCESS_GROUP(access_group)) {
return LSM_ERR_INVALID_ACCESS_GROUP;
}

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

std::map<std::string, Value> p;
- p["access_group"] = access_group_to_value(group);
+ p["access_group"] = access_group_to_value(access_group);
p["init_id"] = Value(initiator_id);
p["flags"] = Value(flags);

Value parameters(p);
Value response;

- return rpc(c, "access_group_initiator_delete", parameters, response);
+ int rc = rpc(c, "access_group_initiator_delete", parameters, response);
+ if( LSM_ERR_OK == rc ) {
+ //We should be getting a value back.
+ if( Value::object_t == response.valueType() ) {
+ *updated_access_group = value_to_access_group(response);
+ }
+ }
+
+ return rc;
}

int lsm_volume_mask(lsm_connect *c, lsm_access_group *access_group,
diff --git a/c_binding/lsm_plugin_ipc.cpp b/c_binding/lsm_plugin_ipc.cpp
index 5e10647..983648e 100644
--- a/c_binding/lsm_plugin_ipc.cpp
+++ b/c_binding/lsm_plugin_ipc.cpp
@@ -1227,13 +1227,20 @@ static int ag_initiator_add(lsm_plugin_ptr p, Value &params, Value &response)

lsm_access_group *ag = value_to_access_group(v_group);
if( ag ) {
+ lsm_access_group *updated_access_group = NULL;
const char *id = v_init_id.asC_str();
lsm_initiator_type id_type = (lsm_initiator_type)
v_init_type.asInt32_t();

rc = p->san_ops->ag_add_initiator(p, ag, id, id_type,
+ &updated_access_group,
LSM_FLAG_GET_VALUE(params));

+ if( LSM_ERR_OK == rc ) {
+ response = access_group_to_value(updated_access_group);
+ lsm_access_group_record_free(updated_access_group);
+ }
+
lsm_access_group_record_free(ag);
} else {
rc = LSM_ERR_NO_MEMORY;
@@ -1263,9 +1270,17 @@ static int ag_initiator_del(lsm_plugin_ptr p, Value &params, Value &response)
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,
+ &updated_access_group,
LSM_FLAG_GET_VALUE(params));
+
+ if( LSM_ERR_OK == rc ) {
+ response = access_group_to_value(updated_access_group);
+ lsm_access_group_record_free(updated_access_group);
+ }
+
lsm_access_group_record_free(ag);
} else {
rc = LSM_ERR_NO_MEMORY;
diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 08c5c54..f19708b 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -1229,6 +1229,7 @@ static int access_group_initiator_add( lsm_plugin_ptr c,
lsm_access_group *group,
const char *initiator_id,
lsm_initiator_type id_type,
+ lsm_access_group **updated_access_group,
lsm_flag flags)
{
int rc = LSM_ERR_OK;
@@ -1241,6 +1242,13 @@ static int access_group_initiator_add( lsm_plugin_ptr c,
if( find ) {
lsm_string_list *inits = lsm_access_group_initiator_id_get(find->ag);
rc = lsm_string_list_append(inits, initiator_id);
+
+ if( LSM_ERR_OK == rc ) {
+ *updated_access_group = lsm_access_group_record_copy(find->ag);
+ if( !*updated_access_group ) {
+ rc = LSM_ERR_NO_MEMORY;
+ }
+ }
} else {
rc = lsm_log_error_basic(c, LSM_ERR_NOT_FOUND_ACCESS_GROUP,
"access group not found");
@@ -1251,6 +1259,7 @@ 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,
+ lsm_access_group **updated_access_group,
lsm_flag flags)
{
int rc = LSM_ERR_INITIATOR_NOT_IN_ACCESS_GROUP;
@@ -1271,6 +1280,14 @@ static int access_group_initiator_delete( lsm_plugin_ptr c,
break;
}
}
+
+ if( LSM_ERR_OK == rc ) {
+ *updated_access_group = lsm_access_group_record_copy(find->ag);
+ if( !*updated_access_group ) {
+ rc = LSM_ERR_NO_MEMORY;
+ }
+ }
+
} else {
rc = lsm_log_error_basic(c, LSM_ERR_NOT_FOUND_ACCESS_GROUP,
"access group not found");
diff --git a/test/tester.c b/test/tester.c
index 526279c..59a2829 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -648,38 +648,37 @@ START_TEST(test_access_groups)
G(rc, lsm_access_group_record_array_free, groups, count);
groups = NULL;
count = 0;
-
char *job = NULL;
+ lsm_access_group *updated = NULL;

- rc = lsm_access_group_initiator_add(c, group, "iqn.1994-05.com.domain:01.89bd02", LSM_INITIATOR_ISCSI, LSM_FLAG_RSVD);
-
- if( LSM_ERR_JOB_STARTED == rc ) {
- wait_for_job(c, &job);
- } else {
- fail_unless(LSM_ERR_OK == rc, "Expected success on lsmAccessGroupInitiatorAdd %d %d", rc, which_plugin);
- }
+ rc = lsm_access_group_initiator_add(c, group, "iqn.1994-05.com.domain:01.89bd02", LSM_INITIATOR_ISCSI, &updated, LSM_FLAG_RSVD);
+ fail_unless(LSM_ERR_OK == rc, "Expected success on lsmAccessGroupInitiatorAdd %d %d", rc, which_plugin);

G(rc, lsm_access_group_list, c, NULL, NULL, &groups, &count, LSM_FLAG_RSVD);
fail_unless( 1 == count );

+ fail_unless( updated != NULL );
+ lsm_access_group_record_free(updated);
+ updated = NULL;
+
if( count ) {
init_list = lsm_access_group_initiator_id_get(groups[0]);
- fail_unless( lsm_string_list_size(init_list) == 2, "Expecting 2 initiators, current num = %d\n", lsm_string_list_size(init_list) );
+ fail_unless( lsm_string_list_size(init_list) == 2,
+ "Expecting 2 initiators, current num = %d\n",
+ lsm_string_list_size(init_list) );
for( i = 0; i < lsm_string_list_size(init_list); ++i) {
printf("%d = %s\n", i, lsm_string_list_elem_get(init_list, i));

-
printf("Deleting initiator %s from group!\n",
lsm_string_list_elem_get(init_list, i));
- rc = lsm_access_group_initiator_delete(
- c, groups[0], lsm_string_list_elem_get(init_list, i),
- LSM_FLAG_RSVD);

- if( LSM_ERR_JOB_STARTED == rc ) {
- wait_for_job_fs(c, &job);
- } else {
- fail_unless(LSM_ERR_OK == rc);
- }
+ G(rc, lsm_access_group_initiator_delete, c, groups[0],
+ lsm_string_list_elem_get(init_list, i), &updated,
+ LSM_FLAG_RSVD)
+
+ fail_unless(updated != NULL);
+ lsm_access_group_record_free(updated);
+ updated = NULL;
}
init_list = NULL;
}
@@ -1531,14 +1530,14 @@ START_TEST(test_invalid_input)
fail_unless(rc == LSM_ERR_INVALID_ACCESS_GROUP, "rc = %d", rc);

/* lsmAccessGroupInitiatorAdd */
- rc = lsm_access_group_initiator_add(c, NULL, NULL, 0, LSM_FLAG_RSVD);
+ rc = lsm_access_group_initiator_add(c, NULL, NULL, 0, NULL, LSM_FLAG_RSVD);
fail_unless(rc == LSM_ERR_INVALID_ACCESS_GROUP, "rc = %d", rc);


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

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

[Tony Asleson]

Signed-off-by: Tony Asleson <***@redhat.com>
---
c_binding/include/libstoragemgmt/libstoragemgmt.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt.h b/c_binding/include/libstoragemgmt/libstoragemgmt.h
index 53f6e7d..4569cdb 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt.h
@@ -62,7 +62,7 @@ extern "C" {
* @param[in] uri Uniform Resource Identifier (see URI documentation)
* @param[in] password Password for the storage array (optional, can be NULL)
* @param[out] conn The connection to use for all the other library calls
- * @param[in] timeout Time-out in milli-seconds, (initial value).
+ * @param[in] timeout Time-out in milliseconds, (initial value).
* @param[out] e Error data if connection failed.
* @param[in] flags Reserved for future use, must be zero.
* @return LSM_ERR_OK on success, else error code @see lsm_error_number
@@ -253,7 +253,7 @@ extern "C" {
* new pool will be created from SAS disks
* only.
* @param [out] pool Newly created pool
- * @param [out] job Job ID of aysnc.
+ * @param [out] job Job ID of async. operation
* @param [in] flags Reserved for future use, must be zero
* @return LSM_ERR_OK on success, LSM_ERR_JOB_STARTED if async.,
* else error code
@@ -280,7 +280,7 @@ extern "C" {
* @param [in] num_disks Number of disks in disks array
* @param [in] raid_type The RAID type for new pool
* @param [out] pool Newly created pool
- * @param [out] job Job ID of aysnc.
+ * @param [out] job Job ID of async. operation
* @param [in] flags Reserved for future use, must be zero
* @return LSM_ERR_OK on success, LSM_ERR_JOB_STARTED if async.,
* else error code
@@ -300,7 +300,7 @@ extern "C" {
* @param [in] pool The pool to create new pool from
* @param [in] size_bytes Desired size of new pool
* @param [out] created_pool Newly created pool
- * @param [out] job Job ID of aysnc.
+ * @param [out] job Job ID of async.
* @param [in] flags Reserved for future use, must be zero
* @return LSM_ERR_OK on success, LSM_ERR_JOB_STARTED if async.,
* else error code
@@ -365,7 +365,7 @@ extern "C" {
* @param[in] pool Valid pool @see lsm_pool (OPTIONAL, use NULL for plug-in choice)
* @param[in] volume_name Human recognizable name (not all arrays support)
* @param[in] size Size of new volume in bytes (actual size will
- * be based on array rounding to blocksize)
+ * be based on array rounding to block size)
* @param[in] provisioning Type of volume provisioning to use
* @param[out] new_volume Valid volume @see lsm_volume
* @param[out] job Indicates job id
@@ -381,9 +381,9 @@ extern "C" {
/**
* Resize an existing volume.
* @param[in] conn Valid connection @see lsm_connect_password
- * @param[in] volume volume to resize
- * @param[in] new_size New size of volume
- * @param[out] resized_volume Pointer to newly resized lun.
+ * @param[in] volume volume to re-size
+ * @param[in] new_size New size of volume
+ * @param[out] resized_volume Pointer to newly re-sized lun.
* @param[out] job Indicates job id
* @param[in] flags Reserved for future use, must be zero.
* @return LSM_ERR_OK on success, LSM_ERR_JOB_STARTED if async. , else error code

[Gris Ge]

* Return updated AccessGroup instance for access_group_initiator_add()
and access_group_initiator_delete()

Signed-off-by: Gris Ge <***@redhat.com>
---
python_binding/lsm/_client.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python_binding/lsm/_client.py b/python_binding/lsm/_client.py
index 8591285..7a8f4fd 100644
--- a/python_binding/lsm/_client.py
+++ b/python_binding/lsm/_client.py
@@ -673,7 +673,7 @@ class Client(INetworkAttachedStorage):
# @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(None)
+ @_return_requires(AccessGroup)
def access_group_initiator_add(self, access_group, init_id, init_type,
flags=0):
"""
@@ -687,7 +687,7 @@ class Client(INetworkAttachedStorage):
# @param init_id The initiator to remove from the group
# @param flags Reserved for future use, must be zero.
# @returns None on success, throws LsmError on errors.
- @_return_requires(None)
+ @_return_requires(AccessGroup)
def access_group_initiator_delete(self, access_group, init_id, flags=0):
"""
Deletes an initiator from an access group

[Gris Ge]

lsmcli: Display updated AccessGroup for 'access-group-add' and
'access-group-remove' command.

Signed-off-by: Gris Ge <***@redhat.com>
---
tools/lsmcli/cmdline.py | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/lsmcli/cmdline.py b/tools/lsmcli/cmdline.py
index b212848..160ed31 100644
--- a/tools/lsmcli/cmdline.py
+++ b/tools/lsmcli/cmdline.py
@@ -958,17 +958,18 @@ class CmdLine:

if op:
init_type = ag_init_type_str_to_lsm(args.init_type)
- self.c.access_group_initiator_add(lsm_ag, args.init, init_type)
+ return self.c.access_group_initiator_add(lsm_ag, args.init,
+ init_type)
else:
- self.c.access_group_initiator_delete(lsm_ag, args.init)
+ return self.c.access_group_initiator_delete(lsm_ag, args.init)

## Adds an initiator from an access group
def access_group_add(self, args):
- self._add_rm_access_grp_init(args, True)
+ self.display_data([self._add_rm_access_grp_init(args, True)])

## Removes an initiator from an access group
def access_group_remove(self, args):
- self._add_rm_access_grp_init(args, False)
+ self.display_data([self._add_rm_access_grp_init(args, False)])

def access_group_volumes(self, args):
agl = self.c.access_groups()

[Gris Ge]

* Updated these plugins for access_group_initiator_add() and
access_group_initiator_delete() return:
* nstor
* sim
* ontap

* Extra changes:
* ontap:
1. Use FilerError to ignore duplicate call of
access_group_initiator_add() and access_group_initiator_delete()
when initiator already added or removed.
2. Use FilerError to detect non-exist access group.
* nstor:
1. Check access group existence before making changes.
2. Check whether initiator already added or removed before making
changes.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/nstor/nstor.py | 35 +++++++++++++++++++++++++++++++++--
plugin/ontap/na.py | 7 ++++++-
plugin/ontap/ontap.py | 40 ++++++++++++++++++++++++++++++++++++++--
plugin/sim/simarray.py | 12 +++++++-----
plugin/sim/simulator.py | 3 ++-
5 files changed, 86 insertions(+), 11 deletions(-)

diff --git a/plugin/nstor/nstor.py b/plugin/nstor/nstor.py
index 7916b87..4c0a56b 100644
--- a/plugin/nstor/nstor.py
+++ b/plugin/nstor/nstor.py
@@ -745,16 +745,47 @@ class NexentaStor(INfs, IStorageAreaNetwork):
raise LsmError(ErrorNumber.NO_SUPPORT,
"Nstor only support iSCSI Access Group")

+ hg_list = self._request("list_hostgroups", "stmf", [])
+ if access_group.name not in hg_list:
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%s) not found" %
+ (access_group.name, access_group.id))
+
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ if init_id in init_ids:
+ # Already in requested group.
+ return access_group
+
self._add_initiator(access_group.name, init_id)
- return None
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ return AccessGroup(access_group.name, access_group.name,
+ init_ids, AccessGroup.INIT_TYPE_ISCSI_IQN,
+ access_group.id)

@handle_nstor_errors
def access_group_initiator_delete(self, access_group, init_id, flags=0):
"""
Deletes an initiator from an access group
"""
+ hg_list = self._request("list_hostgroups", "stmf", [])
+ if access_group.name not in hg_list:
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%s) not found" %
+ (access_group.name, access_group.id))
+
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ if init_id not in init_ids:
+ # Already removed from requested group.
+ return access_group
self._add_initiator(access_group.name, init_id, True)
- return None
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ return AccessGroup(access_group.name, access_group.name,
+ init_ids, AccessGroup.INIT_TYPE_ISCSI_IQN,
+ access_group.id)

@handle_nstor_errors
def volumes_accessible_by_access_group(self, access_group, flags=0):
diff --git a/plugin/ontap/na.py b/plugin/ontap/na.py
index fb9d36f..cd015a5 100644
--- a/plugin/ontap/na.py
+++ b/plugin/ontap/na.py
@@ -134,6 +134,9 @@ class FilerError(Exception):
"""
Class represents a NetApp bad return code
"""
+ IGROUP_NOT_CONTAIN_GIVEN_INIT = 9007
+ IGROUP_ALREADY_HAS_INIT = 9008
+ NO_SUCH_IGROUP = 9003

def __init__(self, errno, reason, *args, **kwargs):
Exception.__init__(self, *args, **kwargs)
@@ -521,7 +524,9 @@ class Filer(object):

def igroup_del_initiator(self, ig, initiator):
self._invoke('igroup-remove',
- {'initiator-group-name': ig, 'initiator': initiator})
+ {'initiator-group-name': ig,
+ 'initiator': initiator,
+ 'force': 'true'})

def lun_map(self, igroup, lun_path):
self._invoke('lun-map', {'initiator-group': igroup, 'path': lun_path})
diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index 40bfe86..aa776b8 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -738,11 +738,47 @@ class Ontap(IStorageAreaNetwork, INfs):
@handle_ontap_errors
def access_group_initiator_add(self, access_group, init_id, init_type,
flags=0):
- return self.f.igroup_add_initiator(access_group.name, init_id)
+ try:
+ self.f.igroup_add_initiator(access_group.name, init_id)
+ except na.FilerError as oe:
+ if oe.errno == na.FilerError.IGROUP_ALREADY_HAS_INIT:
+ return access_group
+ elif oe.errno == na.FilerError.NO_SUCH_IGROUP:
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%d) not found" %
+ (access_group.name, access_group.id))
+ else:
+ raise
+ na_ags = self.f.igroups(access_group.name)
+ if len(na_ags) != 1:
+ raise LsmError(ErrorNumber.LSM_BUG,
+ "access_group_initiator_add(): Got unexpected"
+ "(not 1) count of na_ag: %s" % na_ags)
+
+ return self._access_group(na_ags[0])

@handle_ontap_errors
def access_group_initiator_delete(self, access_group, init_id, flags=0):
- return self.f.igroup_del_initiator(access_group.name, init_id)
+ try:
+ self.f.igroup_del_initiator(access_group.name, init_id)
+ except na.FilerError as oe:
+ error_code, error_msg = error_map(oe)
+ if oe.errno == na.FilerError.IGROUP_NOT_CONTAIN_GIVEN_INIT:
+ return access_group
+ elif oe.errno == na.FilerError.NO_SUCH_IGROUP:
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%d) not found" %
+ (access_group.name, access_group.id))
+ else:
+ raise
+ na_ags = self.f.igroups(access_group.name)
+ if len(na_ags) != 1:
+ raise LsmError(ErrorNumber.LSM_BUG,
+ "access_group_initiator_add(): Got unexpected"
+ "(not 1) count of na_ag: %s" % na_ags)
+
+ return self._access_group(na_ags[0])
+

@handle_ontap_errors
def volumes_accessible_by_access_group(self, access_group, flags=0):
diff --git a/plugin/sim/simarray.py b/plugin/sim/simarray.py
index 888858e..ad6f9fe 100644
--- a/plugin/sim/simarray.py
+++ b/plugin/sim/simarray.py
@@ -351,11 +351,13 @@ class SimArray(object):
return self.data.access_group_delete(ag_id, flags)

def access_group_initiator_add(self, ag_id, init_id, init_type, flags=0):
- return self.data.access_group_initiator_add(
+ sim_ag = self.data.access_group_initiator_add(
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):
- return self.data.access_group_initiator_delete(ag_id, init_id, flags)
+ sim_ag = self.data.access_group_initiator_delete(ag_id, init_id, flags)
+ return SimArray._sim_ag_2_lsm(sim_ag)

def volume_mask(self, ag_id, vol_id, flags=0):
return self.data.volume_mask(ag_id, vol_id, flags)
@@ -1079,12 +1081,12 @@ class SimData(object):
raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
"Access group not found")
if init_id in self.ag_dict[ag_id]['init_ids']:
- return None
+ return self.ag_dict[ag_id]

self._check_dup_init(init_id)

self.ag_dict[ag_id]['init_ids'].extend([init_id])
- return None
+ return self.ag_dict[ag_id]

def access_group_initiator_delete(self, ag_id, init_id, flags=0):
if ag_id not in self.ag_dict.keys():
@@ -1098,7 +1100,7 @@ class SimData(object):
new_init_ids.extend([cur_init_id])
del(self.ag_dict[ag_id]['init_ids'])
self.ag_dict[ag_id]['init_ids'] = new_init_ids
- return None
+ return self.ag_dict[ag_id]

def volume_mask(self, ag_id, vol_id, flags=0):
if ag_id not in self.ag_dict.keys():
diff --git a/plugin/sim/simulator.py b/plugin/sim/simulator.py
index fd866ce..bdcf24c 100644
--- a/plugin/sim/simulator.py
+++ b/plugin/sim/simulator.py
@@ -195,8 +195,9 @@ class SimPlugin(INfs, IStorageAreaNetwork):

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

def volume_mask(self, access_group, volume, flags=0):
return self.sim_array.volume_mask(

[Tony Asleson]

Patch committed.

Thanks,
Tony

Post by Gris Ge
* Updated these plugins for access_group_initiator_add() and
* nstor
* sim
* ontap
1. Use FilerError to ignore duplicate call of
access_group_initiator_add() and access_group_initiator_delete()
when initiator already added or removed.
2. Use FilerError to detect non-exist access group.
1. Check access group existence before making changes.
2. Check whether initiator already added or removed before making
changes.
---
plugin/nstor/nstor.py | 35 +++++++++++++++++++++++++++++++++--
plugin/ontap/na.py | 7 ++++++-
plugin/ontap/ontap.py | 40 ++++++++++++++++++++++++++++++++++++++--
plugin/sim/simarray.py | 12 +++++++-----
plugin/sim/simulator.py | 3 ++-
5 files changed, 86 insertions(+), 11 deletions(-)
diff --git a/plugin/nstor/nstor.py b/plugin/nstor/nstor.py
index 7916b87..4c0a56b 100644
--- a/plugin/nstor/nstor.py
+++ b/plugin/nstor/nstor.py
raise LsmError(ErrorNumber.NO_SUPPORT,
"Nstor only support iSCSI Access Group")
+ hg_list = self._request("list_hostgroups", "stmf", [])
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%s) not found" %
+ (access_group.name, access_group.id))
+
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ # Already in requested group.
+ return access_group
+
self._add_initiator(access_group.name, init_id)
- return None
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ return AccessGroup(access_group.name, access_group.name,
+ init_ids, AccessGroup.INIT_TYPE_ISCSI_IQN,
+ access_group.id)
@handle_nstor_errors
"""
Deletes an initiator from an access group
"""
+ hg_list = self._request("list_hostgroups", "stmf", [])
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%s) not found" %
+ (access_group.name, access_group.id))
+
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ # Already removed from requested group.
+ return access_group
self._add_initiator(access_group.name, init_id, True)
- return None
+ init_ids = self._request("list_hostgroup_members", "stmf",
+ [access_group.name])
+ return AccessGroup(access_group.name, access_group.name,
+ init_ids, AccessGroup.INIT_TYPE_ISCSI_IQN,
+ access_group.id)
@handle_nstor_errors
diff --git a/plugin/ontap/na.py b/plugin/ontap/na.py
index fb9d36f..cd015a5 100644
--- a/plugin/ontap/na.py
+++ b/plugin/ontap/na.py
"""
Class represents a NetApp bad return code
"""
+ IGROUP_NOT_CONTAIN_GIVEN_INIT = 9007
+ IGROUP_ALREADY_HAS_INIT = 9008
+ NO_SUCH_IGROUP = 9003
Exception.__init__(self, *args, **kwargs)
self._invoke('igroup-remove',
- {'initiator-group-name': ig, 'initiator': initiator})
+ {'initiator-group-name': ig,
+ 'initiator': initiator,
+ 'force': 'true'})
self._invoke('lun-map', {'initiator-group': igroup, 'path': lun_path})
diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index 40bfe86..aa776b8 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@handle_ontap_errors
def access_group_initiator_add(self, access_group, init_id, init_type,
- return self.f.igroup_add_initiator(access_group.name, init_id)
+ self.f.igroup_add_initiator(access_group.name, init_id)
+ return access_group
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%d) not found" %
+ (access_group.name, access_group.id))
+ raise
+ na_ags = self.f.igroups(access_group.name)
+ raise LsmError(ErrorNumber.LSM_BUG,
+ "access_group_initiator_add(): Got unexpected"
+ "(not 1) count of na_ag: %s" % na_ags)
+
+ return self._access_group(na_ags[0])
@handle_ontap_errors
- return self.f.igroup_del_initiator(access_group.name, init_id)
+ self.f.igroup_del_initiator(access_group.name, init_id)
+ error_code, error_msg = error_map(oe)
+ return access_group
+ raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
+ "AccessGroup %s(%d) not found" %
+ (access_group.name, access_group.id))
+ raise
+ na_ags = self.f.igroups(access_group.name)
+ raise LsmError(ErrorNumber.LSM_BUG,
+ "access_group_initiator_add(): Got unexpected"
+ "(not 1) count of na_ag: %s" % na_ags)
+
+ return self._access_group(na_ags[0])
+
@handle_ontap_errors
diff --git a/plugin/sim/simarray.py b/plugin/sim/simarray.py
index 888858e..ad6f9fe 100644
--- a/plugin/sim/simarray.py
+++ b/plugin/sim/simarray.py
return self.data.access_group_delete(ag_id, flags)
- return self.data.access_group_initiator_add(
+ sim_ag = self.data.access_group_initiator_add(
ag_id, init_id, init_type, flags)
+ return SimArray._sim_ag_2_lsm(sim_ag)
- return self.data.access_group_initiator_delete(ag_id, init_id, flags)
+ sim_ag = self.data.access_group_initiator_delete(ag_id, init_id, flags)
+ return SimArray._sim_ag_2_lsm(sim_ag)
return self.data.volume_mask(ag_id, vol_id, flags)
raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
"Access group not found")
- return None
+ return self.ag_dict[ag_id]
self._check_dup_init(init_id)
self.ag_dict[ag_id]['init_ids'].extend([init_id])
- return None
+ return self.ag_dict[ag_id]
new_init_ids.extend([cur_init_id])
del(self.ag_dict[ag_id]['init_ids'])
self.ag_dict[ag_id]['init_ids'] = new_init_ids
- return None
+ return self.ag_dict[ag_id]
diff --git a/plugin/sim/simulator.py b/plugin/sim/simulator.py
index fd866ce..bdcf24c 100644
--- a/plugin/sim/simulator.py
+++ b/plugin/sim/simulator.py
def access_group_initiator_delete(self, access_group, init_id,
- return self.sim_array.access_group_initiator_delete(
+ sim_ag = self.sim_array.access_group_initiator_delete(
access_group.id, init_id, flags)
+ return SimPlugin._sim_data_2_lsm(sim_ag)
return self.sim_array.volume_mask(

[Tony Asleson]

Patch committed.

Thanks,
Tony

Post by Gris Ge
lsmcli: Display updated AccessGroup for 'access-group-add' and
'access-group-remove' command.
---
tools/lsmcli/cmdline.py | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/lsmcli/cmdline.py b/tools/lsmcli/cmdline.py
index b212848..160ed31 100644
--- a/tools/lsmcli/cmdline.py
+++ b/tools/lsmcli/cmdline.py
init_type = ag_init_type_str_to_lsm(args.init_type)
- self.c.access_group_initiator_add(lsm_ag, args.init, init_type)
+ return self.c.access_group_initiator_add(lsm_ag, args.init,
+ init_type)
- self.c.access_group_initiator_delete(lsm_ag, args.init)
+ return self.c.access_group_initiator_delete(lsm_ag, args.init)
## Adds an initiator from an access group
- self._add_rm_access_grp_init(args, True)
+ self.display_data([self._add_rm_access_grp_init(args, True)])
## Removes an initiator from an access group
- self._add_rm_access_grp_init(args, False)
+ self.display_data([self._add_rm_access_grp_init(args, False)])
agl = self.c.access_groups()

[Tony Asleson]

Patch committed.

Thanks,
Tony

Post by Gris Ge
* Return updated AccessGroup instance for access_group_initiator_add()
and access_group_initiator_delete()
---
python_binding/lsm/_client.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/python_binding/lsm/_client.py b/python_binding/lsm/_client.py
index 8591285..7a8f4fd 100644
--- a/python_binding/lsm/_client.py
+++ b/python_binding/lsm/_client.py
def access_group_initiator_add(self, access_group, init_id, init_type,
"""
"""
Deletes an initiator from an access group