Discussion:
[Libstoragemgmt-devel] [PATCH 0/6] Raise NO_STATE_CHANGE error in volume mask and unmask.
Gris Ge
2014-11-22 07:50:48 UTC
Permalink
* When volume is already masked to requested access group in volume_mask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.

* When volume is not masked to requested access group in volume_unmask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.

* This patch set contains:
1. Plugins update for implementation mentioned above.
2. Plugin test update for conditions above.

* All patches are tested against their array for error conditions above.

Gris Ge (6):
SMI-S Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
Targetd Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
ONTAP Plugin: Raise NO_STATE_CHANGE error if already unmasked.
Nstor Plugin: Raise NO_STATE_CHANGE when already masked.
Simulator C Plugin: Raise LSM_ERR_NO_STATE_CHANGE when not masked.
Plugin Test: Add NO_STATE_CHANGE error detection for volume mask and
unmask.

plugin/nstor/nstor.py | 8 ++
plugin/ontap/na.py | 1 +
plugin/ontap/ontap.py | 13 +++-
plugin/simc/simc_lsmplugin.c | 10 ++-
plugin/smispy/smis.py | 170 +++++++++++++++++++++++++------------------
plugin/smispy/smis_ag.py | 21 ++++++
plugin/targetd/targetd.py | 25 +++++--
test/plugin_test.py | 21 ++++++
8 files changed, 186 insertions(+), 83 deletions(-)
--
1.8.3.1
Gris Ge
2014-11-22 07:50:49 UTC
Permalink
* Raise LsmError ErrorNumber.NO_STATE_CHANGE in
volume_mask and volume_unmask if already masked or not masked.

* Introduced new method:
smis_ag.cim_vols_masked_to_cim_spc_path()
# To get all masked cim_vols for give cim_spc.

* Tested on EMC VNX for 'Masking and Mapping' profile.

* Tested on EMC VMAX for 'Group Masking and Mapping' profile.

* Full plugin_test passed on:
* EMC VMAX
* EMC VNX
* HP 3PAR

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 170 +++++++++++++++++++++++++++--------------------
plugin/smispy/smis_ag.py | 21 ++++++
2 files changed, 119 insertions(+), 72 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index a46dfdd..e7789c6 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -807,8 +807,6 @@ def _volume_mask_group(self, access_group, volume, flags=0):
target ports(all FC and FCoE port for access_group.init_type == WWPN,
and the same to iSCSI)
"""
- cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)
-
cim_init_mg_path = smis_ag.lsm_ag_to_cim_init_mg_path(
self._c, access_group)

@@ -839,6 +837,9 @@ def _volume_mask_group(self, access_group, volume, flags=0):

if len(cim_spcs_path) == 0:
# We have to create the SPC and dev_mg now.
+ cim_sys = smis_sys.cim_sys_of_sys_id(
+ self._c, access_group.system_id)
+
cim_tgt_mg_path = self._cim_tgt_mg_path_create(
cim_sys.path, cim_gmms.path, access_group.name,
access_group.init_type)
@@ -853,18 +854,16 @@ def _volume_mask_group(self, access_group, volume, flags=0):
# many tgt_mg. It's seldom use, but possible.
for cim_spc_path in cim_spcs_path:
# Check whether already masked
- cim_vol_pros = smis_vol.cim_vol_id_pros()
- cim_vols = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros)
+ cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, smis_vol.cim_vol_id_pros())
for cur_cim_vol in cim_vols:
if smis_vol.vol_id_of_cim_vol(cur_cim_vol) == volume.id:
- # Masked.
- return None
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume already masked to requested access group")

- # spc one-one map to dev_mg is mandatory in 1.5r6
+ # SNIA require each cim_spc only have one cim_dev_mg
+ # associated
cim_dev_mg_path = self._c.AssociatorNames(
cim_spc_path,
AssocClass='CIM_AssociatedDeviceMaskingGroup',
@@ -903,6 +902,15 @@ def _volume_mask_old(self, access_group, volume, flags):
access_group.id +
"will not do volume_mask()")

+ # Pre-Check: Already masked
+ masked_cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, smis_vol.cim_vol_id_pros())
+ for masked_cim_vol in masked_cim_vols:
+ if smis_vol.vol_id_of_cim_vol(masked_cim_vol) == volume.id:
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume already masked to requested access group")
+
cim_ccs = self._c.cim_ccs_of_sys_id(volume.system_id)

cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
@@ -922,7 +930,6 @@ def _volume_unmask_group(self, access_group, volume):
If SupportedDeviceGroupFeatures does not allow empty
DeviceMaskingGroup in SPC, we remove SPC and DeviceMaskingGroup.
"""
- cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, volume.system_id)

cim_gmms_cap = self._c.Associators(
@@ -951,59 +958,74 @@ def _volume_unmask_group(self, access_group, volume):
"DeviceMaskingGroup in SPC. But target SMI-S provider "
"does not support any of these")

- cim_gmms = self._c.cim_gmms_of_sys_id(volume.system_id)
-
- cim_spcs_path = self._c.AssociatorNames(
+ cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
+ vol_cim_spcs_path = self._c.AssociatorNames(
cim_vol_path,
AssocClass='CIM_ProtocolControllerForUnit',
ResultClass='CIM_SCSIProtocolController')
- if len(cim_spcs_path) == 0:
+
+ if len(vol_cim_spcs_path) == 0:
# Already unmasked
- return None
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requestd access group")

- flag_init_mg_found = False
- cur_cim_init_mg = None
- # Searching for CIM_DeviceMaskingGroup
- for cim_spc_path in cim_spcs_path:
- cim_init_mgs = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_AssociatedInitiatorMaskingGroup',
- ResultClass='CIM_InitiatorMaskingGroup',
- PropertyList=['InstanceID'])
- for cim_init_mg in cim_init_mgs:
- if md5(cim_init_mg['InstanceID']) == access_group.id:
- flag_init_mg_found = True
+ cim_init_mg_path = smis_ag.lsm_ag_to_cim_init_mg_path(
+ self._c, access_group)
+ ag_cim_spcs_path = self._c.AssociatorNames(
+ cim_init_mg_path,
+ AssocClass='CIM_AssociatedInitiatorMaskingGroup',
+ ResultClass='CIM_SCSIProtocolController')
+
+ if len(ag_cim_spcs_path) == 0:
+ # Already unmasked
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+
+ found_cim_spc_path = None
+ for ag_cim_spc_path in ag_cim_spcs_path:
+ for vol_cim_spc_path in vol_cim_spcs_path:
+ if vol_cim_spc_path == ag_cim_spc_path:
+ found_cim_spc_path = vol_cim_spc_path
break

