[Libstoragemgmt-devel] [PATCH 1/4] Replace NO_MAPPING with NO_STATE_CHANGE

[Gris Ge]

* Remove lsm.ErrorNumber.NO_MAPPING and LSM_ERR_NO_MAPPING.
* Change volume_unmask() to use NO_STATE_CHANGE in stead of NO_MAPPING.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/include/libstoragemgmt/libstoragemgmt_error.h | 1 -
c_binding/lsm_plugin_ipc.cpp | 2 +-
plugin/nstor/nstor.py | 6 +++---
plugin/sim/simarray.py | 4 +++-
plugin/simc/simc_lsmplugin.c | 2 +-
python_binding/lsm/_common.py | 1 -
6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt_error.h b/c_binding/include/libstoragemgmt/libstoragemgmt_error.h
index fdb6ffa..d1c3cb7 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt_error.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt_error.h
@@ -62,7 +62,6 @@ typedef enum {
LSM_ERR_HOSTDOWN = 141, /**< Host unreachable on network */
LSM_ERR_NETWORK_ERROR = 142, /**< Generic network error */

- LSM_ERR_NO_MAPPING = 151, /**< There is no access for initiator and volume */
LSM_ERR_NO_MEMORY = 152, /**< Memory allocation failure */
LSM_ERR_NO_SUPPORT = 153, /**< Feature not supported */

diff --git a/c_binding/lsm_plugin_ipc.cpp b/c_binding/lsm_plugin_ipc.cpp
index fb002f9..34a8cdd 100644
--- a/c_binding/lsm_plugin_ipc.cpp
+++ b/c_binding/lsm_plugin_ipc.cpp
@@ -1477,7 +1477,7 @@ static int fs_delete(lsm_plugin_ptr p, Value &params, Value &response)
}
lsm_fs_record_free(fs);
} else {
- rc = LSM_ERR_NO_MAPPING;
+ rc = LSM_ERR_NOT_FOUND_FS;
}

} else {
diff --git a/plugin/nstor/nstor.py b/plugin/nstor/nstor.py
index b23a452..3188962 100644
--- a/plugin/nstor/nstor.py
+++ b/plugin/nstor/nstor.py
@@ -668,9 +668,9 @@ class NexentaStor(INfs, IStorageAreaNetwork):
if view['host_group'] == access_group.name:
view_number = view['entry_number']
if view_number == -1:
- raise LsmError(ErrorNumber.NO_MAPPING, "There is no such mapping "
- "for volume %s" %
- volume.name)
+ raise LsmError(ErrorNumber.NO_STATE_CHANGE,
+ "There is no such mapping for volume %s" %
+ volume.name)

self._request("remove_lun_mapping_entry", "scsidisk",
[volume.name, view_number])
diff --git a/plugin/sim/simarray.py b/plugin/sim/simarray.py
index 9c22546..3e0c755 100644
--- a/plugin/sim/simarray.py
+++ b/plugin/sim/simarray.py
@@ -1210,7 +1210,9 @@ class SimData(object):
raise LsmError(ErrorNumber.NOT_FOUND_VOLUME,
"No such Volume: %s" % vol_id)
if 'mask' not in self.vol_dict[vol_id].keys():
- return None
+ raise LsmError(ErrorNumber.NO_STATE_CHANGE, "Volume not "
+ "masked to access "
+ "group")

if ag_id not in self.vol_dict[vol_id]['mask'].keys():
raise LsmError(ErrorNumber.NO_STATE_CHANGE, "Volume not "
diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 39255c8..53fd94b 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -1220,7 +1220,7 @@ static int volume_unmask(lsm_plugin_ptr c,
lsm_volume *volume,
lsm_flag flags)
{
- int rc = LSM_ERR_NO_MAPPING;
+ int rc = LSM_ERR_NO_STATE_CHANGE;
struct plugin_data *pd = (struct plugin_data*)lsm_private_data_get(c);

struct allocated_ag *find = (struct allocated_ag *)
diff --git a/python_binding/lsm/_common.py b/python_binding/lsm/_common.py
index 6cc928b..4aedbd1 100644
--- a/python_binding/lsm/_common.py
+++ b/python_binding/lsm/_common.py
@@ -440,7 +440,6 @@ class ErrorNumber(object):
NETWORK_HOSTDOWN = 141 # Host unreachable on network
NETWORK_ERROR = 142 # Generic network error

- NO_MAPPING = 151
NO_MEMORY = 152
NO_SUPPORT = 153

[Gris Ge]

Add new error LSM_ERR_IS_MASKED:
1. lsm_volume_delete() when volume is masked to any access group.
2. lsm_access_group_delete() when access group has any volume masked.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/include/libstoragemgmt/libstoragemgmt_error.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt_error.h b/c_binding/include/libstoragemgmt/libstoragemgmt_error.h
index d1c3cb7..2bd0143 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt_error.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt_error.h
@@ -65,6 +65,8 @@ typedef enum {
LSM_ERR_NO_MEMORY = 152, /**< Memory allocation failure */
LSM_ERR_NO_SUPPORT = 153, /**< Feature not supported */

+ LSM_ERR_IS_MASKED = 160, /**< Volume masked to Access Group*/
+
LSM_ERR_NOT_FOUND_ACCESS_GROUP = 200, /**< Specified access group not found */
LSM_ERR_NOT_FOUND_FS = 201, /**< Specified FS not found */
LSM_ERR_NOT_FOUND_JOB = 202, /**< Specified JOB not found */

[Gris Ge]

Add new error lsm.ErrorNumber.IS_MASKED:
1. volume_delete() when volume is masked to any access group.
2. access_group_delete() when access group has any access group masked.

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

diff --git a/python_binding/lsm/_common.py b/python_binding/lsm/_common.py
index 4aedbd1..9281ec1 100644
--- a/python_binding/lsm/_common.py
+++ b/python_binding/lsm/_common.py
@@ -443,6 +443,9 @@ class ErrorNumber(object):
NO_MEMORY = 152
NO_SUPPORT = 153

+ # Deletion related errors
+ IS_MASKED = 160 # Volume is masked to access group.
+
NOT_FOUND_ACCESS_GROUP = 200
NOT_FOUND_FS = 201
NOT_FOUND_JOB = 202

[Gris Ge]

Updated these plugin to use new error IS_MASKED:
1. ontap
# I don't use handle_ontap_errors which use netapp error log directly.
# Which might cause confusing(LUN vs volume, NetApp volume vs LSM Volume)
2. sim
3. targetd
# New class TargetdError. _jsonrequest() will raise TargetdError
# instead of LsmError. We should directly use targetd error number
# as LsmError number even most of time they are the same.
# Anyway, in this case, targetd is use 303, IS_MASKED is 160.

Two plugins skipped for this change:
1. SMI-S
# Not worth my time when SMI-S has __no__ good way.
2. nstor
# Their API allowing deleting masked volume with no indication.
# they even raise no error when deleting non-exist volume.
# Meanwhile, I failed to find any API document to match current code.
# but it works, so I try not touch it.
3. simc
# Sorry, I am not an export of C.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/ontap/na.py | 3 +++
plugin/ontap/ontap.py | 26 ++++++++++++++++----------
plugin/sim/simarray.py | 11 ++++++++++-
plugin/targetd/targetd.py | 29 +++++++++++++++++++++++++----
4 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/plugin/ontap/na.py b/plugin/ontap/na.py
index 7803c65..16c3587 100644
--- a/plugin/ontap/na.py
+++ b/plugin/ontap/na.py
@@ -137,6 +137,9 @@ class FilerError(Exception):
IGROUP_NOT_CONTAIN_GIVEN_INIT = 9007
IGROUP_ALREADY_HAS_INIT = 9008
NO_SUCH_IGROUP = 9003
+ EVDISK_ERROR_VDISK_EXPORTED = 9013 # LUN is currently mapped
+ EVDISK_ERROR_INITGROUP_MAPS_EXIST = 9029 # LUN maps for this initiator
+ # group exist

def __init__(self, errno, reason, *args, **kwargs):
Exception.__init__(self, *args, **kwargs)
diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index 89b0e4d..f6caf62 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -519,16 +519,15 @@ class Ontap(IStorageAreaNetwork, INfs):

@handle_ontap_errors
def volume_delete(self, volume, flags=0):
- vol = Ontap._vol_to_na_volume_name(volume)
-
- luns = self.f.luns_get_specific(aggr=volume.pool_id,
- na_volume_name=vol)
-
- if len(luns) == 1 and na.Filer.LSM_VOL_PREFIX in vol:
- self.f.volume_delete(_lsm_vol_to_na_vol_path(volume))
- else:
+ try:
self.f.lun_delete(_lsm_vol_to_na_vol_path(volume))
-
+ except na.FilerError as f_error:
+ # We don't use handle_ontap_errors which use netapp
+ # error message which is not suitable for LSM user.
+ if f_error.errno == na.FilerError.EVDISK_ERROR_VDISK_EXPORTED:
+ raise LsmError(ErrorNumber.IS_MASKED,
+ "Volume is masked to access group")
+ raise
return None

@staticmethod
@@ -658,7 +657,14 @@ class Ontap(IStorageAreaNetwork, INfs):

@handle_ontap_errors
def access_group_delete(self, access_group, flags=0):
- return self.f.igroup_delete(access_group.name)
+ try:
+ return self.f.igroup_delete(access_group.name)
+ except na.FilerError as f_error:
+ if f_error.errno == \
+ na.FilerError.EVDISK_ERROR_INITGROUP_MAPS_EXIST:
+ raise LsmError(ErrorNumber.IS_MASKED,
+ "Access Group has volume masked")
+ raise

@handle_ontap_errors
def access_group_initiator_add(self, access_group, init_id, init_type,
diff --git a/plugin/sim/simarray.py b/plugin/sim/simarray.py
index 3e0c755..143c11e 100644
--- a/plugin/sim/simarray.py
+++ b/plugin/sim/simarray.py
@@ -935,8 +935,12 @@ class SimData(object):

def volume_delete(self, vol_id, flags=0):
if vol_id in self.vol_dict.keys():
+ if 'mask' in self.vol_dict[vol_id].keys() and \
+ self.vol_dict[vol_id]['mask']:
+ raise LsmError(ErrorNumber.IS_MASKED,
+ "Volume is masked to access group")
del(self.vol_dict[vol_id])
- return
+ return None
raise LsmError(ErrorNumber.NOT_FOUND_VOLUME,
"No such volume: %s" % vol_id)

@@ -1148,6 +1152,11 @@ class SimData(object):
if ag_id not in self.ag_dict.keys():
raise LsmError(ErrorNumber.NOT_FOUND_ACCESS_GROUP,
"Access group not found")
+ # Check whether any volume masked to.
+ for sim_vol in self.vol_dict.values():
+ if 'mask' in sim_vol.keys() and ag_id in sim_vol['mask']:
+ raise LsmError(ErrorNumber.IS_MASKED,
+ "Access group is masked to volume")
del(self.ag_dict[ag_id])
return None

diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index 9f1cac1..d3af6be 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
@@ -39,12 +39,27 @@ def handle_errors(method):
def target_wrapper(*args, **kwargs):
try:
return method(*args, **kwargs)
+ except TargetdError as te:
+ raise LsmError(ErrorNumber.PLUGIN_BUG,
+ "Got error %d from targetd: %s"
+ % te.errno, te.reason)
+ except LsmError:
+ raise
except Exception as e:
common_urllib2_error_handler(e)

return target_wrapper


+class TargetdError(Exception):
+ VOLUME_MASKED = 303
+
+ def __init__(self, errno, reason, *args, **kwargs):
+ Exception.__init__(self, *args, **kwargs)
+ self.errno = int(errno)
+ self.reason = reason
+
+
class TargetdStorage(IStorageAreaNetwork, INfs):

def __init__(self):
@@ -299,8 +314,14 @@ class TargetdStorage(IStorageAreaNetwork, INfs):

@handle_errors
def volume_delete(self, volume, flags=0):
- self._jsonrequest("vol_destroy",
- dict(pool=volume.pool_id, name=volume.name))
+ try:
+ self._jsonrequest("vol_destroy",
+ dict(pool=volume.pool_id, name=volume.name))
+ except TargetdError as te:
+ if te.errno == TargetdError.VOLUME_MASKED:
+ raise LsmError(ErrorNumber.IS_MASKED,
+ "Volume is masked to access group")
+ raise

@handle_errors
def volume_replicate(self, pool, rep_type, volume_src, name, flags=0):
@@ -610,8 +631,8 @@ class TargetdStorage(IStorageAreaNetwork, INfs):
#error_text = "%s:%s" % (str(response['error']['code']),
# response['error'].get('message', ''))

- raise LsmError(abs(int(response['error']['code'])),
- response['error'].get('message', ''))
+ raise TargetdError(abs(int(response['error']['code'])),
+ response['error'].get('message', ''))
else: # +code is async execution id
#Async completion, polling for results
async_code = response['error']['code']

[Tony Asleson]

Review comments

- I edited & amended the first patch, see updated commit comment V2
- For the case of SMI-S and others we could use the methods
volumes_accessible_by_access_group & access_groups_granted_to_volume to
check the preconditions before deleting a volume or access group.
- Adding a unit test(s) when introducing a new error code(s) like this
would be greatly appreciated.

I committed amended patch series.

Thanks!

Regards,
Tony

Please check comments in patches.
Thank you for your time on reviewing this patch set.
'[PATCH 1/4] ontap.py: Remove the 'hidden' NetApp Volume (lun container)'
'Clean up system status constants'
Replace NO_MAPPING with NO_STATE_CHANGE
C library: Add new error: LSM_ERR_IS_MASKED
Python library: Add new error: lsm.ErrorNumber.IS_MASKED
Plugins: Handle lsm.ErrorNumber.IS_MASKED
.../include/libstoragemgmt/libstoragemgmt_error.h | 3 ++-
c_binding/lsm_plugin_ipc.cpp | 2 +-
plugin/nstor/nstor.py | 6 ++---
plugin/ontap/na.py | 3 +++
plugin/ontap/ontap.py | 26 +++++++++++--------
plugin/sim/simarray.py | 15 +++++++++--
plugin/simc/simc_lsmplugin.c | 2 +-
plugin/targetd/targetd.py | 29 +++++++++++++++++++---
python_binding/lsm/_common.py | 4 ++-
9 files changed, 67 insertions(+), 23 deletions(-)