- if flag_init_mg_found:
- cim_dev_mgs_path = self._c.AssociatorNames(
- cim_spc_path,
- AssocClass='CIM_AssociatedDeviceMaskingGroup',
- ResultClass='CIM_DeviceMaskingGroup')
-
- for cim_dev_mg_path in cim_dev_mgs_path:
- if flag_empty_dev_in_spc is False:
- # We have to check whether this volume is the last
- # one in the DeviceMaskingGroup, if so, we have to
- # delete the SPC
- cur_cim_vols_path = self._c.AssociatorNames(
- cim_dev_mg_path,
- AssocClass='CIM_OrderedMemberOfCollection',
- ResultClass='CIM_StorageVolume')
- if len(cur_cim_vols_path) == 1:
- # Now, delete SPC
- in_params = {
- 'ProtocolController': cim_spc_path,
- }
- self._c.invoke_method_wait(
- 'DeleteMaskingView', cim_gmms.path, in_params)
-
- in_params = {
- 'MaskingGroup': cim_dev_mg_path,
- 'Members': [cim_vol_path],
- }
- self._c.invoke_method_wait(
- 'RemoveMembers', cim_gmms.path, in_params)
+ if found_cim_spc_path is None:
+ # Already unmasked
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+
+ # SNIA require each cim_spc only have one cim_dev_mg associated.
+ cim_dev_mg_path = self._c.AssociatorNames(
+ found_cim_spc_path,
+ AssocClass='CIM_AssociatedDeviceMaskingGroup',
+ ResultClass='CIM_DeviceMaskingGroup')[0]
+
+ cim_gmms = self._c.cim_gmms_of_sys_id(volume.system_id)
+
+ if flag_empty_dev_in_spc is False:
+ # We have to check whether this volume is the last
+ # one in the DeviceMaskingGroup, if so, we have to
+ # delete the SPC
+ cur_cim_vols_path = self._c.AssociatorNames(
+ cim_dev_mg_path,
+ AssocClass='CIM_OrderedMemberOfCollection',
+ ResultClass='CIM_StorageVolume')
+ if len(cur_cim_vols_path) == 1:
+ # last volume, should delete SPC
+ in_params = {
+ 'ProtocolController': found_cim_spc_path,
+ }
+ self._c.invoke_method_wait(
+ 'DeleteMaskingView', cim_gmms.path, in_params)
+
+ in_params = {
+ 'MaskingGroup': cim_dev_mg_path,
+ 'Members': [cim_vol_path],
+ }
+ self._c.invoke_method_wait(
+ 'RemoveMembers', cim_gmms.path, in_params)

return None

@@ -1022,12 +1044,22 @@ def volume_unmask(self, access_group, volume, flags=0):

def _volume_unmask_old(self, access_group, volume):
cim_ccs = self._c.cim_ccs_of_sys_id(volume.system_id)
-
- cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
- cim_vol = self._c.GetInstance(cim_vol_path, PropertyList=['Name'])
-
cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(self._c, access_group)

+ # Pre-check: not masked
+ cim_vol = None
+ cim_vol_pros = merge_list(['Name'], smis_vol.cim_vol_id_pros())
+ masked_cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros)
+ for masked_cim_vol in masked_cim_vols:
+ if smis_vol.vol_id_of_cim_vol(masked_cim_vol) == volume.id:
+ cim_vol = masked_cim_vol
+ break
+ if cim_vol is None:
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+
hide_params = {'LUNames': [cim_vol['Name']],
'ProtocolControllers': [cim_spc_path]}

@@ -1114,19 +1146,13 @@ def volumes_accessible_by_access_group(self, access_group, flags=0):

for cim_spc_path in cim_spcs_path:
cim_vols.extend(
- self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros))
+ smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros))
else:
cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(
self._c, access_group)
- cim_vols = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros)
+ cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros)
rc = []
for cim_vol in cim_vols:
pool_id = smis_pool.pool_id_of_cim_vol(self._c, cim_vol.path)
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index 9a6d81b..5ac1d20 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -262,3 +262,24 @@ def cim_init_path_check_or_create(smis_common, system_id, init_id, init_type):
return smis_common.invoke_method_wait(
'CreateStorageHardwareID', cim_hwms.path, in_params,
out_key='HardwareID', expect_class='CIM_StorageHardwareID')
+
+
+def cim_vols_masked_to_cim_spc_path(smis_common, cim_spc_path,
+ property_list=None):
+ """
+ Use this association to find out masked volume for certain cim_spc:
+ CIM_SCSIProtocolController
+ |
+ | CIM_ProtocolControllerForUnit
+ v
+ CIM_StorageVolume
+ Return a list of CIMInstance
+ """
+ if property_list is None:
+ property_list = []
+
+ return smis_common.Associators(
+ cim_spc_path,
+ AssocClass='CIM_ProtocolControllerForUnit',
+ ResultClass='CIM_StorageVolume',
+ PropertyList=property_list)
--
1.8.3.1
Tony Asleson
2014-11-24 18:05:22 UTC
Permalink
Post by Gris Ge
* Raise LsmError ErrorNumber.NO_STATE_CHANGE in
volume_mask and volume_unmask if already masked or not masked.
smis_ag.cim_vols_masked_to_cim_spc_path()
# To get all masked cim_vols for give cim_spc.
* Tested on EMC VNX for 'Masking and Mapping' profile.
* Tested on EMC VMAX for 'Group Masking and Mapping' profile.
* EMC VMAX
* EMC VNX
* HP 3PAR
---
plugin/smispy/smis.py | 170 +++++++++++++++++++++++++++--------------------
plugin/smispy/smis_ag.py | 21 ++++++
2 files changed, 119 insertions(+), 72 deletions(-)
diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index a46dfdd..e7789c6 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
target ports(all FC and FCoE port for access_group.init_type == WWPN,
and the same to iSCSI)
"""
- cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)
-
cim_init_mg_path = smis_ag.lsm_ag_to_cim_init_mg_path(
self._c, access_group)
# We have to create the SPC and dev_mg now.
+ cim_sys = smis_sys.cim_sys_of_sys_id(
+ self._c, access_group.system_id)
+
cim_tgt_mg_path = self._cim_tgt_mg_path_create(
cim_sys.path, cim_gmms.path, access_group.name,
access_group.init_type)
# many tgt_mg. It's seldom use, but possible.
# Check whether already masked
- cim_vol_pros = smis_vol.cim_vol_id_pros()
- cim_vols = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros)
+ cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, smis_vol.cim_vol_id_pros())
- # Masked.
- return None
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume already masked to requested access group")
- # spc one-one map to dev_mg is mandatory in 1.5r6
+ # SNIA require each cim_spc only have one cim_dev_mg
+ # associated
cim_dev_mg_path = self._c.AssociatorNames(
cim_spc_path,
AssocClass='CIM_AssociatedDeviceMaskingGroup',
access_group.id +
"will not do volume_mask()")
+ # Pre-Check: Already masked
+ masked_cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, smis_vol.cim_vol_id_pros())
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume already masked to requested access group")
+
cim_ccs = self._c.cim_ccs_of_sys_id(volume.system_id)
cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
If SupportedDeviceGroupFeatures does not allow empty
DeviceMaskingGroup in SPC, we remove SPC and DeviceMaskingGroup.
"""
- cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, volume.system_id)
cim_gmms_cap = self._c.Associators(
"DeviceMaskingGroup in SPC. But target SMI-S provider "
"does not support any of these")
- cim_gmms = self._c.cim_gmms_of_sys_id(volume.system_id)
-
- cim_spcs_path = self._c.AssociatorNames(
+ cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
+ vol_cim_spcs_path = self._c.AssociatorNames(
cim_vol_path,
AssocClass='CIM_ProtocolControllerForUnit',
ResultClass='CIM_SCSIProtocolController')
+
# Already unmasked
- return None
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requestd access group")
Typo: requested
Post by Gris Ge
- flag_init_mg_found = False
- cur_cim_init_mg = None
- # Searching for CIM_DeviceMaskingGroup
- cim_init_mgs = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_AssociatedInitiatorMaskingGroup',
- ResultClass='CIM_InitiatorMaskingGroup',
- PropertyList=['InstanceID'])
- flag_init_mg_found = True
+ cim_init_mg_path = smis_ag.lsm_ag_to_cim_init_mg_path(
+ self._c, access_group)
+ ag_cim_spcs_path = self._c.AssociatorNames(
+ cim_init_mg_path,
+ AssocClass='CIM_AssociatedInitiatorMaskingGroup',
+ ResultClass='CIM_SCSIProtocolController')
+
+ # Already unmasked
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
The above 5 lines don't need to exist as the following for loop will not
execute if the path is empty which will result in the same error code
being raised.
Post by Gris Ge
+
+ found_cim_spc_path = None
+ found_cim_spc_path = vol_cim_spc_path
break
- cim_dev_mgs_path = self._c.AssociatorNames(
- cim_spc_path,
- AssocClass='CIM_AssociatedDeviceMaskingGroup',
- ResultClass='CIM_DeviceMaskingGroup')
-
- # We have to check whether this volume is the last
- # one in the DeviceMaskingGroup, if so, we have to
- # delete the SPC
- cur_cim_vols_path = self._c.AssociatorNames(
- cim_dev_mg_path,
- AssocClass='CIM_OrderedMemberOfCollection',
- ResultClass='CIM_StorageVolume')
- # Now, delete SPC
- in_params = {
- 'ProtocolController': cim_spc_path,
- }
- self._c.invoke_method_wait(
- 'DeleteMaskingView', cim_gmms.path, in_params)
-
- in_params = {
- 'MaskingGroup': cim_dev_mg_path,
- 'Members': [cim_vol_path],
- }
- self._c.invoke_method_wait(
- 'RemoveMembers', cim_gmms.path, in_params)
+ # Already unmasked
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+
+ # SNIA require each cim_spc only have one cim_dev_mg associated.
+ cim_dev_mg_path = self._c.AssociatorNames(
+ found_cim_spc_path,
+ AssocClass='CIM_AssociatedDeviceMaskingGroup',
+ ResultClass='CIM_DeviceMaskingGroup')[0]
+
+ cim_gmms = self._c.cim_gmms_of_sys_id(volume.system_id)
+
+ # We have to check whether this volume is the last
+ # one in the DeviceMaskingGroup, if so, we have to
+ # delete the SPC
+ cur_cim_vols_path = self._c.AssociatorNames(
+ cim_dev_mg_path,
+ AssocClass='CIM_OrderedMemberOfCollection',
+ ResultClass='CIM_StorageVolume')
+ # last volume, should delete SPC
+ in_params = {
+ 'ProtocolController': found_cim_spc_path,
+ }
+ self._c.invoke_method_wait(
+ 'DeleteMaskingView', cim_gmms.path, in_params)
+
+ in_params = {
+ 'MaskingGroup': cim_dev_mg_path,
+ 'Members': [cim_vol_path],
+ }
+ self._c.invoke_method_wait(
+ 'RemoveMembers', cim_gmms.path, in_params)
return None
cim_ccs = self._c.cim_ccs_of_sys_id(volume.system_id)
-
- cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
- cim_vol = self._c.GetInstance(cim_vol_path, PropertyList=['Name'])
-
cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(self._c, access_group)
+ # Pre-check: not masked
+ cim_vol = None
+ cim_vol_pros = merge_list(['Name'], smis_vol.cim_vol_id_pros())
+ masked_cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros)
+ cim_vol = masked_cim_vol
+ break
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+
Both _volume_unmask_old and _volume_mask_old use almost identical
pre-checks. We should be able to place this in a common function and
use in both places.
Post by Gris Ge
hide_params = {'LUNames': [cim_vol['Name']],
'ProtocolControllers': [cim_spc_path]}
cim_vols.extend(
- self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros))
+ smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros))
cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(
self._c, access_group)
- cim_vols = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros)
+ cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros)
rc = []
pool_id = smis_pool.pool_id_of_cim_vol(self._c, cim_vol.path)
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index 9a6d81b..5ac1d20 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
return smis_common.invoke_method_wait(
'CreateStorageHardwareID', cim_hwms.path, in_params,
out_key='HardwareID', expect_class='CIM_StorageHardwareID')
+
+
+def cim_vols_masked_to_cim_spc_path(smis_common, cim_spc_path,
+ """
+ CIM_SCSIProtocolController
+ |
+ | CIM_ProtocolControllerForUnit
+ v
+ CIM_StorageVolume
+ Return a list of CIMInstance
+ """
+ property_list = []
+
+ return smis_common.Associators(
+ cim_spc_path,
+ AssocClass='CIM_ProtocolControllerForUnit',
+ ResultClass='CIM_StorageVolume',
+ PropertyList=property_list)
Gris Ge
2014-11-22 07:50:50 UTC
Permalink
* Raise LsmError ErrorNumber.NO_STATE_CHANGE in
volume_mask and volume_unmask if already masked or not masked.

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

diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index b40aa4c..f060a20 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
@@ -259,11 +259,12 @@ def volume_mask(self, access_group, volume, flags=0):

ag_id = md5(access_group.init_ids[0])
vol_id = volume.id
- # Return when found already masked.
tgt_masks = self._mask_infos()
if list(x for x in tgt_masks
if x['vol_id'] == vol_id and x['ag_id'] == ag_id):
- return None
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is already masked to requested access group")

# find lowest unused lun ID
used_lun_ids = [x['lun_id'] for x in tgt_masks]
@@ -283,11 +284,21 @@ def volume_mask(self, access_group, volume, flags=0):

@handle_errors
def volume_unmask(self, volume, access_group, flags=0):
- self._jsonrequest("export_destroy",
- dict(pool=volume.pool_id,
- vol=volume.name,
- initiator_wwn=access_group.init_ids[0]))
- return None
+ # Pre-check if already unmasked
+ ag_id = md5(access_group.init_ids[0])
+ vol_id = volume.id
+ tgt_masks = self._mask_infos()
+ if list(x for x in tgt_masks
+ if x['vol_id'] == vol_id and x['ag_id'] == ag_id):
+ self._jsonrequest("export_destroy",
+ dict(pool=volume.pool_id,
+ vol=volume.name,
+ initiator_wwn=access_group.init_ids[0]))
+ return None
+
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")

@handle_errors
def volumes_accessible_by_access_group(self, access_group, flags=0):
--
1.8.3.1
Tony Asleson
2014-11-24 18:05:30 UTC
Permalink
Post by Gris Ge
* Raise LsmError ErrorNumber.NO_STATE_CHANGE in
volume_mask and volume_unmask if already masked or not masked.
---
plugin/targetd/targetd.py | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index b40aa4c..f060a20 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
ag_id = md5(access_group.init_ids[0])
vol_id = volume.id
- # Return when found already masked.
tgt_masks = self._mask_infos()
if list(x for x in tgt_masks
Please move this common code into a separate function and call in both
mask and unmask paths.

Thanks,
Tony
Gris Ge
2014-11-22 07:50:51 UTC
Permalink
* Raise NO_STATE_CHANGE if volume_unmask() against unmasked relationship.
* Check EVDISK_ERROR_NO_SUCH_LUNMAP filer error for this condition.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/ontap/na.py | 1 +
plugin/ontap/ontap.py | 13 +++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/plugin/ontap/na.py b/plugin/ontap/na.py
index 5db7005..8532e87 100644
--- a/plugin/ontap/na.py
+++ b/plugin/ontap/na.py
@@ -143,6 +143,7 @@ class FilerError(Exception):
EVDISK_ERROR_VDISK_EXPORTED = 9013 # LUN is currently mapped
EVDISK_ERROR_VDISK_NOT_ENABLED = 9014 # LUN is not online
EVDISK_ERROR_VDISK_NOT_DISABLED = 9015 # LUN is not offline
+ EVDISK_ERROR_NO_SUCH_LUNMAP = 9016 # LUN is already unmapped
EVDISK_ERROR_INITGROUP_MAPS_EXIST = 9029 # LUN maps for this initiator
# group exist
EVDISK_ERROR_SIZE_TOO_LARGE = 9034 # LUN size too large.
diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index 31e2366..ad7e3b3 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -764,14 +764,23 @@ def volume_mask(self, access_group, volume, flags=0):
if fe.errno == na.FilerError.EVDISK_ERROR_INITGROUP_HAS_VDISK:
raise LsmError(
ErrorNumber.NO_STATE_CHANGE,
- "Volume already masked to defined access group")
+ "Volume is already masked to requested access group")
else:
raise
return None

@handle_ontap_errors
def volume_unmask(self, access_group, volume, flags=0):
- self.f.lun_unmap(access_group.name, _lsm_vol_to_na_vol_path(volume))
+ try:
+ self.f.lun_unmap(
+ access_group.name, _lsm_vol_to_na_vol_path(volume))
+ except na.FilerError as filer_error:
+ if filer_error.errno == na.FilerError.EVDISK_ERROR_NO_SUCH_LUNMAP:
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+ else:
+ raise
return None

@staticmethod
--
1.8.3.1
Gris Ge
2014-11-22 07:50:52 UTC
Permalink
* Raise NO_STATE_CHANGE error when volume is already masked on
volume_mask().

* As nstor will not raise error on duplicate volume mask, we have to
do a pre-check via volumes_accessible_by_access_group() method.

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

diff --git a/plugin/nstor/nstor.py b/plugin/nstor/nstor.py
index efa8734..bc0e339 100644
--- a/plugin/nstor/nstor.py
+++ b/plugin/nstor/nstor.py
@@ -659,6 +659,14 @@ def volume_mask(self, access_group, volume, flags=0):
"""
Allows an access group to access a volume.
"""
+ # Pre-check for already masked.
+ if list(v.id for v in
+ self.volumes_accessible_by_access_group(access_group)
+ if v.id == volume.id):
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is already masked to requested access group")
+
self._volume_mask(access_group.name, volume.name)
return
--
1.8.3.1
Gris Ge
2014-11-22 07:50:53 UTC
Permalink
* Raise LSM_ERR_NO_STATE_CHANGE when volume is not masked to requested
access group on volume_unmask().

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

diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 8e08be6..403eb6e 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -1277,8 +1277,14 @@ static int volume_unmask(lsm_plugin_ptr c,
lsm_access_group_id_get(find->ag));

if( grants ) {
- g_hash_table_remove(grants, lsm_volume_id_get(volume));
- rc = LSM_ERR_OK;
+ char *vol_id = g_hash_table_lookup(
+ grants, lsm_volume_id_get(volume));
+ if( vol_id ) {
+ g_hash_table_remove(grants, lsm_volume_id_get(volume));
+ rc = LSM_ERR_OK;
+ }else{
+ rc = LSM_ERR_NO_STATE_CHANGE;
+ }
}

} else {
--
1.8.3.1
Gris Ge
2014-11-22 07:50:54 UTC
Permalink
* Plugin should raise NO_STATE_CHANGE when volume already masked on
volume_mask()
* Plugin should raise NO_STATE_CHANGE when volume is not masked on
volume_unmask()

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

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 2aa6c37..7c1c964 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -711,9 +711,30 @@ def test_mask_unmask(self):
if vol is not None and chose_ag is not None:
self.c.volume_mask(chose_ag, vol)
self._masking_state(cap, chose_ag, vol, True)
+
+ # Test duplicate call for NO_STATE_CHANGE error
+ flag_dup_error_found = False
+ try:
+ self.c.volume_mask(chose_ag, vol)
+ except LsmError as lsm_error:
+ self.assertTrue(
+ lsm_error.code == ErrorNumber.NO_STATE_CHANGE)
+ flag_dup_error_found = True
+ self.assertTrue(flag_dup_error_found == True)
+
self.c.volume_unmask(chose_ag, vol)
self._masking_state(cap, chose_ag, vol, False)

+ # Test duplicate call for NO_STATE_CHANGE error
+ flag_dup_error_found = False
+ try:
+ self.c.volume_unmask(chose_ag, vol)
+ except LsmError as lsm_error:
+ self.assertTrue(
+ lsm_error.code == ErrorNumber.NO_STATE_CHANGE)
+ flag_dup_error_found = True
+ self.assertTrue(flag_dup_error_found == True)
+
if vol:
self._volume_delete(vol)
--
1.8.3.1
Tony Asleson
2014-11-24 18:04:44 UTC
Permalink
Hi Gris,

This patch set looks good. Please see specific comments in the patch
replies for suggestions.

Thanks!
-Tony
Post by Gris Ge
* When volume is already masked to requested access group in volume_mask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.
* When volume is not masked to requested access group in volume_unmask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.
1. Plugins update for implementation mentioned above.
2. Plugin test update for conditions above.
* All patches are tested against their array for error conditions above.
SMI-S Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
Targetd Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
ONTAP Plugin: Raise NO_STATE_CHANGE error if already unmasked.
Nstor Plugin: Raise NO_STATE_CHANGE when already masked.
Simulator C Plugin: Raise LSM_ERR_NO_STATE_CHANGE when not masked.
Plugin Test: Add NO_STATE_CHANGE error detection for volume mask and
unmask.
plugin/nstor/nstor.py | 8 ++
plugin/ontap/na.py | 1 +
plugin/ontap/ontap.py | 13 +++-
plugin/simc/simc_lsmplugin.c | 10 ++-
plugin/smispy/smis.py | 170 +++++++++++++++++++++++++------------------
plugin/smispy/smis_ag.py | 21 ++++++
plugin/targetd/targetd.py | 25 +++++--
test/plugin_test.py | 21 ++++++
8 files changed, 186 insertions(+), 83 deletions(-)
Gris Ge
2014-11-25 07:31:44 UTC
Permalink
* When volume is already masked to requested access group in volume_mask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.

* When volume is not masked to requested access group in volume_unmask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.

* This patch set contains:
1. Plugins update for implementation mentioned above.
2. Plugin test update for conditions above.

* All patches are tested against their array for error conditions above.

Gris Ge (6):
SMI-S Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
Targetd Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
ONTAP Plugin: Raise NO_STATE_CHANGE error if already unmasked.
Nstor Plugin: Raise NO_STATE_CHANGE when already masked.
Simulator C Plugin: Raise LSM_ERR_NO_STATE_CHANGE when not masked.
Plugin Test: Add NO_STATE_CHANGE error detection for volume mask and
unmask.

plugin/nstor/nstor.py | 8 ++
plugin/ontap/na.py | 1 +
plugin/ontap/ontap.py | 13 +++-
plugin/simc/simc_lsmplugin.c | 10 ++-
plugin/smispy/smis.py | 174 +++++++++++++++++++++++++------------------
plugin/smispy/smis_ag.py | 21 ++++++
plugin/targetd/targetd.py | 38 +++++++---
test/plugin_test.py | 21 ++++++
8 files changed, 199 insertions(+), 87 deletions(-)
--
1.8.3.1
Gris Ge
2014-11-25 07:31:45 UTC
Permalink
* Raise LsmError ErrorNumber.NO_STATE_CHANGE in
volume_mask and volume_unmask if already masked or not masked.

* Introduced new method:
smis_ag.cim_vols_masked_to_cim_spc_path()
# To get all masked cim_vols for give cim_spc.

* Tested on EMC VNX for 'Masking and Mapping' profile.

* Tested on EMC VMAX for 'Group Masking and Mapping' profile.

* Full plugin_test passed on:
* EMC VMAX
* EMC VNX
* HP 3PAR

Changes in V2:
* Fix typo of 'requestd'.
* Remove unneeded check for ag_cim_spcs_path in _volume_unmask_group().
* New private method: _cim_vol_masked_to_spc() used in _volume_unmask_old()
and _volume_mask_old().

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 174 ++++++++++++++++++++++++++++-------------------
plugin/smispy/smis_ag.py | 21 ++++++
2 files changed, 124 insertions(+), 71 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index a46dfdd..77b1e8a 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -807,8 +807,6 @@ def _volume_mask_group(self, access_group, volume, flags=0):
target ports(all FC and FCoE port for access_group.init_type == WWPN,
and the same to iSCSI)
"""
- cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)
-
cim_init_mg_path = smis_ag.lsm_ag_to_cim_init_mg_path(
self._c, access_group)

@@ -839,6 +837,9 @@ def _volume_mask_group(self, access_group, volume, flags=0):

if len(cim_spcs_path) == 0:
# We have to create the SPC and dev_mg now.
+ cim_sys = smis_sys.cim_sys_of_sys_id(
+ self._c, access_group.system_id)
+
cim_tgt_mg_path = self._cim_tgt_mg_path_create(
cim_sys.path, cim_gmms.path, access_group.name,
access_group.init_type)
@@ -853,18 +854,16 @@ def _volume_mask_group(self, access_group, volume, flags=0):
# many tgt_mg. It's seldom use, but possible.
for cim_spc_path in cim_spcs_path:
# Check whether already masked
- cim_vol_pros = smis_vol.cim_vol_id_pros()
- cim_vols = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros)
+ cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, smis_vol.cim_vol_id_pros())
for cur_cim_vol in cim_vols:
if smis_vol.vol_id_of_cim_vol(cur_cim_vol) == volume.id:
- # Masked.
- return None
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume already masked to requested access group")

- # spc one-one map to dev_mg is mandatory in 1.5r6
+ # SNIA require each cim_spc only have one cim_dev_mg
+ # associated
cim_dev_mg_path = self._c.AssociatorNames(
cim_spc_path,
AssocClass='CIM_AssociatedDeviceMaskingGroup',
@@ -893,6 +892,25 @@ def volume_mask(self, access_group, volume, flags=0):
return self._volume_mask_group(access_group, volume, flags)
return self._volume_mask_old(access_group, volume, flags)

+ def _cim_vol_masked_to_spc(self, cim_spc_path, vol_id, property_list=None):
+ """
+ Check whether provided volume id is masked to cim_spc_path.
+ If so, return cim_vol, or return None
+ """
+ if property_list is None:
+ property_list = smis_vol.cim_vol_id_pros()
+ else:
+ property_list = merge_list(
+ property_list, smis_vol.cim_vol_id_pros())
+
+ masked_cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, property_list)
+ for masked_cim_vol in masked_cim_vols:
+ if smis_vol.vol_id_of_cim_vol(masked_cim_vol) == vol_id:
+ return masked_cim_vol
+
+ return None
+
def _volume_mask_old(self, access_group, volume, flags):
cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(self._c, access_group)

@@ -903,6 +921,12 @@ def _volume_mask_old(self, access_group, volume, flags):
access_group.id +
"will not do volume_mask()")

+ # Pre-Check: Already masked
+ if self._cim_vol_masked_to_spc(cim_spc_path, volume.id):
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume already masked to requested access group")
+
cim_ccs = self._c.cim_ccs_of_sys_id(volume.system_id)

cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
@@ -922,7 +946,6 @@ def _volume_unmask_group(self, access_group, volume):
If SupportedDeviceGroupFeatures does not allow empty
DeviceMaskingGroup in SPC, we remove SPC and DeviceMaskingGroup.
"""
- cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, volume.system_id)

cim_gmms_cap = self._c.Associators(
@@ -951,59 +974,68 @@ def _volume_unmask_group(self, access_group, volume):
"DeviceMaskingGroup in SPC. But target SMI-S provider "
"does not support any of these")

- cim_gmms = self._c.cim_gmms_of_sys_id(volume.system_id)
-
- cim_spcs_path = self._c.AssociatorNames(
+ cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
+ vol_cim_spcs_path = self._c.AssociatorNames(
cim_vol_path,
AssocClass='CIM_ProtocolControllerForUnit',
ResultClass='CIM_SCSIProtocolController')
- if len(cim_spcs_path) == 0:
+
+ if len(vol_cim_spcs_path) == 0:
# Already unmasked
- return None
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")

- flag_init_mg_found = False
- cur_cim_init_mg = None
- # Searching for CIM_DeviceMaskingGroup
- for cim_spc_path in cim_spcs_path:
- cim_init_mgs = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_AssociatedInitiatorMaskingGroup',
- ResultClass='CIM_InitiatorMaskingGroup',
- PropertyList=['InstanceID'])
- for cim_init_mg in cim_init_mgs:
- if md5(cim_init_mg['InstanceID']) == access_group.id:
- flag_init_mg_found = True
+ cim_init_mg_path = smis_ag.lsm_ag_to_cim_init_mg_path(
+ self._c, access_group)
+ ag_cim_spcs_path = self._c.AssociatorNames(
+ cim_init_mg_path,
+ AssocClass='CIM_AssociatedInitiatorMaskingGroup',
+ ResultClass='CIM_SCSIProtocolController')
+
+ found_cim_spc_path = None
+ for ag_cim_spc_path in ag_cim_spcs_path:
+ for vol_cim_spc_path in vol_cim_spcs_path:
+ if vol_cim_spc_path == ag_cim_spc_path:
+ found_cim_spc_path = vol_cim_spc_path
break

- if flag_init_mg_found:
- cim_dev_mgs_path = self._c.AssociatorNames(
- cim_spc_path,
- AssocClass='CIM_AssociatedDeviceMaskingGroup',
- ResultClass='CIM_DeviceMaskingGroup')
-
- for cim_dev_mg_path in cim_dev_mgs_path:
- if flag_empty_dev_in_spc is False:
- # We have to check whether this volume is the last
- # one in the DeviceMaskingGroup, if so, we have to
- # delete the SPC
- cur_cim_vols_path = self._c.AssociatorNames(
- cim_dev_mg_path,
- AssocClass='CIM_OrderedMemberOfCollection',
- ResultClass='CIM_StorageVolume')
- if len(cur_cim_vols_path) == 1:
- # Now, delete SPC
- in_params = {
- 'ProtocolController': cim_spc_path,
- }
- self._c.invoke_method_wait(
- 'DeleteMaskingView', cim_gmms.path, in_params)
-
- in_params = {
- 'MaskingGroup': cim_dev_mg_path,
- 'Members': [cim_vol_path],
- }
- self._c.invoke_method_wait(
- 'RemoveMembers', cim_gmms.path, in_params)
+ if found_cim_spc_path is None:
+ # Already unmasked
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+
+ # SNIA require each cim_spc only have one cim_dev_mg associated.
+ cim_dev_mg_path = self._c.AssociatorNames(
+ found_cim_spc_path,
+ AssocClass='CIM_AssociatedDeviceMaskingGroup',
+ ResultClass='CIM_DeviceMaskingGroup')[0]
+
+ cim_gmms = self._c.cim_gmms_of_sys_id(volume.system_id)
+
+ if flag_empty_dev_in_spc is False:
+ # We have to check whether this volume is the last
+ # one in the DeviceMaskingGroup, if so, we have to
+ # delete the SPC
+ cur_cim_vols_path = self._c.AssociatorNames(
+ cim_dev_mg_path,
+ AssocClass='CIM_OrderedMemberOfCollection',
+ ResultClass='CIM_StorageVolume')
+ if len(cur_cim_vols_path) == 1:
+ # last volume, should delete SPC
+ in_params = {
+ 'ProtocolController': found_cim_spc_path,
+ }
+ self._c.invoke_method_wait(
+ 'DeleteMaskingView', cim_gmms.path, in_params)
+
+ in_params = {
+ 'MaskingGroup': cim_dev_mg_path,
+ 'Members': [cim_vol_path],
+ }
+ self._c.invoke_method_wait(
+ 'RemoveMembers', cim_gmms.path, in_params)

return None

@@ -1022,11 +1054,17 @@ def volume_unmask(self, access_group, volume, flags=0):

def _volume_unmask_old(self, access_group, volume):
cim_ccs = self._c.cim_ccs_of_sys_id(volume.system_id)
+ cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(self._c, access_group)

- cim_vol_path = smis_vol.lsm_vol_to_cim_vol_path(self._c, volume)
- cim_vol = self._c.GetInstance(cim_vol_path, PropertyList=['Name'])
+ # Pre-check: not masked
+ cim_vol_pros = merge_list(['Name'], smis_vol.cim_vol_id_pros())
+ cim_vol = self._cim_vol_masked_to_spc(
+ cim_spc_path, volume.id, cim_vol_pros)

- cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(self._c, access_group)
+ if cim_vol is None:
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")

hide_params = {'LUNames': [cim_vol['Name']],
'ProtocolControllers': [cim_spc_path]}
@@ -1114,19 +1152,13 @@ def volumes_accessible_by_access_group(self, access_group, flags=0):

for cim_spc_path in cim_spcs_path:
cim_vols.extend(
- self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros))
+ smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros))
else:
cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(
self._c, access_group)
- cim_vols = self._c.Associators(
- cim_spc_path,
- AssocClass='CIM_ProtocolControllerForUnit',
- ResultClass='CIM_StorageVolume',
- PropertyList=cim_vol_pros)
+ cim_vols = smis_ag.cim_vols_masked_to_cim_spc_path(
+ self._c, cim_spc_path, cim_vol_pros)
rc = []
for cim_vol in cim_vols:
pool_id = smis_pool.pool_id_of_cim_vol(self._c, cim_vol.path)
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index 9a6d81b..5ac1d20 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -262,3 +262,24 @@ def cim_init_path_check_or_create(smis_common, system_id, init_id, init_type):
return smis_common.invoke_method_wait(
'CreateStorageHardwareID', cim_hwms.path, in_params,
out_key='HardwareID', expect_class='CIM_StorageHardwareID')
+
+
+def cim_vols_masked_to_cim_spc_path(smis_common, cim_spc_path,
+ property_list=None):
+ """
+ Use this association to find out masked volume for certain cim_spc:
+ CIM_SCSIProtocolController
+ |
+ | CIM_ProtocolControllerForUnit
+ v
+ CIM_StorageVolume
+ Return a list of CIMInstance
+ """
+ if property_list is None:
+ property_list = []
+
+ return smis_common.Associators(
+ cim_spc_path,
+ AssocClass='CIM_ProtocolControllerForUnit',
+ ResultClass='CIM_StorageVolume',
+ PropertyList=property_list)
--
1.8.3.1
Gris Ge
2014-11-25 07:31:47 UTC
Permalink
* Raise NO_STATE_CHANGE if volume_unmask() against unmasked relationship.
* Check EVDISK_ERROR_NO_SUCH_LUNMAP filer error for this condition.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/ontap/na.py | 1 +
plugin/ontap/ontap.py | 13 +++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/plugin/ontap/na.py b/plugin/ontap/na.py
index 5db7005..8532e87 100644
--- a/plugin/ontap/na.py
+++ b/plugin/ontap/na.py
@@ -143,6 +143,7 @@ class FilerError(Exception):
EVDISK_ERROR_VDISK_EXPORTED = 9013 # LUN is currently mapped
EVDISK_ERROR_VDISK_NOT_ENABLED = 9014 # LUN is not online
EVDISK_ERROR_VDISK_NOT_DISABLED = 9015 # LUN is not offline
+ EVDISK_ERROR_NO_SUCH_LUNMAP = 9016 # LUN is already unmapped
EVDISK_ERROR_INITGROUP_MAPS_EXIST = 9029 # LUN maps for this initiator
# group exist
EVDISK_ERROR_SIZE_TOO_LARGE = 9034 # LUN size too large.
diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index 31e2366..ad7e3b3 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -764,14 +764,23 @@ def volume_mask(self, access_group, volume, flags=0):
if fe.errno == na.FilerError.EVDISK_ERROR_INITGROUP_HAS_VDISK:
raise LsmError(
ErrorNumber.NO_STATE_CHANGE,
- "Volume already masked to defined access group")
+ "Volume is already masked to requested access group")
else:
raise
return None

@handle_ontap_errors
def volume_unmask(self, access_group, volume, flags=0):
- self.f.lun_unmap(access_group.name, _lsm_vol_to_na_vol_path(volume))
+ try:
+ self.f.lun_unmap(
+ access_group.name, _lsm_vol_to_na_vol_path(volume))
+ except na.FilerError as filer_error:
+ if filer_error.errno == na.FilerError.EVDISK_ERROR_NO_SUCH_LUNMAP:
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")
+ else:
+ raise
return None

@staticmethod
--
1.8.3.1
Gris Ge
2014-11-25 07:31:48 UTC
Permalink
* Raise NO_STATE_CHANGE error when volume is already masked on
volume_mask().

* As nstor will not raise error on duplicate volume mask, we have to
do a pre-check via volumes_accessible_by_access_group() method.

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

diff --git a/plugin/nstor/nstor.py b/plugin/nstor/nstor.py
index efa8734..bc0e339 100644
--- a/plugin/nstor/nstor.py
+++ b/plugin/nstor/nstor.py
@@ -659,6 +659,14 @@ def volume_mask(self, access_group, volume, flags=0):
"""
Allows an access group to access a volume.
"""
+ # Pre-check for already masked.
+ if list(v.id for v in
+ self.volumes_accessible_by_access_group(access_group)
+ if v.id == volume.id):
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is already masked to requested access group")
+
self._volume_mask(access_group.name, volume.name)
return
--
1.8.3.1
Gris Ge
2014-11-25 07:31:49 UTC
Permalink
* Raise LSM_ERR_NO_STATE_CHANGE when volume is not masked to requested
access group on volume_unmask().

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

diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index 8e08be6..403eb6e 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -1277,8 +1277,14 @@ static int volume_unmask(lsm_plugin_ptr c,
lsm_access_group_id_get(find->ag));

if( grants ) {
- g_hash_table_remove(grants, lsm_volume_id_get(volume));
- rc = LSM_ERR_OK;
+ char *vol_id = g_hash_table_lookup(
+ grants, lsm_volume_id_get(volume));
+ if( vol_id ) {
+ g_hash_table_remove(grants, lsm_volume_id_get(volume));
+ rc = LSM_ERR_OK;
+ }else{
+ rc = LSM_ERR_NO_STATE_CHANGE;
+ }
}

} else {
--
1.8.3.1
Gris Ge
2014-11-25 07:31:50 UTC
Permalink
* Plugin should raise NO_STATE_CHANGE when volume already masked on
volume_mask()
* Plugin should raise NO_STATE_CHANGE when volume is not masked on
volume_unmask()

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

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 2aa6c37..7c1c964 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -711,9 +711,30 @@ def test_mask_unmask(self):
if vol is not None and chose_ag is not None:
self.c.volume_mask(chose_ag, vol)
self._masking_state(cap, chose_ag, vol, True)
+
+ # Test duplicate call for NO_STATE_CHANGE error
+ flag_dup_error_found = False
+ try:
+ self.c.volume_mask(chose_ag, vol)
+ except LsmError as lsm_error:
+ self.assertTrue(
+ lsm_error.code == ErrorNumber.NO_STATE_CHANGE)
+ flag_dup_error_found = True
+ self.assertTrue(flag_dup_error_found == True)
+
self.c.volume_unmask(chose_ag, vol)
self._masking_state(cap, chose_ag, vol, False)

+ # Test duplicate call for NO_STATE_CHANGE error
+ flag_dup_error_found = False
+ try:
+ self.c.volume_unmask(chose_ag, vol)
+ except LsmError as lsm_error:
+ self.assertTrue(
+ lsm_error.code == ErrorNumber.NO_STATE_CHANGE)
+ flag_dup_error_found = True
+ self.assertTrue(flag_dup_error_found == True)
+
if vol:
self._volume_delete(vol)
--
1.8.3.1
Gris Ge
2014-11-25 07:31:46 UTC
Permalink
* Raise LsmError ErrorNumber.NO_STATE_CHANGE in
volume_mask and volume_unmask if already masked or not masked.

Changes in V2:

* New private method _is_masked() used in volume_mask() and volume_unmask()
to detect masking status of requesting volume.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/targetd/targetd.py | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index b40aa4c..db58739 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
@@ -243,6 +243,17 @@ def _mask_infos(self):
}])
return tgt_masks

+ def _is_masked(self, init_id, vol_id):
+ """
+ Check whether volume is masked to certain initiator.
+ Return True or False.
+ """
+ ag_id = md5(init_id)
+ for tgt_mask in self._mask_infos():
+ if tgt_mask['vol_id'] == vol_id and tgt_mask['ag_id'] == ag_id:
+ return True
+ return False
+
def volume_mask(self, access_group, volume, flags=0):
if len(access_group.init_ids) == 0:
raise LsmError(ErrorNumber.INVALID_ARGUMENT,
@@ -257,13 +268,10 @@ def volume_mask(self, access_group, volume, flags=0):
raise LsmError(ErrorNumber.NO_SUPPORT,
"Targetd only support ISCSI initiator group type")

- ag_id = md5(access_group.init_ids[0])
- vol_id = volume.id
- # Return when found already masked.
- tgt_masks = self._mask_infos()
- if list(x for x in tgt_masks
- if x['vol_id'] == vol_id and x['ag_id'] == ag_id):
- return None
+ if self._is_masked(access_group.init_ids[0], volume.id):
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is already masked to requested access group")

# find lowest unused lun ID
used_lun_ids = [x['lun_id'] for x in tgt_masks]
@@ -283,11 +291,17 @@ def volume_mask(self, access_group, volume, flags=0):

@handle_errors
def volume_unmask(self, volume, access_group, flags=0):
- self._jsonrequest("export_destroy",
- dict(pool=volume.pool_id,
- vol=volume.name,
- initiator_wwn=access_group.init_ids[0]))
- return None
+ # Pre-check if already unmasked
+ if self._is_masked(access_group.init_ids[0], volume.id):
+ self._jsonrequest("export_destroy",
+ dict(pool=volume.pool_id,
+ vol=volume.name,
+ initiator_wwn=access_group.init_ids[0]))
+ return None
+
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume is not masked to requested access group")

@handle_errors
def volumes_accessible_by_access_group(self, access_group, flags=0):
--
1.8.3.1
Tony Asleson
2014-11-25 22:02:09 UTC
Permalink
Hi Gris,

There was a couple of things I ran into and instead of having you come
back with another change set I amended 2 of the patches, the one for
targetd and the smi-s one.

Amended patch set pushed, thanks!

Regards,
Tony
Post by Gris Ge
* When volume is already masked to requested access group in volume_mask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.
* When volume is not masked to requested access group in volume_unmask()
call, we should raise NO_STATE_CHANGE error instead of silently pass.
1. Plugins update for implementation mentioned above.
2. Plugin test update for conditions above.
* All patches are tested against their array for error conditions above.
SMI-S Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
Targetd Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
ONTAP Plugin: Raise NO_STATE_CHANGE error if already unmasked.
Nstor Plugin: Raise NO_STATE_CHANGE when already masked.
Simulator C Plugin: Raise LSM_ERR_NO_STATE_CHANGE when not masked.
Plugin Test: Add NO_STATE_CHANGE error detection for volume mask and
unmask.
plugin/nstor/nstor.py | 8 ++
plugin/ontap/na.py | 1 +
plugin/ontap/ontap.py | 13 +++-
plugin/simc/simc_lsmplugin.c | 10 ++-
plugin/smispy/smis.py | 174 +++++++++++++++++++++++++------------------
plugin/smispy/smis_ag.py | 21 ++++++
plugin/targetd/targetd.py | 38 +++++++---
test/plugin_test.py | 21 ++++++
8 files changed, 199 insertions(+), 87 deletions(-)
Loading...