Discussion:
[Libstoragemgmt-devel] [PATCH 0/7] Improve access group related methods.
Gris Ge
2014-11-10 15:17:09 UTC
Permalink
* This patch set is based on:
[PATCH V2] SMI-S Plugin: Move code to smis_ag.py and
smis_vol.py (squashed commits)

* Each patch is tested individually.

* Remove unused methods, merge similar methods, improve public method.


Gris Ge (7):
SMI-S Plugin: Move job wait method to SmisCommon.
SMI-S Plugin: Move cim_init check into smis_ag.py
SMI-S Plugin: Remove unused methods from smis.py
SMI-S Plugin: Move lsm init_id to SNIA converter
SMI-S Plugin: Raise NO_STATE_CHANGE error if not masked or already
masked.
SMI-S Plugin: Raise error for duplicate call of access_group_create()
SMI-S Plugin: Raise API defined error when
access_group_initiator_add()

plugin/smispy/smis.py | 694 ++++++++++++-------------------------------
plugin/smispy/smis_ag.py | 175 ++++++++++-
plugin/smispy/smis_common.py | 113 +++++++
3 files changed, 480 insertions(+), 502 deletions(-)
--
1.8.3.1


------------------------------------------------------------------------------
Gris Ge
2014-11-10 15:17:10 UTC
Permalink
* Replaced Smis._wait_invoke() with SmisCommon.invoke_method_wait().
* Removed Smis._poll() and replace it with SmisCommon.invoke_method_wait().
* Replaced Smis._job_completed_ok with SmisCommon.cim_job_completed_ok() as
it used by Smis.job_status() and SmisCommon.invoke_method_wait().
* Tested on EMC VMAX for access group create/delete, volume mask/unmask,
volume create/delete.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 241 +++++++++----------------------------------
plugin/smispy/smis_common.py | 113 ++++++++++++++++++++
2 files changed, 159 insertions(+), 195 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index bdca963..1847e07 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -128,9 +128,6 @@ class Smis(IStorageAreaNetwork):
SMI-S plug-ing which exposes a small subset of the overall provided
functionality of SMI-S
"""
- _INVOKE_MAX_LOOP_COUNT = 60
- _INVOKE_CHECK_INTERVAL = 5
-
_JOB_ERROR_HANDLER = {
SmisCommon.JOB_RETRIEVE_VOLUME_CREATE:
smis_vol.volume_create_error_handler,
@@ -241,25 +238,6 @@ def capabilities(self, system, flags=0):
def plugin_info(self, flags=0):
return "Generic SMI-S support", VERSION

- @staticmethod
- def _job_completed_ok(status):
- """
- Given a concrete job instance, check the operational status. This
- is a little convoluted as different SMI-S proxies return the values in
- different positions in list :-)
- """
- rc = False
- op = status['OperationalStatus']
-
- if (len(op) > 1 and
- ((op[0] == dmtf.OP_STATUS_OK and
- op[1] == dmtf.OP_STATUS_COMPLETED) or
- (op[0] == dmtf.OP_STATUS_COMPLETED and
- op[1] == dmtf.OP_STATUS_OK))):
- rc = True
-
- return rc
-
@handle_cim_errors
def job_status(self, job_id, flags=0):
"""
@@ -298,7 +276,7 @@ def job_status(self, job_id, flags=0):
status = JobStatus.COMPLETE
percent_complete = 100

- if Smis._job_completed_ok(cim_job):
+ if SmisCommon.cim_job_completed_ok(cim_job):
if retrieve_data == SmisCommon.JOB_RETRIEVE_VOLUME or \
retrieve_data == SmisCommon.JOB_RETRIEVE_VOLUME_CREATE:
completed_item = self._new_vol_from_job(cim_job)
@@ -597,21 +575,6 @@ def volume_create(self, pool, volume_name, size_bytes, provisioning,
retrieve_data=SmisCommon.JOB_RETRIEVE_VOLUME_CREATE,
method_data=volume_name)

- def _poll(self, msg, job):
- if job:
- while True:
- (s, percent, i) = self.job_status(job)
-
- if s == JobStatus.INPROGRESS:
- time.sleep(0.25)
- elif s == JobStatus.COMPLETE:
- self.job_free(job)
- return i
- else:
- raise LsmError(
- ErrorNumber.PLUGIN_BUG,
- msg + ", job error code= " + str(s))
-
def _detach_netapp_e(self, vol, sync):
#Get the Configuration service for the system we are interested in.
cim_scs = self._c.cim_scs_of_sys_id(vol.system_id)
@@ -619,10 +582,8 @@ def _detach_netapp_e(self, vol, sync):
in_params = {'Operation': pywbem.Uint16(2),
'Synchronization': sync.path}

- job_id = self._c.invoke_method(
- 'ModifySynchronization', cim_scs.path, in_params)[0]
-
- self._poll("ModifySynchronization, detach", job_id)
+ self._c.invoke_method_wait(
+ 'ModifySynchronization', cim_scs.path, in_params)

def _detach(self, vol, sync):
if self._c.is_netappe():
@@ -634,10 +595,8 @@ def _detach(self, vol, sync):
in_params = {'Operation': pywbem.Uint16(8),
'Synchronization': sync.path}

- job_id = self._c.invoke_method(
- 'ModifyReplicaSynchronization', cim_rs.path, in_params)[0]
-
- self._poll("ModifyReplicaSynchronization, detach", job_id)
+ self._c.invoke_method_wait(
+ 'ModifyReplicaSynchronization', cim_rs.path, in_params)

@staticmethod
def _cim_name_match(a, b):
@@ -917,18 +876,15 @@ def _cim_dev_mg_path_create(self, cim_gmms_path, name, cim_vol_path,
try:
(rc, out) = self._c.InvokeMethod('CreateGroup', cim_gmms_path,
**in_params)
- except CIMError as ce:
- if ce[0] == pywbem.CIM_ERR_FAILED:
- cim_dev_mg_path = self._check_exist_cim_dev_mg(
- name, cim_gmms_path, cim_vol_path, vol_id)
- if cim_dev_mg_path is None:
- raise
- else:
- raise
- if cim_dev_mg_path is None:
- cim_dev_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
+ cim_dev_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms_path, in_params,
+ out_key='MaskingGroup',
expect_class='CIM_TargetMaskingGroup')
+ except (LsmError, CIMError):
+ cim_dev_mg_path = self._check_exist_cim_dev_mg(
+ name, cim_gmms_path, cim_vol_path, vol_id)
+ if cim_dev_mg_path is None:
+ raise

return cim_dev_mg_path

@@ -963,21 +919,14 @@ def _cim_tgt_mg_path_create(self, cim_sys_path, cim_gmms_path, name,

cim_tgt_mg_path = None
try:
- (rc, out) = self._c.InvokeMethod('CreateGroup', cim_gmms_path,
- **in_params)
- except CIMError as ce:
- if ce[0] == pywbem.CIM_ERR_FAILED:
- cim_tgt_mg_path = self._check_exist_cim_tgt_mg(name)
- if cim_tgt_mg_path is None:
- raise
- else:
+ cim_tgt_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms_path, in_params,
+ out_key='MaskingGroup', expect_class='CIM_TargetMaskingGroup')
+ except (LsmError, CIMError):
+ cim_tgt_mg_path = self._check_exist_cim_tgt_mg(name)
+ if cim_tgt_mg_path is None:
raise

- if cim_tgt_mg_path is None:
- cim_tgt_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
- expect_class='CIM_TargetMaskingGroup')
-
return cim_tgt_mg_path

def _cim_spc_path_create(self, cim_gmms_path, cim_init_mg_path,
@@ -989,11 +938,9 @@ def _cim_spc_path_create(self, cim_gmms_path, cim_init_mg_path,
'DeviceMaskingGroup': cim_dev_mg_path,
}

- (rc, out) = self._c.InvokeMethod('CreateMaskingView', cim_gmms_path,
- **in_params)
-
- return self._wait_invoke(
- rc, out, out_key='ProtocolController',
+ return self._c.invoke_method_wait(
+ 'CreateMaskingView', cim_gmms_path, in_params,
+ out_key='ProtocolController',
expect_class='CIM_SCSIProtocolController')

def _volume_mask_group(self, access_group, volume, flags=0):
@@ -1075,10 +1022,8 @@ def _volume_mask_group(self, access_group, volume, flags=0):
'MaskingGroup': cim_dev_mg_path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'AddMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'AddMembers', cim_gmms.path, in_params)
return None

@handle_cim_errors
@@ -1116,10 +1061,7 @@ def _volume_mask_old(self, access_group, volume, flags):
'ProtocolControllers': [cim_spc_path],
'DeviceAccesses': [dmtf.CTRL_CONF_SRV_DA_RW]}

- (rc, out) = self._c.InvokeMethod(
- 'ExposePaths',
- cim_ccs.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('ExposePaths', cim_ccs.path, in_params)
return None

def _volume_unmask_group(self, access_group, volume):
@@ -1202,19 +1144,15 @@ def _volume_unmask_group(self, access_group, volume):
in_params = {
'ProtocolController': cim_spc_path,
}
- (rc, out) = self._c.InvokeMethod(
- 'DeleteMaskingView',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'DeleteMaskingView', cim_gmms.path, in_params)

in_params = {
'MaskingGroup': cim_dev_mg_path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'RemoveMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'RemoveMembers', cim_gmms.path, in_params)

return None

@@ -1242,10 +1180,7 @@ def _volume_unmask_old(self, access_group, volume):
hide_params = {'LUNames': [cim_vol['Name']],
'ProtocolControllers': [cim_spc_path]}

- (rc, out) = self._c.InvokeMethod('HidePaths', cim_ccs.path,
- **hide_params)
- self._wait_invoke(rc, out)
-
+ self._c.invoke_method_wait('HidePaths', cim_ccs.path, hide_params)
return None

def _is_access_group(self, cim_spc):
@@ -1484,12 +1419,10 @@ def _cim_init_path_create(self, system_id, init_id, dmtf_id_type):
in_params = {'StorageID': init_id,
'IDType': pywbem.Uint16(dmtf_id_type)}

- (rc, out) = self._c.InvokeMethod('CreateStorageHardwareID',
- cim_hwms.path, **in_params)
# CreateStorageHardwareID does not allow ASYNC
- return self._wait_invoke(
- rc, out, out_key='HardwareID',
- expect_class='CIM_StorageHardwareID')
+ return self._c.invoke_method_wait(
+ 'CreateStorageHardwareID', cim_hwms.path, in_params,
+ out_key='HardwareID', expect_class='CIM_StorageHardwareID')

def _ag_init_add_group(self, access_group, init_id, init_type):
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)
@@ -1520,12 +1453,10 @@ def _ag_init_add_group(self, access_group, init_id, init_type):
'MaskingGroup': cim_init_mg_path,
'Members': [cim_init_path],
}
- (rc, out) = self._c.InvokeMethod('AddMembers',
- cim_gmms.path, **in_params)

- new_cim_init_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
- expect_class='CIM_InitiatorMaskingGroup')
+ new_cim_init_mg_path = self._c.invoke_method_wait(
+ 'AddMembers', cim_gmms.path, in_params,
+ out_key='MaskingGroup', expect_class='CIM_InitiatorMaskingGroup')
cim_init_mg_pros = smis_ag.cim_init_mg_pros()
new_cim_init_mg = self._c.GetInstance(
new_cim_init_mg_path, PropertyList=cim_init_mg_pros,
@@ -1577,10 +1508,9 @@ def _ag_init_add_old(self, access_group, init_id, init_type):
in_params = {'InitiatorPortIDs': [init_id],
'ProtocolControllers': [cim_spc_path]}

- (rc, out) = self._c.InvokeMethod('ExposePaths',
- cim_ccs.path, **in_params)
- cim_spc_path = self._wait_invoke(
- rc, out, out_key='ProtocolControllers', flag_out_array=True,
+ cim_spc_path = self._c.invoke_method_wait(
+ 'ExposePaths', cim_ccs.path, in_params,
+ out_key='ProtocolControllers', flag_out_array=True,
expect_class='CIM_SCSIProtocolController')

cim_spc_pros = smis_ag.cim_spc_pros()
@@ -1623,10 +1553,7 @@ def _ag_init_del_group(self, access_group, init_id):
'Members': [cim_init.path],
}

- (rc, out) = self._c.InvokeMethod(
- 'RemoveMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('RemoveMembers', cim_gmms.path, in_params)

cim_init_mg_pros = smis_ag.cim_init_mg_pros()
cim_init_mg = self._c.GetInstance(
@@ -1660,10 +1587,8 @@ def _ag_init_del_old(self, access_group, init_id):

hide_params = {'InitiatorPortIDs': [init_id],
'ProtocolControllers': [cim_spc_path]}
- (rc, out) = self._c.InvokeMethod(
- 'HidePaths', cim_ccs.path, **hide_params)
+ self._c.invoke_method_wait('HidePaths', cim_ccs.path, hide_params)

- self._wait_invoke(rc, out)
return None

@handle_cim_errors
@@ -2052,71 +1977,6 @@ def target_ports(self, search_key=None, search_value=None, flags=0):

return search_property(rc, search_key, search_value)

- def _wait_invoke(self, rc, out, out_key=None, expect_class=None,
- flag_out_array=False,):
- """
- Return out[out_key] if found rc == SmisCommon.SNIA_INVOKE_OK.
- For rc == SmisCommon.SNIA_INVOKE_ASYNC, we check every
- Smis._INVOKE_CHECK_INTERVAL
- seconds until done. Then return association via CIM_AffectedJobElement
- Return CIM_InstanceName
- Assuming only one CIM_InstanceName will get.
- """
- if rc == SmisCommon.SNIA_INVOKE_OK:
- if out_key is None:
- return None
- if out_key in out:
- if flag_out_array:
- if len(out[out_key]) != 1:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(), %s is not length 1: %s"
- % (out_key, out.items()))
- return out[out_key][0]
- return out[out_key]
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(), %s not exist in out %s" %
- (out_key, out.items()))
- elif rc == SmisCommon.SNIA_INVOKE_ASYNC:
- cim_job_path = out['Job']
- loop_counter = 0
- job_pros = ['JobState', 'PercentComplete', 'ErrorDescription',
- 'OperationalStatus']
- cim_xxxs_path = []
- while(loop_counter <= Smis._INVOKE_MAX_LOOP_COUNT):
- cim_job = self._c.GetInstance(cim_job_path,
- PropertyList=job_pros,
- LocalOnly=False)
- job_state = cim_job['JobState']
- if job_state in (dmtf.JOB_STATE_NEW, dmtf.JOB_STATE_STARTING,
- dmtf.JOB_STATE_RUNNING):
- loop_counter += 1
- time.sleep(Smis._INVOKE_CHECK_INTERVAL)
- continue
- elif job_state == dmtf.JOB_STATE_COMPLETED:
- if expect_class is None:
- return None
- cim_xxxs_path = self._c.AssociatorNames(
- cim_job.path,
- AssocClass='CIM_AffectedJobElement',
- ResultClass=expect_class)
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): Got unknown job state "
- "%d: %s" % (job_state, cim_job.items()))
- if len(cim_xxxs_path) != 1:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): got unexpected(not 1) "
- "return from CIM_AffectedJobElement: "
- "%s, out: %s, job: %s" %
- (cim_xxxs_path, out.items(),
- cim_job.items()))
- return cim_xxxs_path[0]
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): Got unexpected rc code "
- "%d, out: %s" % (rc, out.items()))
-
def _cim_pep_path_of_fc_tgt(self, cim_fc_tgt_path):
"""
Return CIMInstanceName of CIM_SCSIProtocolEndpoint of CIM_FCPort
@@ -2178,10 +2038,7 @@ def _check_exist_cim_dev_mg(self, name, cim_gmms_path, cim_vol_path,
'MaskingGroup': cim_dev_mg.path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'AddMembers',
- cim_gmms_path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('AddMembers', cim_gmms_path, in_params)
return cim_dev_mg.path

return None
@@ -2248,13 +2105,10 @@ def access_group_create(self, name, init_id, init_type, system,
cim_init_mg_pros = smis_ag.cim_init_mg_pros()

try:
- (rc, out) = self._c.InvokeMethod(
- 'CreateGroup', cim_gmms.path, **in_params)
-
- cim_init_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
+ cim_init_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms.path, in_params,
+ out_key='MaskingGroup',
expect_class='CIM_InitiatorMaskingGroup')
-
except (LsmError, CIMError):
# Check possible failure
# 1. Initiator already exist in other group.
@@ -2327,8 +2181,5 @@ def access_group_delete(self, access_group, flags=0):
'Force': True,
}

- (rc, out) = self._c.InvokeMethod('DeleteGroup', cim_gmms.path,
- **in_params)
-
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('DeleteGroup', cim_gmms.path, in_params)
return None
diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
index 456e6bc..5783a89 100644
--- a/plugin/smispy/smis_common.py
+++ b/plugin/smispy/smis_common.py
@@ -27,6 +27,7 @@
import traceback
import os
import datetime
+import time

import dmtf
from lsm import LsmError, ErrorNumber, md5
@@ -180,6 +181,9 @@ class SmisCommon(object):
IAAN_WBEM_HTTP_PORT = 5988
IAAN_WBEM_HTTPS_PORT = 5989

+ _INVOKE_MAX_LOOP_COUNT = 60
+ _INVOKE_CHECK_INTERVAL = 5
+
def __init__(self, url, username, password,
namespace=dmtf.DEFAULT_NAMESPACE,
no_ssl_verify=False, debug_path=None, system_list=None):
@@ -359,6 +363,25 @@ def _job_id_of_cim_job(cim_job, retrieve_data, method_data):
md5(cim_job['InstanceID']), int(retrieve_data), str(method_data))

@staticmethod
+ def cim_job_completed_ok(cim_job):
+ """
+ Given a CIM_ConcreteJob, check the operational status. This
+ is a little convoluted as different SMI-S proxies return the values in
+ different positions in list :-)
+ """
+ rc = False
+ op = cim_job['OperationalStatus']
+
+ if (len(op) > 1 and
+ ((op[0] == dmtf.OP_STATUS_OK and
+ op[1] == dmtf.OP_STATUS_COMPLETED) or
+ (op[0] == dmtf.OP_STATUS_COMPLETED and
+ op[1] == dmtf.OP_STATUS_OK))):
+ rc = True
+
+ return rc
+
+ @staticmethod
def parse_job_id(job_id):
"""
job_id is assembled by a md5 string, retrieve_data and method_data
@@ -457,6 +480,96 @@ def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
else:
raise

+ def invoke_method_wait(self, cmd, cim_path, in_params,
+ out_key=None, expect_class=None,
+ flag_out_array=False):
+ """
+ InvokeMethod and wait it untile done.
+ Retrun a CIMInstanceName from out[out_key] or from cim_job:
+ CIM_ConcreteJob
+ |
+ | CIM_AffectedJobElement
+ v
+ CIMInstanceName # expect_class
+ If flag_out_array is True, return the first element of out[out_key].
+ """
+ (rc, out) = self.InvokeMethod(cmd, cim_path, **in_params)
+
+ if rc == SmisCommon.SNIA_INVOKE_OK:
+ if out_key is None:
+ return None
+ if out_key in out:
+ if flag_out_array:
+ if len(out[out_key]) == 1:
+ return out[out_key][0]
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(), output contains %d " %
+ len(out[out_key]) +
+ "elements: %s" % out[out_key])
+ return out[out_key]
+ else:
+ raise LsmError(ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(), %s not exist in out %s" %
+ (out_key, out.items()))
+
+ elif rc == SmisCommon.SNIA_INVOKE_ASYNC:
+ cim_job_path = out['Job']
+ loop_counter = 0
+ job_pros = ['JobState', 'ErrorDescription',
+ 'OperationalStatus']
+ cim_xxxs_path = []
+ while(loop_counter <= SmisCommon._INVOKE_MAX_LOOP_COUNT):
+ cim_job = self.GetInstance(cim_job_path,
+ PropertyList=job_pros)
+ job_state = cim_job['JobState']
+ if job_state in (dmtf.JOB_STATE_NEW, dmtf.JOB_STATE_STARTING,
+ dmtf.JOB_STATE_RUNNING):
+ loop_counter += 1
+ time.sleep(SmisCommon._INVOKE_CHECK_INTERVAL)
+ continue
+ elif job_state == dmtf.JOB_STATE_COMPLETED:
+ if not SmisCommon.cim_job_completed_ok(cim_job):
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ str(cim_job['ErrorDescription']))
+ if expect_class is None:
+ return None
+ cim_xxxs_path = self.AssociatorNames(
+ cim_job.path,
+ AssocClass='CIM_AffectedJobElement',
+ ResultClass=expect_class)
+ break
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): Got unknown job state "
+ "%d: %s" % (job_state, cim_job.items()))
+
+ if loop_counter > SmisCommon._INVOKE_MAX_LOOP_COUNT:
+ raise LsmError(
+ ErrorNumber.TIMEOUT,
+ "The job generated by %s() failed to finish in %ds" %
+ (cmd,
+ SmisCommon._INVOKE_CHECK_INTERVAL *
+ SmisCommon._INVOKE_MAX_LOOP_COUNT))
+
+ if len(cim_xxxs_path) == 1:
+ return cim_xxxs_path[0]
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): got unexpected(not 1) "
+ "return from CIM_AffectedJobElement: "
+ "%s, out: %s, job: %s" %
+ (cim_xxxs_path, out.items(), cim_job.items()))
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): Got unexpected rc code "
+ "%d, out: %s" % (rc, out.items()))
+
def _cim_srv_of_sys_id(self, srv_name, sys_id, raise_error):
property_list = ['SystemName']
--
1.8.3.1


------------------------------------------------------------------------------
Tony Asleson
2014-11-10 20:09:35 UTC
Permalink
Post by Gris Ge
* Replaced Smis._wait_invoke() with SmisCommon.invoke_method_wait().
* Removed Smis._poll() and replace it with SmisCommon.invoke_method_wait().
Is there any reason we couldn't have the implementation of
invoke_method_wait be implemented as a call to invoke_method followed by
_poll?

My main issue is we have invoke_method which will retrieve debug data if
requested, but invoke_method_wait does not. Having two similar but
different code paths results in bugs like this which I would like to avoid.

Note: In future, move in one commit, then rename methods moved in
another commit.

More comments below.

Thanks,
Tony
Post by Gris Ge
* Replaced Smis._job_completed_ok with SmisCommon.cim_job_completed_ok() as
it used by Smis.job_status() and SmisCommon.invoke_method_wait().
* Tested on EMC VMAX for access group create/delete, volume mask/unmask,
volume create/delete.
---
plugin/smispy/smis.py | 241 +++++++++----------------------------------
plugin/smispy/smis_common.py | 113 ++++++++++++++++++++
2 files changed, 159 insertions(+), 195 deletions(-)
diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index bdca963..1847e07 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
SMI-S plug-ing which exposes a small subset of the overall provided
functionality of SMI-S
"""
- _INVOKE_MAX_LOOP_COUNT = 60
- _INVOKE_CHECK_INTERVAL = 5
-
_JOB_ERROR_HANDLER = {
smis_vol.volume_create_error_handler,
return "Generic SMI-S support", VERSION
- """
- Given a concrete job instance, check the operational status. This
- is a little convoluted as different SMI-S proxies return the values in
- different positions in list :-)
- """
- rc = False
- op = status['OperationalStatus']
-
- if (len(op) > 1 and
- ((op[0] == dmtf.OP_STATUS_OK and
- op[1] == dmtf.OP_STATUS_COMPLETED) or
- (op[0] == dmtf.OP_STATUS_COMPLETED and
- rc = True
-
- return rc
-
@handle_cim_errors
"""
status = JobStatus.COMPLETE
percent_complete = 100
if retrieve_data == SmisCommon.JOB_RETRIEVE_VOLUME or \
completed_item = self._new_vol_from_job(cim_job)
@@ -597,21 +575,6 @@ def volume_create(self, pool, volume_name, size_bytes, provisioning,
retrieve_data=SmisCommon.JOB_RETRIEVE_VOLUME_CREATE,
method_data=volume_name)
- (s, percent, i) = self.job_status(job)
-
- time.sleep(0.25)
- self.job_free(job)
- return i
- raise LsmError(
- ErrorNumber.PLUGIN_BUG,
- msg + ", job error code= " + str(s))
-
#Get the Configuration service for the system we are interested in.
cim_scs = self._c.cim_scs_of_sys_id(vol.system_id)
in_params = {'Operation': pywbem.Uint16(2),
'Synchronization': sync.path}
- job_id = self._c.invoke_method(
- 'ModifySynchronization', cim_scs.path, in_params)[0]
-
- self._poll("ModifySynchronization, detach", job_id)
+ self._c.invoke_method_wait(
+ 'ModifySynchronization', cim_scs.path, in_params)
in_params = {'Operation': pywbem.Uint16(8),
'Synchronization': sync.path}
- job_id = self._c.invoke_method(
- 'ModifyReplicaSynchronization', cim_rs.path, in_params)[0]
-
- self._poll("ModifyReplicaSynchronization, detach", job_id)
+ self._c.invoke_method_wait(
+ 'ModifyReplicaSynchronization', cim_rs.path, in_params)
@staticmethod
@@ -917,18 +876,15 @@ def _cim_dev_mg_path_create(self, cim_gmms_path, name, cim_vol_path,
(rc, out) = self._c.InvokeMethod('CreateGroup', cim_gmms_path,
**in_params)
- cim_dev_mg_path = self._check_exist_cim_dev_mg(
- name, cim_gmms_path, cim_vol_path, vol_id)
- raise
- raise
- cim_dev_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
+ cim_dev_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms_path, in_params,
+ out_key='MaskingGroup',
expect_class='CIM_TargetMaskingGroup')
What am I missing here? It looks like we are calling:

(rc, out) = self._c.InvokeMethod('CreateGroup', cim_gmms_path,
**in_params)

immediately followed by:

cim_dev_mg_path = self._c.invoke_method_wait(
'CreateGroup', cim_gmms_path, in_params,
out_key='MaskingGroup',
expect_class='CIM_TargetMaskingGroup')
Post by Gris Ge
+ cim_dev_mg_path = self._check_exist_cim_dev_mg(
+ name, cim_gmms_path, cim_vol_path, vol_id)
+ raise
return cim_dev_mg_path
@@ -963,21 +919,14 @@ def _cim_tgt_mg_path_create(self, cim_sys_path, cim_gmms_path, name,
cim_tgt_mg_path = None
- (rc, out) = self._c.InvokeMethod('CreateGroup', cim_gmms_path,
- **in_params)
- cim_tgt_mg_path = self._check_exist_cim_tgt_mg(name)
- raise
+ cim_tgt_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms_path, in_params,
+ out_key='MaskingGroup', expect_class='CIM_TargetMaskingGroup')
+ cim_tgt_mg_path = self._check_exist_cim_tgt_mg(name)
raise
- cim_tgt_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
- expect_class='CIM_TargetMaskingGroup')
-
return cim_tgt_mg_path
def _cim_spc_path_create(self, cim_gmms_path, cim_init_mg_path,
@@ -989,11 +938,9 @@ def _cim_spc_path_create(self, cim_gmms_path, cim_init_mg_path,
'DeviceMaskingGroup': cim_dev_mg_path,
}
- (rc, out) = self._c.InvokeMethod('CreateMaskingView', cim_gmms_path,
- **in_params)
-
- return self._wait_invoke(
- rc, out, out_key='ProtocolController',
+ return self._c.invoke_method_wait(
+ 'CreateMaskingView', cim_gmms_path, in_params,
+ out_key='ProtocolController',
expect_class='CIM_SCSIProtocolController')
'MaskingGroup': cim_dev_mg_path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'AddMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'AddMembers', cim_gmms.path, in_params)
return None
@handle_cim_errors
'ProtocolControllers': [cim_spc_path],
'DeviceAccesses': [dmtf.CTRL_CONF_SRV_DA_RW]}
- (rc, out) = self._c.InvokeMethod(
- 'ExposePaths',
- cim_ccs.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('ExposePaths', cim_ccs.path, in_params)
return None
in_params = {
'ProtocolController': cim_spc_path,
}
- (rc, out) = self._c.InvokeMethod(
- 'DeleteMaskingView',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'DeleteMaskingView', cim_gmms.path, in_params)
in_params = {
'MaskingGroup': cim_dev_mg_path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'RemoveMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'RemoveMembers', cim_gmms.path, in_params)
return None
hide_params = {'LUNames': [cim_vol['Name']],
'ProtocolControllers': [cim_spc_path]}
- (rc, out) = self._c.InvokeMethod('HidePaths', cim_ccs.path,
- **hide_params)
- self._wait_invoke(rc, out)
-
+ self._c.invoke_method_wait('HidePaths', cim_ccs.path, hide_params)
return None
in_params = {'StorageID': init_id,
'IDType': pywbem.Uint16(dmtf_id_type)}
- (rc, out) = self._c.InvokeMethod('CreateStorageHardwareID',
- cim_hwms.path, **in_params)
# CreateStorageHardwareID does not allow ASYNC
- return self._wait_invoke(
- rc, out, out_key='HardwareID',
- expect_class='CIM_StorageHardwareID')
+ return self._c.invoke_method_wait(
+ 'CreateStorageHardwareID', cim_hwms.path, in_params,
+ out_key='HardwareID', expect_class='CIM_StorageHardwareID')
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)
'MaskingGroup': cim_init_mg_path,
'Members': [cim_init_path],
}
- (rc, out) = self._c.InvokeMethod('AddMembers',
- cim_gmms.path, **in_params)
- new_cim_init_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
- expect_class='CIM_InitiatorMaskingGroup')
+ new_cim_init_mg_path = self._c.invoke_method_wait(
+ 'AddMembers', cim_gmms.path, in_params,
+ out_key='MaskingGroup', expect_class='CIM_InitiatorMaskingGroup')
cim_init_mg_pros = smis_ag.cim_init_mg_pros()
new_cim_init_mg = self._c.GetInstance(
new_cim_init_mg_path, PropertyList=cim_init_mg_pros,
in_params = {'InitiatorPortIDs': [init_id],
'ProtocolControllers': [cim_spc_path]}
- (rc, out) = self._c.InvokeMethod('ExposePaths',
- cim_ccs.path, **in_params)
- cim_spc_path = self._wait_invoke(
- rc, out, out_key='ProtocolControllers', flag_out_array=True,
+ cim_spc_path = self._c.invoke_method_wait(
+ 'ExposePaths', cim_ccs.path, in_params,
+ out_key='ProtocolControllers', flag_out_array=True,
expect_class='CIM_SCSIProtocolController')
cim_spc_pros = smis_ag.cim_spc_pros()
'Members': [cim_init.path],
}
- (rc, out) = self._c.InvokeMethod(
- 'RemoveMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('RemoveMembers', cim_gmms.path, in_params)
cim_init_mg_pros = smis_ag.cim_init_mg_pros()
cim_init_mg = self._c.GetInstance(
hide_params = {'InitiatorPortIDs': [init_id],
'ProtocolControllers': [cim_spc_path]}
- (rc, out) = self._c.InvokeMethod(
- 'HidePaths', cim_ccs.path, **hide_params)
+ self._c.invoke_method_wait('HidePaths', cim_ccs.path, hide_params)
- self._wait_invoke(rc, out)
return None
@handle_cim_errors
return search_property(rc, search_key, search_value)
- def _wait_invoke(self, rc, out, out_key=None, expect_class=None,
- """
- Return out[out_key] if found rc == SmisCommon.SNIA_INVOKE_OK.
- For rc == SmisCommon.SNIA_INVOKE_ASYNC, we check every
- Smis._INVOKE_CHECK_INTERVAL
- seconds until done. Then return association via CIM_AffectedJobElement
- Return CIM_InstanceName
- Assuming only one CIM_InstanceName will get.
- """
- return None
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(), %s is not length 1: %s"
- % (out_key, out.items()))
- return out[out_key][0]
- return out[out_key]
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(), %s not exist in out %s" %
- (out_key, out.items()))
- cim_job_path = out['Job']
- loop_counter = 0
- job_pros = ['JobState', 'PercentComplete', 'ErrorDescription',
- 'OperationalStatus']
- cim_xxxs_path = []
- cim_job = self._c.GetInstance(cim_job_path,
- PropertyList=job_pros,
- LocalOnly=False)
- job_state = cim_job['JobState']
- if job_state in (dmtf.JOB_STATE_NEW, dmtf.JOB_STATE_STARTING,
- loop_counter += 1
- time.sleep(Smis._INVOKE_CHECK_INTERVAL)
- continue
- return None
- cim_xxxs_path = self._c.AssociatorNames(
- cim_job.path,
- AssocClass='CIM_AffectedJobElement',
- ResultClass=expect_class)
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): Got unknown job state "
- "%d: %s" % (job_state, cim_job.items()))
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): got unexpected(not 1) "
- "return from CIM_AffectedJobElement: "
- "%s, out: %s, job: %s" %
- (cim_xxxs_path, out.items(),
- cim_job.items()))
- return cim_xxxs_path[0]
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): Got unexpected rc code "
- "%d, out: %s" % (rc, out.items()))
-
"""
Return CIMInstanceName of CIM_SCSIProtocolEndpoint of CIM_FCPort
@@ -2178,10 +2038,7 @@ def _check_exist_cim_dev_mg(self, name, cim_gmms_path, cim_vol_path,
'MaskingGroup': cim_dev_mg.path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'AddMembers',
- cim_gmms_path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('AddMembers', cim_gmms_path, in_params)
return cim_dev_mg.path
return None
@@ -2248,13 +2105,10 @@ def access_group_create(self, name, init_id, init_type, system,
cim_init_mg_pros = smis_ag.cim_init_mg_pros()
- (rc, out) = self._c.InvokeMethod(
- 'CreateGroup', cim_gmms.path, **in_params)
-
- cim_init_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
+ cim_init_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms.path, in_params,
+ out_key='MaskingGroup',
expect_class='CIM_InitiatorMaskingGroup')
-
# Check possible failure
# 1. Initiator already exist in other group.
'Force': True,
}
- (rc, out) = self._c.InvokeMethod('DeleteGroup', cim_gmms.path,
- **in_params)
-
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('DeleteGroup', cim_gmms.path, in_params)
return None
diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
index 456e6bc..5783a89 100644
--- a/plugin/smispy/smis_common.py
+++ b/plugin/smispy/smis_common.py
@@ -27,6 +27,7 @@
import traceback
import os
import datetime
+import time
import dmtf
from lsm import LsmError, ErrorNumber, md5
IAAN_WBEM_HTTP_PORT = 5988
IAAN_WBEM_HTTPS_PORT = 5989
+ _INVOKE_MAX_LOOP_COUNT = 60
+ _INVOKE_CHECK_INTERVAL = 5
+
def __init__(self, url, username, password,
namespace=dmtf.DEFAULT_NAMESPACE,
md5(cim_job['InstanceID']), int(retrieve_data), str(method_data))
@staticmethod
+ """
+ Given a CIM_ConcreteJob, check the operational status. This
+ is a little convoluted as different SMI-S proxies return the values in
+ different positions in list :-)
+ """
+ rc = False
+ op = cim_job['OperationalStatus']
+
+ if (len(op) > 1 and
+ ((op[0] == dmtf.OP_STATUS_OK and
+ op[1] == dmtf.OP_STATUS_COMPLETED) or
+ (op[0] == dmtf.OP_STATUS_COMPLETED and
+ rc = True
+
+ return rc
+
"""
job_id is assembled by a md5 string, retrieve_data and method_data
@@ -457,6 +480,96 @@ def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
raise
+ def invoke_method_wait(self, cmd, cim_path, in_params,
+ out_key=None, expect_class=None,
+ """
+ InvokeMethod and wait it untile done.
Type: until
Typo: Return
Post by Gris Ge
+ CIM_ConcreteJob
+ |
+ | CIM_AffectedJobElement
+ v
+ CIMInstanceName # expect_class
+ If flag_out_array is True, return the first element of out[out_key].
+ """
+ (rc, out) = self.InvokeMethod(cmd, cim_path, **in_params)
+
+ return None
+ return out[out_key][0]
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(), output contains %d " %
+ len(out[out_key]) +
+ "elements: %s" % out[out_key])
+ return out[out_key]
+ raise LsmError(ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(), %s not exist in out %s" %
+ (out_key, out.items()))
+
+ cim_job_path = out['Job']
+ loop_counter = 0
+ job_pros = ['JobState', 'ErrorDescription',
+ 'OperationalStatus']
+ cim_xxxs_path = []
+ cim_job = self.GetInstance(cim_job_path,
+ PropertyList=job_pros)
+ job_state = cim_job['JobState']
+ if job_state in (dmtf.JOB_STATE_NEW, dmtf.JOB_STATE_STARTING,
+ loop_counter += 1
+ time.sleep(SmisCommon._INVOKE_CHECK_INTERVAL)
+ continue
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ str(cim_job['ErrorDescription']))
+ return None
+ cim_xxxs_path = self.AssociatorNames(
+ cim_job.path,
+ AssocClass='CIM_AffectedJobElement',
+ ResultClass=expect_class)
+ break
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): Got unknown job state "
+ "%d: %s" % (job_state, cim_job.items()))
+
+ raise LsmError(
+ ErrorNumber.TIMEOUT,
+ "The job generated by %s() failed to finish in %ds" %
+ (cmd,
+ SmisCommon._INVOKE_CHECK_INTERVAL *
+ SmisCommon._INVOKE_MAX_LOOP_COUNT))
+
+ return cim_xxxs_path[0]
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): got unexpected(not 1) "
+ "return from CIM_AffectedJobElement: "
+ "%s, out: %s, job: %s" %
+ (cim_xxxs_path, out.items(), cim_job.items()))
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): Got unexpected rc code "
+ "%d, out: %s" % (rc, out.items()))
+
property_list = ['SystemName']
Gris Ge
2014-11-11 12:23:48 UTC
Permalink
Post by Tony Asleson
Post by Gris Ge
* Replaced Smis._wait_invoke() with SmisCommon.invoke_method_wait().
* Removed Smis._poll() and replace it with SmisCommon.invoke_method_wait().
Is there any reason we couldn't have the implementation of
invoke_method_wait be implemented as a call to invoke_method followed by
_poll?
Hi Tony,

Sorry, I should explained them in patch comments.

Some methods[1] in access group create/delete/add/etc require extracting
CIM_XXX from InvokeMethod which is not possible(in a easy way) for
current SmisCommon.invoke_method()

The difference between invoke_method() and invoke_method_wait():
1. The invoke_method() return (job_id, data).
The invoke_method_wait() return cim_xxx.path.
2. The invoke_method() does not wait ASYNC job.
The invoke_method_wait() wait ASYN job.

The invoke_method_wait() is not depend on smis.Smis.job_status()
method while _poll() do. The job_status() cannot return a
cim_xxx.path, hence we need a dedicate method.

[1] Example: When adding initiator to access group, we have to create
CIM_StorageHardwareID via CreateStorageHardwareID() which
should be wait and get the newly created CIM_StorageHardwareID
instance.
Post by Tony Asleson
My main issue is we have invoke_method which will retrieve debug data if
requested, but invoke_method_wait does not. Having two similar but
different code paths results in bugs like this which I would like to avoid.
I will add debug data feature into invoke_method_wait() sharing codes
with invoke_method().
Post by Tony Asleson
Note: In future, move in one commit, then rename methods moved in
another commit.
Noted.
Post by Tony Asleson
More comments below.
Thanks for the detail review.
Post by Tony Asleson
Thanks,
Tony
--
Gris Ge
Gris Ge
2014-11-10 15:17:11 UTC
Permalink
* Replace smis.Smis._cim_init_path_check_or_create() with
smis_ag.cim_init_path_check_or_create

* Replace smis.Smis._init_id() with smis_ag.init_id_of_cim_init()

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 59 ++++++++----------------------------------------
plugin/smispy/smis_ag.py | 51 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 1847e07..6143727 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -362,12 +362,6 @@ def _sys_id_child(self, cim_xxx):
"""
return self._id('SystemChild', cim_xxx)

- def _init_id(self, cim_init):
- """
- Retrieve Initiator ID from CIM_StorageHardwareID
- """
- return self._id('Initiator', cim_init)
-
def _id(self, class_type, cim_xxx):
"""
Return the ID of certain class.
@@ -1389,41 +1383,6 @@ def access_groups(self, search_key=None, search_value=None, flags=0):

return search_property(rc, search_key, search_value)

- def _cim_init_path_check_or_create(self, system_id, init_id, init_type):
- """
- Check whether CIM_StorageHardwareID exists, if not, create new one.
- """
- cim_init = self._get_cim_instance_by_id(
- 'Initiator', init_id, raise_error=False)
-
- if cim_init:
- return cim_init.path
-
- # Create new one
- dmtf_id_type = _lsm_init_type_to_dmtf(init_type)
- if dmtf_id_type is None:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "SMI-S Plugin does not support init_type %d" %
- init_type)
-
- return self._cim_init_path_create(system_id, init_id, dmtf_id_type)
-
- def _cim_init_path_create(self, system_id, init_id, dmtf_id_type):
- """
- Create a CIM_StorageHardwareID.
- Return CIMInstanceName
- Raise error if failed. Return if pass.
- """
- cim_hwms = self._c.cim_hwms_of_sys_id(system_id)
-
- in_params = {'StorageID': init_id,
- 'IDType': pywbem.Uint16(dmtf_id_type)}
-
- # CreateStorageHardwareID does not allow ASYNC
- return self._c.invoke_method_wait(
- 'CreateStorageHardwareID', cim_hwms.path, in_params,
- out_key='HardwareID', expect_class='CIM_StorageHardwareID')
-
def _ag_init_add_group(self, access_group, init_id, init_type):
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)

@@ -1441,11 +1400,11 @@ def _ag_init_add_group(self, access_group, init_id, init_type):

# Check whether already added.
for exist_cim_init in exist_cim_inits:
- if self._init_id(exist_cim_init) == init_id:
+ if smis_ag.init_id_of_cim_init(exist_cim_init) == init_id:
return copy.deepcopy(access_group)

- cim_init_path = self._cim_init_path_check_or_create(
- access_group.system_id, init_id, init_type)
+ cim_init_path = smis_ag.cim_init_path_check_or_create(
+ self._c, access_group.system_id, init_id, init_type)

cim_gmms = self._c.cim_gmms_of_sys_id(access_group.system_id)

@@ -1494,14 +1453,14 @@ def _ag_init_add_old(self, access_group, init_id, init_type):
self._c, cim_spc_path)

for exist_cim_init in exist_cim_inits:
- if self._init_id(exist_cim_init) == init_id:
+ if smis_ag.init_id_of_cim_init(exist_cim_init) == init_id:
return copy.deepcopy(access_group)

# Check to see if we have this initiator already, if not we
# create it and then add to the view.

- self._cim_init_path_check_or_create(
- access_group.system_id, init_id, init_type)
+ smis_ag.cim_init_path_check_or_create(
+ self._c, access_group.system_id, init_id, init_type)

cim_ccs = self._c.cim_ccs_of_sys_id(access_group.system_id)

@@ -1531,7 +1490,7 @@ def _ag_init_del_group(self, access_group, init_id):

cim_init = None
for cur_cim_init in cur_cim_inits:
- if self._init_id(cur_cim_init) == init_id:
+ if smis_ag.init_id_of_cim_init(cur_cim_init) == init_id:
cim_init = cur_cim_init
break

@@ -2092,8 +2051,8 @@ def access_group_create(self, name, init_id, init_type, system,
"iSCSI target port, which not allow creating "
"iSCSI IQN access group")

- cim_init_path = self._cim_init_path_check_or_create(
- system.id, init_id, init_type)
+ cim_init_path = smis_ag.cim_init_path_check_or_create(
+ self._c, system.id, init_id, init_type)

# Create CIM_InitiatorMaskingGroup
cim_gmms = self._c.cim_gmms_of_sys_id(system.id)
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index 3c125ef..9fd6a50 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -36,10 +36,10 @@ def _cim_inits_to_lsm_init_id_and_type(cim_inits):
init_types = []
for cim_init in cim_inits:
if cim_init['IDType'] == dmtf.ID_TYPE_WWPN:
- init_ids.append(cim_init['StorageID'])
+ init_ids.append(init_id_of_cim_init(cim_init))
init_types.append(AccessGroup.INIT_TYPE_WWPN)
if cim_init['IDType'] == dmtf.ID_TYPE_ISCSI:
- init_ids.append(cim_init['StorageID'])
+ init_ids.append(init_id_of_cim_init(cim_init))
init_types.append(AccessGroup.INIT_TYPE_ISCSI_IQN)
# Skip if not a iscsi initiator IQN or WWPN.
continue
@@ -193,3 +193,50 @@ def lsm_ag_to_cim_init_mg_path(smis_common, lsm_ag):
caller should make sure that.
"""
return lsm_ag_to_cim_spc_path(smis_common, lsm_ag)
+
+
+def init_id_of_cim_init(cim_init):
+ """
+ Return CIM_StorageHardwareID['StorageID']
+ """
+ if 'StorageID' in cim_init:
+ return cim_init['StorageID']
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "init_id_of_cim_init() got cim_init without 'StorageID' %s: %s" %
+ cim_init.path, cim_init.items())
+
+
+def cim_init_path_check_or_create(smis_common, system_id, init_id, init_type):
+ """
+ Check whether CIM_StorageHardwareID exists, if not, create new one.
+ """
+ cim_inits = smis_common.EnumerateInstances(
+ 'CIM_StorageHardwareID',
+ PropertyList=_CIM_INIT_PROS)
+
+ if len(cim_inits):
+ for cim_init in cim_inits:
+ if init_id_of_cim_init(cim_init) == init_id:
+ return cim_init.path
+
+ # Create new one
+ dmtf_id_type = None
+ if init_type == AccessGroup.INIT_TYPE_WWPN:
+ dmtf_id_type = dmtf.ID_TYPE_WWPN
+ elif init_type == AccessGroup.INIT_TYPE_ISCSI_IQN:
+ dmtf_id_type = dmtf.ID_TYPE_ISCSI
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "cim_init_path_check_or_create(): Got invalid init_type: %d" %
+ init_type)
+
+ cim_hwms = smis_common.cim_hwms_of_sys_id(system_id)
+ in_params = {
+ 'StorageID': init_id,
+ 'IDType': dmtf_id_type,
+ }
+ return smis_common.invoke_method_wait(
+ 'CreateStorageHardwareID', cim_hwms.path, in_params,
+ out_key='HardwareID', expect_class='CIM_StorageHardwareID')
--
1.8.3.1


------------------------------------------------------------------------------
Gris Ge
2014-11-10 15:17:12 UTC
Permalink
* Removed these methods from smis.py since not used by others:
* _lsm_init_type_to_dmtf()
* _get_cim_instance_by_id()
* _cim_class_name_of()
* _not_found_error_of_class()
* _property_list_of_id()
* _sys_id_child()
* _id()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 6143727..7967544 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -114,15 +114,6 @@ def _lsm_tgt_port_type_of_cim_fc_tgt(cim_fc_tgt):
return TargetPort.TYPE_FC


-def _lsm_init_type_to_dmtf(init_type):
- if init_type == AccessGroup.INIT_TYPE_WWPN:
- return dmtf.ID_TYPE_WWPN
- if init_type == AccessGroup.INIT_TYPE_ISCSI_IQN:
- return dmtf.ID_TYPE_ISCSI
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Does not support provided init_type: %d" % init_type)
-
-
class Smis(IStorageAreaNetwork):
"""
SMI-S plug-ing which exposes a small subset of the overall provided
@@ -137,37 +128,6 @@ def __init__(self):
self._c = None
self.tmo = 0

- def _get_cim_instance_by_id(self, class_type, requested_id,
- property_list=None, raise_error=True):
- """
- Find out the CIM_XXXX Instance which holding the requested_id
- Return None when error and raise_error is False
- """
- class_name = Smis._cim_class_name_of(class_type)
- error_numer = Smis._not_found_error_of_class(class_type)
- id_pros = Smis._property_list_of_id(class_type, property_list)
-
- if property_list is None:
- property_list = id_pros
- else:
- property_list = merge_list(property_list, id_pros)
-
- cim_xxxs = self._c.EnumerateInstances(
- class_name, PropertyList=property_list)
- org_requested_id = requested_id
- if class_type == 'Job':
- (requested_id, ignore, ignore) = SmisCommon.parse_job_id(
- requested_id)
- for cim_xxx in cim_xxxs:
- if self._id(class_type, cim_xxx) == requested_id:
- return cim_xxx
- if raise_error is False:
- return None
-
- raise LsmError(error_numer,
- "Cannot find %s Instance with " % class_name +
- "%s ID '%s'" % (class_type, org_requested_id))
-
@handle_cim_errors
def plugin_register(self, uri, password, timeout, flags=0):
"""
@@ -296,105 +256,6 @@ def job_status(self, job_id, flags=0):

return status, percent_complete, completed_item

- @staticmethod
- def _cim_class_name_of(class_type):
- if class_type == 'Volume':
- return 'CIM_StorageVolume'
- if class_type == 'Pool':
- return 'CIM_StoragePool'
- if class_type == 'Disk':
- return 'CIM_DiskDrive'
- if class_type == 'Job':
- return 'CIM_ConcreteJob'
- if class_type == 'AccessGroup':
- return 'CIM_SCSIProtocolController'
- if class_type == 'Initiator':
- return 'CIM_StorageHardwareID'
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Smis._cim_class_name_of() got unknown " +
- "class_type %s" % class_type)
-
- @staticmethod
- def _not_found_error_of_class(class_type):
- if class_type == 'Volume':
- return ErrorNumber.NOT_FOUND_VOLUME
- if class_type == 'Pool':
- return ErrorNumber.NOT_FOUND_POOL
- if class_type == 'Job':
- return ErrorNumber.NOT_FOUND_JOB
- if class_type == 'AccessGroup':
- return ErrorNumber.NOT_FOUND_ACCESS_GROUP
- if class_type == 'Initiator':
- return ErrorNumber.INVALID_ARGUMENT
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Smis._cim_class_name_of() got unknown " +
- "class_type %s" % class_type)
-
- @staticmethod
- def _property_list_of_id(class_type, extra_properties=None):
- """
- Return a PropertyList which the ID of current class is basing on
- """
- rc = []
- if class_type == 'Volume':
- rc = ['SystemName', 'DeviceID']
- elif class_type == 'SystemChild':
- rc = ['SystemName']
- elif class_type == 'Disk':
- rc = ['SystemName', 'DeviceID']
- elif class_type == 'Job':
- rc = ['InstanceID']
- elif class_type == 'Initiator':
- rc = ['StorageID']
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Smis._property_list_of_id() got unknown " +
- "class_type %s" % class_type)
-
- if extra_properties:
- rc = merge_list(rc, extra_properties)
- return rc
-
- def _sys_id_child(self, cim_xxx):
- """
- Find out the system id of Pool/Volume/Disk/AccessGroup/Initiator
- Currently, we just use SystemName of cim_xxx
- """
- return self._id('SystemChild', cim_xxx)
-
- def _id(self, class_type, cim_xxx):
- """
- Return the ID of certain class.
- When ID is based on two or more properties, we use MD5 hash of them.
- If not, return the property value.
- """
- property_list = Smis._property_list_of_id(class_type)
- for key in property_list:
- if key not in cim_xxx:
- cim_xxx = self._c.GetInstance(cim_xxx.path,
- PropertyList=property_list,
- LocalOnly=False)
- break
-
- id_str = ''
- for key in property_list:
- if key not in cim_xxx:
- cim_class_name = ''
- if class_type == 'SystemChild':
- cim_class_name = str(cim_xxx.classname)
- else:
- cim_class_name = Smis._cim_class_name_of(class_type)
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "%s %s " % (cim_class_name, cim_xxx.path) +
- "does not have property %s " % str(key) +
- "calculate out %s id" % class_type)
- else:
- id_str += cim_xxx[key]
- if len(property_list) == 1 and class_type != 'Job':
- return id_str
- else:
- return md5(id_str)
-
def _new_vol_from_name(self, out):
"""
Given a volume by CIMInstanceName, return a lsm Volume object
--
1.8.3.1


------------------------------------------------------------------------------
Gris Ge
2014-11-10 15:17:13 UTC
Permalink
* Replace smis._lsm_init_id_to_snia() with smis_ag.lsm_init_id_to_snia()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 7967544..2f6dbd3 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -85,18 +85,6 @@
# Group M&M SNIA SMI-S 'Group Masking and Mapping' profile


-def _lsm_init_id_to_snia(lsm_init_id):
- """
- If lsm_init_id is a WWPN, convert it to SNIA format:
- [0-9A-F]{16}
- If not, return directly.
- """
- val, init_type, init_id = AccessGroup.initiator_id_verify(lsm_init_id)
- if val and init_type == AccessGroup.INIT_TYPE_WWPN:
- return lsm_init_id.replace(':', '').upper()
- return lsm_init_id
-
-
def _lsm_tgt_port_type_of_cim_fc_tgt(cim_fc_tgt):
"""
We are assuming we got CIM_FCPort. Caller should make sure of that.
@@ -1287,7 +1275,7 @@ def _ag_init_add_group(self, access_group, init_id, init_type):
@handle_cim_errors
def access_group_initiator_add(self, access_group, init_id, init_type,
flags=0):
- init_id = _lsm_init_id_to_snia(init_id)
+ init_id = smis_ag.lsm_init_id_to_snia(init_id)
mask_type = smis_cap.mask_type(self._c, raise_error=True)

if mask_type == smis_cap.MASK_TYPE_GROUP:
@@ -1392,7 +1380,7 @@ def access_group_initiator_delete(self, access_group, init_id, init_type,
raise LsmError(ErrorNumber.NO_SUPPORT,
"SMI-S plugin does not support "
"access_group_initiator_delete() against NetApp-E")
- init_id = _lsm_init_id_to_snia(init_id)
+ init_id = smis_ag.lsm_init_id_to_snia(init_id)
mask_type = smis_cap.mask_type(self._c, raise_error=True)

if mask_type == smis_cap.MASK_TYPE_GROUP:
@@ -1875,7 +1863,7 @@ def access_group_create(self, name, init_id, init_type, system,
1. Create CIM_InitiatorMaskingGroup
"""
org_init_id = init_id
- init_id = _lsm_init_id_to_snia(init_id)
+ init_id = smis_ag.lsm_init_id_to_snia(init_id)

self._c.profile_check(SmisCommon.SNIA_GROUP_MASK_PROFILE,
SmisCommon.SMIS_SPEC_VER_1_5,
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index 9fd6a50..8bd44de 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -207,6 +207,18 @@ def init_id_of_cim_init(cim_init):
cim_init.path, cim_init.items())


+def lsm_init_id_to_snia(lsm_init_id):
+ """
+ If lsm_init_id is a WWPN, convert it to SNIA format:
+ [0-9A-F]{16}
+ If not, return original directly.
+ """
+ val, init_type, init_id = AccessGroup.initiator_id_verify(lsm_init_id)
+ if val and init_type == AccessGroup.INIT_TYPE_WWPN:
+ return lsm_init_id.replace(':', '').upper()
+ return lsm_init_id
+
+
def cim_init_path_check_or_create(smis_common, system_id, init_id, init_type):
"""
Check whether CIM_StorageHardwareID exists, if not, create new one.
--
1.8.3.1


------------------------------------------------------------------------------
Gris Ge
2014-11-10 15:17:14 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.

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 2f6dbd3..d1fa370 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -799,8 +799,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)

@@ -831,6 +829,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)
@@ -845,18 +846,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',
@@ -895,6 +894,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)
@@ -914,7 +922,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(
@@ -943,59 +950,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 already unmasked from 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')
+
+ if len(ag_cim_spcs_path) == 0:
+ # Already unmasked
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Volume already unmasked from 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 already unmasked from 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

@@ -1014,12 +1036,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 already unmasked from requested access group")
+
hide_params = {'LUNames': [cim_vol['Name']],
'ProtocolControllers': [cim_spc_path]}

@@ -1106,19 +1138,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 8bd44de..b5df456 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -252,3 +252,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-10 20:28:52 UTC
Permalink
Post by Gris Ge
* Raise LsmError ErrorNumber.NO_STATE_CHANGE in
volume_mask and volume_unmask if already masked or not masked.
I don't think we are testing for this in plugin_test.py. Please add if
we are missing so that we can ensure all plug-ins are behaving the same.

Thanks,
Tony
Gris Ge
2014-11-10 15:17:15 UTC
Permalink
* Previously, SMI-S plugin treat duplicate call of access_group_create()
as silently pass, as API defecation, we should raise
ErrorNumber.EXISTS_INITIATOR even owner access group name is the same as
requested.

* Tested on EMC VMAX for both NAME_CONFLICT and EXISTS_INITIATOR error.

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index d1fa370..c36d46d 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -1947,26 +1947,18 @@ def access_group_create(self, name, init_id, init_type, system,
# Check possible failure
# 1. Initiator already exist in other group.
# If that group hold the same name as requested.
- # We consider as a duplicate call, return the exist one.
- exist_cim_init_mgs = self._c.Associators(
+ # We consider as a duplicate call.
+ exist_cim_init_mg_paths = self._c.AssociatorNames(
cim_init_path,
AssocClass='CIM_MemberOfCollection',
- ResultClass='CIM_InitiatorMaskingGroup',
- PropertyList=cim_init_mg_pros)
-
- if len(exist_cim_init_mgs) != 0:
- for exist_cim_init_mg in exist_cim_init_mgs:
- if exist_cim_init_mg['ElementName'] == name:
- return smis_ag.cim_init_mg_to_lsm_ag(
- self._c, exist_cim_init_mg, system.id)
+ ResultClass='CIM_InitiatorMaskingGroup')

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

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

@handle_cim_errors
--
1.8.3.1


------------------------------------------------------------------------------
Gris Ge
2014-11-10 15:17:16 UTC
Permalink
* Raise NO_STATE_CHANGE or EXISTS_INITIATOR as API document defined
for access_group_initiator_add():
NO_STATE_CHANGE: Requested initiator already in defined access group.
EXISTS_INITIATOR: Requested initiator is in other access group.
* Use new way for error detecting:
1. Check only when error found, pre-check is not needed here.
2. Use smis_ag.cim_init_mg_path_of_cim_init_path() and
smis_ag.cim_init_mg_path_of_cim_init_path() to identify
the root cause more quickly.
* Tested on:
1. HP 3PAR the only array I found supporting access_group_initiator_add()
via SNIA "Masking and Mapping" profile.
2. EMC VMAX the only array I found supporting access_group_initiator_add()
via SNIA "Group Masking and Mapping" profile.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 74 ++++++++++++++++++++++++---------------
plugin/smispy/smis_ag.py | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 137 insertions(+), 28 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index c36d46d..52fb31b 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -1270,14 +1270,6 @@ def _ag_init_add_group(self, access_group, init_id, init_type):
cim_init_mg_path = smis_ag.lsm_ag_to_cim_init_mg_path(
self._c, access_group)

- exist_cim_inits = smis_ag.cim_init_of_cim_init_mg_path(
- self._c, cim_init_mg_path)
-
- # Check whether already added.
- for exist_cim_init in exist_cim_inits:
- if smis_ag.init_id_of_cim_init(exist_cim_init) == init_id:
- return copy.deepcopy(access_group)
-
cim_init_path = smis_ag.cim_init_path_check_or_create(
self._c, access_group.system_id, init_id, init_type)

@@ -1288,15 +1280,31 @@ def _ag_init_add_group(self, access_group, init_id, init_type):
'Members': [cim_init_path],
}

- new_cim_init_mg_path = self._c.invoke_method_wait(
- 'AddMembers', cim_gmms.path, in_params,
- out_key='MaskingGroup', expect_class='CIM_InitiatorMaskingGroup')
+ try:
+ self._c.invoke_method_wait(
+ 'AddMembers', cim_gmms.path, in_params,
+ out_key='MaskingGroup',
+ expect_class='CIM_InitiatorMaskingGroup')
+ except (LsmError, CIMError):
+ exist_cim_init_mg_path = \
+ smis_ag.cim_init_mg_path_of_cim_init_path(
+ self._c, cim_init_path)
+ if exist_cim_init_mg_path == cim_init_mg_path:
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "Initiator already in requested access group")
+ elif exist_cim_init_mg_path:
+ raise LsmError(
+ ErrorNumber.EXISTS_INITIATOR,
+ "Initiator used by other access group")
+ else:
+ raise
+
cim_init_mg_pros = smis_ag.cim_init_mg_pros()
- new_cim_init_mg = self._c.GetInstance(
- new_cim_init_mg_path, PropertyList=cim_init_mg_pros,
- LocalOnly=False)
+ cim_init_mg = self._c.GetInstance(
+ cim_init_mg_path, PropertyList=cim_init_mg_pros)
return smis_ag.cim_init_mg_to_lsm_ag(
- self._c, new_cim_init_mg, access_group.system_id)
+ self._c, cim_init_mg, access_group.system_id)

@handle_cim_errors
def access_group_initiator_add(self, access_group, init_id, init_type,
@@ -1324,17 +1332,9 @@ def _ag_init_add_old(self, access_group, init_id, init_type):
cim_spc_path = smis_ag.lsm_ag_to_cim_spc_path(
self._c, access_group)

- exist_cim_inits = smis_ag.cim_init_of_cim_spc_path(
- self._c, cim_spc_path)
-
- for exist_cim_init in exist_cim_inits:
- if smis_ag.init_id_of_cim_init(exist_cim_init) == init_id:
- return copy.deepcopy(access_group)
-
# Check to see if we have this initiator already, if not we
# create it and then add to the view.
-
- smis_ag.cim_init_path_check_or_create(
+ cim_init_path = smis_ag.cim_init_path_check_or_create(
self._c, access_group.system_id, init_id, init_type)

cim_ccs = self._c.cim_ccs_of_sys_id(access_group.system_id)
@@ -1342,10 +1342,28 @@ def _ag_init_add_old(self, access_group, init_id, init_type):
in_params = {'InitiatorPortIDs': [init_id],
'ProtocolControllers': [cim_spc_path]}

- cim_spc_path = self._c.invoke_method_wait(
- 'ExposePaths', cim_ccs.path, in_params,
- out_key='ProtocolControllers', flag_out_array=True,
- expect_class='CIM_SCSIProtocolController')
+ try:
+ cim_spc_path = self._c.invoke_method_wait(
+ 'ExposePaths', cim_ccs.path, in_params,
+ out_key='ProtocolControllers', flag_out_array=True,
+ expect_class='CIM_SCSIProtocolController')
+
+ except (LsmError, CIMError):
+ # Post-check:
+ # 1. Already exists in requested access group.
+ # 2. Already exists in other access group.
+ exist_cim_spc = smis_ag.cim_spc_path_of_cim_init_path(
+ self._c, cim_init_path)
+ if exist_cim_spc:
+ if exist_cim_spc == cim_spc_path:
+ raise LsmError(
+ ErrorNumber.NO_STATE_CHANGE,
+ "initiator already in requested access group")
+ else:
+ raise LsmError(
+ ErrorNumber.EXISTS_INITIATOR,
+ "initiator used by other access group")
+

cim_spc_pros = smis_ag.cim_spc_pros()
cim_spc = self._c.GetInstance(
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index b5df456..bc114d4 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -73,6 +73,97 @@ def cim_init_mg_pros():
return ['ElementName', 'InstanceID']


+def cim_spc_path_of_cim_init_path(smis_common, cim_init_path):
+ """
+ Return CIMInstanceName of CIM_SCSIProtocolController which request
+ cim_init associated. As SNIA defined, one cim_init can only associatored
+ to one cim_spc.
+ SNIA 1.6 first:
+ CIM_SCSIProtocolController
+ |
+ | CIM_AssociatedPrivilege
+ v
+ CIM_StorageHardwareID
+ SNIA 1.4, 1.5:
+ CIM_SCSIProtocolController
+ |
+ | CIM_AuthorizedTarget
+ v
+ CIM_AuthorizedPrivilege
+ |
+ | CIM_AuthorizedSubject
+ v
+ CIM_StorageHardwareID
+ Return None if not
+ """
+ cim_spc_paths = []
+ if smis_common.profile_check(SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_6,
+ raise_error=False):
+ try:
+ cim_spc_paths = smis_common.AssociatorNames(
+ cim_init_path,
+ AssocClass='CIM_AssociatedPrivilege',
+ ResultClass='CIM_SCSIProtocolController')
+ except CIMError as ce:
+ if ce[0] == CIM_ERR_NOT_FOUND:
+ pass
+ else:
+ raise
+ if len(cim_spc_paths) == 0:
+ cim_aps_path = smis_common.AssociatorNames(
+ cim_init_path,
+ AssocClass='CIM_AuthorizedSubject',
+ ResultClass='CIM_AuthorizedPrivilege')
+ for cim_ap_path in cim_aps_path:
+ cim_spc_paths.extend(smis_common.AssociatorNames(
+ cim_ap_path,
+ AssocClass='CIM_AuthorizedTarget',
+ ResultClass='CIM_SCSIProtocolController'))
+
+ if len(cim_spc_paths) == 1:
+ return cim_spc_paths[0]
+ elif len(cim_spc_paths) > 1:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "cim_spc_path_of_cim_init_path(): Got %d cim_spc assocated to " %
+ len(cim_spc_paths) +
+ "cim_init %s: %s" % (cim_init_path, cim_spc_paths))
+ else:
+ return None
+
+
+def cim_init_mg_path_of_cim_init_path(smis_common, cim_init_path):
+ """
+ Return CIMInstanceName of CIM_InitiatorMaskingGroup associated by
+ cim_init.
+ As cim_init to cim_spc is one-to-one map, and cim_spc to cim_init_mg
+ is one-to-one map, hence cim_init to cim_init_mg is one-to-one map.
+ Use this association:
+ CIM_StorageHardwareID
+ |
+ | CIM_MemberOfCollection
+ v
+ CIM_InitiatorMaskingGroup
+ If not found, return None
+ """
+ cim_init_mg_paths = smis_common.AssociatorNames(
+ cim_init_path,
+ AssocClass='CIM_MemberOfCollection',
+ ResultClass='CIM_InitiatorMaskingGroup')
+ if len(cim_init_mg_paths) == 1:
+ return cim_init_mg_paths[0]
+ elif len(cim_init_mg_paths) > 1:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "cim_init_mg_path_of_cim_init_path(): Got %d cim_init_mg " %
+ len(cim_init_mg_paths) +
+ "assocated to cim_init %s: %s" %
+ (cim_init_path, cim_init_mg_paths))
+ else:
+ return None
+
+
def cim_init_of_cim_spc_path(smis_common, cim_spc_path):
"""
Return a list of CIM_StorageHardwareID associated to cim_spc.
--
1.8.3.1


------------------------------------------------------------------------------
Gris Ge
2014-11-20 06:58:57 UTC
Permalink
* Move smis.Smis._job_completed_ok() to
smis_common.SmisCommon.cim_job_completed_ok()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 189289e..dd662db 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -241,25 +241,6 @@ def capabilities(self, system, flags=0):
def plugin_info(self, flags=0):
return "Generic SMI-S support", VERSION

- @staticmethod
- def _job_completed_ok(status):
- """
- Given a concrete job instance, check the operational status. This
- is a little convoluted as different SMI-S proxies return the values in
- different positions in list :-)
- """
- rc = False
- op = status['OperationalStatus']
-
- if (len(op) > 1 and
- ((op[0] == dmtf.OP_STATUS_OK and
- op[1] == dmtf.OP_STATUS_COMPLETED) or
- (op[0] == dmtf.OP_STATUS_COMPLETED and
- op[1] == dmtf.OP_STATUS_OK))):
- rc = True
-
- return rc
-
@handle_cim_errors
def job_status(self, job_id, flags=0):
"""
@@ -298,7 +279,7 @@ def job_status(self, job_id, flags=0):
status = JobStatus.COMPLETE
percent_complete = 100

- if Smis._job_completed_ok(cim_job):
+ if SmisCommon.cim_job_completed_ok(cim_job):
if retrieve_data == SmisCommon.JOB_RETRIEVE_VOLUME or \
retrieve_data == SmisCommon.JOB_RETRIEVE_VOLUME_CREATE:
completed_item = self._new_vol_from_job(cim_job)
diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
index 5343865..bf5dc89 100644
--- a/plugin/smispy/smis_common.py
+++ b/plugin/smispy/smis_common.py
@@ -524,3 +524,22 @@ def cim_hwms_of_sys_id(self, sys_id, raise_error=True):
"""
return self._cim_srv_of_sys_id(
'CIM_StorageHardwareIDManagementService', sys_id, raise_error)
+
+ @staticmethod
+ def cim_job_completed_ok(status):
+ """
+ Given a concrete job instance, check the operational status. This
+ is a little convoluted as different SMI-S proxies return the values in
+ different positions in list :-)
+ """
+ rc = False
+ op = status['OperationalStatus']
+
+ if (len(op) > 1 and
+ ((op[0] == dmtf.OP_STATUS_OK and
+ op[1] == dmtf.OP_STATUS_COMPLETED) or
+ (op[0] == dmtf.OP_STATUS_COMPLETED and
+ op[1] == dmtf.OP_STATUS_OK))):
+ rc = True
+
+ return rc
--
1.8.3.1
Gris Ge
2014-11-20 06:58:58 UTC
Permalink
* Move WBEM xml dump code to smis_common.SmisCommon._dump_wbem_xml() method.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis_common.py | 39 ++++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
index bf5dc89..b1fb54d 100644
--- a/plugin/smispy/smis_common.py
+++ b/plugin/smispy/smis_common.py
@@ -371,6 +371,27 @@ def parse_job_id(job_id):
method_data = tmp_list[2]
return (md5_str, retrieve_data, method_data)

+ def _dump_wbem_xml(self, file_prefix):
+ """
+ When debugging issues with providers it's helpful to have
+ the xml request/reply to give to provider developers.
+ """
+ if self._debug_path is not None:
+ if not os.path.exists(self._debug_path):
+ os.makedirs(self._debug_path)
+
+ if os.path.isdir(self._debug_path):
+ debug_fn = "%s_%s" % (
+ file_prefix, datetime.datetime.now().isoformat())
+ debug_full = os.path.join(
+ self._debug_path, debug_fn)
+
+ # Dump the request & reply to a file
+ with open(debug_full, 'w') as d:
+ d.write("REQ:\n%s\n\nREPLY:\n%s\n" %
+ (self.last_request, self.last_reply))
+
+
def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
error_handler=None, retrieve_data=None,
method_data=None):
@@ -421,24 +442,8 @@ def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
ErrorNumber.NO_SUPPORT,
'SMI-S error code indicates operation not supported')
else:
- # When debugging issues with providers it's helpful to have
- # the xml request/reply to give to provider developers.
try:
- if self._debug_path is not None:
- if not os.path.exists(self._debug_path):
- os.makedirs(self._debug_path)
-
- if os.path.isdir(self._debug_path):
- debug_fn = "%s_%s" % (
- cmd, datetime.datetime.now().isoformat())
- debug_full = os.path.join(
- self._debug_path, debug_fn)
-
- # Dump the request & reply to a file
- with open(debug_full, 'w') as d:
- d.write("REQ:\n%s\n\nREPLY:\n%s\n" %
- (self.last_request, self.last_reply))
-
+ self._dump_wbem_xml(cmd)
except Exception:
tb = traceback.format_exc()
raise LsmError(ErrorNumber.PLUGIN_BUG,
--
1.8.3.1
Gris Ge
2014-11-20 06:59:00 UTC
Permalink
* Replace smis.Smis._init_id() with smis_ag.init_id_of_cim_init()

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 12 +++---------
plugin/smispy/smis_ag.py | 16 ++++++++++++++--
2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 7ab9cb3..57daac0 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -361,12 +361,6 @@ def _sys_id_child(self, cim_xxx):
"""
return self._id('SystemChild', cim_xxx)

- def _init_id(self, cim_init):
- """
- Retrieve Initiator ID from CIM_StorageHardwareID
- """
- return self._id('Initiator', cim_init)
-
def _id(self, class_type, cim_xxx):
"""
Return the ID of certain class.
@@ -1448,7 +1442,7 @@ def _ag_init_add_group(self, access_group, init_id, init_type):

# Check whether already added.
for exist_cim_init in exist_cim_inits:
- if self._init_id(exist_cim_init) == init_id:
+ if smis_ag.init_id_of_cim_init(exist_cim_init) == init_id:
return copy.deepcopy(access_group)

cim_init_path = self._cim_init_path_check_or_create(
@@ -1501,7 +1495,7 @@ def _ag_init_add_old(self, access_group, init_id, init_type):
self._c, cim_spc_path)

for exist_cim_init in exist_cim_inits:
- if self._init_id(exist_cim_init) == init_id:
+ if smis_ag.init_id_of_cim_init(exist_cim_init) == init_id:
return copy.deepcopy(access_group)

# Check to see if we have this initiator already, if not we
@@ -1538,7 +1532,7 @@ def _ag_init_del_group(self, access_group, init_id):

cim_init = None
for cur_cim_init in cur_cim_inits:
- if self._init_id(cur_cim_init) == init_id:
+ if smis_ag.init_id_of_cim_init(cur_cim_init) == init_id:
cim_init = cur_cim_init
break

diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index 42d27b4..e93956d 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -41,10 +41,10 @@ def _init_id_and_type_of(cim_inits):
init_types = []
for cim_init in cim_inits:
if cim_init['IDType'] == dmtf.ID_TYPE_WWPN:
- init_ids.append(cim_init['StorageID'])
+ init_ids.append(init_id_of_cim_init(cim_init))
init_types.append(AccessGroup.INIT_TYPE_WWPN)
if cim_init['IDType'] == dmtf.ID_TYPE_ISCSI:
- init_ids.append(cim_init['StorageID'])
+ init_ids.append(init_id_of_cim_init(cim_init))
init_types.append(AccessGroup.INIT_TYPE_ISCSI_IQN)
# Skip if not a iscsi initiator IQN or WWPN.
continue
@@ -203,3 +203,15 @@ def lsm_ag_to_cim_init_mg_path(smis_common, lsm_ag):
caller should make sure that.
"""
return lsm_ag_to_cim_spc_path(smis_common, lsm_ag)
+
+
+def init_id_of_cim_init(cim_init):
+ """
+ Return CIM_StorageHardwareID['StorageID']
+ """
+ if 'StorageID' in cim_init:
+ return cim_init['StorageID']
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "init_id_of_cim_init() got cim_init without 'StorageID' %s: %s" %
+ (cim_init.path, cim_init.items()))
--
1.8.3.1
Gris Ge
2014-11-20 06:58:59 UTC
Permalink
* Replaced Smis._wait_invoke() with SmisCommon.invoke_method_wait().
* Removed Smis._poll() and replace it with SmisCommon.invoke_method_wait().
* Tested on EMC VMAX for access group create/delete, volume mask/unmask,
volume create/delete.

Changes in V2:
* Support debug_path URI parameter in invoke_method_wait().
* Fix typo of utile and retrun.
* More explanation about why we keep invoke_method_wait() along with
invoke_method() in stead of using smis.Smis._poll():

* The SmisCommon.invoke_method_wait() method is different from
invoke_method():
1. The invoke_method() return job_id and require a output handler.
The invoke_method_wait() return a CIMInstanceName which require
key name for output data.
2. The invoke_method() does not wait ASYNC job.
The invoke_method_wait() wait ASYNC job and return CIMInstanceName
via association CIM_AffectedJobElement.

* The invoke_method_wait() was required for actions like this:
1. LSM API require action done on return. Example:
smis.Smis.volume_mask()
2. Inter-action. Example:
smis.Smis.access_group_initiator_add() require cim_init be created
before adding it into cim_spc.
The action of creating cim_init should be done before adding it
into cim_spc.

* The limitation of smis.Smis._poll() compare to invoke_method_wait():
1. _poll() require a job_id which add one extra WBEM call for
converting job id to cim_job.
2. _poll() cannot return CIMInstanceName via association of
CIM_AffectedJobElement.

* Code change status indicate we provide a slightly simpler way to do the
same thing:
147 insertions(+), 177 deletions(-)

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 222 +++++++++----------------------------------
plugin/smispy/smis_common.py | 102 ++++++++++++++++++++
2 files changed, 147 insertions(+), 177 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index dd662db..7ab9cb3 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -128,9 +128,6 @@ class Smis(IStorageAreaNetwork):
SMI-S plug-ing which exposes a small subset of the overall provided
functionality of SMI-S
"""
- _INVOKE_MAX_LOOP_COUNT = 60
- _INVOKE_CHECK_INTERVAL = 5
-
_JOB_ERROR_HANDLER = {
SmisCommon.JOB_RETRIEVE_VOLUME_CREATE:
smis_vol.volume_create_error_handler,
@@ -586,21 +583,6 @@ def volume_create(self, pool, volume_name, size_bytes, provisioning,
retrieve_data=SmisCommon.JOB_RETRIEVE_VOLUME_CREATE,
method_data=volume_name)

- def _poll(self, msg, job):
- if job:
- while True:
- (s, percent, i) = self.job_status(job)
-
- if s == JobStatus.INPROGRESS:
- time.sleep(0.25)
- elif s == JobStatus.COMPLETE:
- self.job_free(job)
- return i
- else:
- raise LsmError(
- ErrorNumber.PLUGIN_BUG,
- msg + ", job error code= " + str(s))
-
def _detach_netapp_e(self, vol, sync):
#Get the Configuration service for the system we are interested in.
cim_scs = self._c.cim_scs_of_sys_id(vol.system_id)
@@ -608,10 +590,8 @@ def _detach_netapp_e(self, vol, sync):
in_params = {'Operation': pywbem.Uint16(2),
'Synchronization': sync.path}

- job_id = self._c.invoke_method(
- 'ModifySynchronization', cim_scs.path, in_params)[0]
-
- self._poll("ModifySynchronization, detach", job_id)
+ self._c.invoke_method_wait(
+ 'ModifySynchronization', cim_scs.path, in_params)

def _detach(self, vol, sync):
if self._c.is_netappe():
@@ -623,10 +603,8 @@ def _detach(self, vol, sync):
in_params = {'Operation': pywbem.Uint16(8),
'Synchronization': sync.path}

- job_id = self._c.invoke_method(
- 'ModifyReplicaSynchronization', cim_rs.path, in_params)[0]
-
- self._poll("ModifyReplicaSynchronization, detach", job_id)
+ self._c.invoke_method_wait(
+ 'ModifyReplicaSynchronization', cim_rs.path, in_params)

@staticmethod
def _cim_name_match(a, b):
@@ -906,20 +884,15 @@ def _cim_dev_mg_path_create(self, cim_gmms_path, name, cim_vol_path,

cim_dev_mg_path = None
try:
- (rc, out) = self._c.InvokeMethod('CreateGroup', cim_gmms_path,
- **in_params)
- except CIMError as ce:
- if ce[0] == pywbem.CIM_ERR_FAILED:
- cim_dev_mg_path = self._check_exist_cim_dev_mg(
- name, cim_gmms_path, cim_vol_path, vol_id)
- if cim_dev_mg_path is None:
- raise
- else:
- raise
- if cim_dev_mg_path is None:
- cim_dev_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
+ cim_dev_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms_path, in_params,
+ out_key='MaskingGroup',
expect_class='CIM_TargetMaskingGroup')
+ except (LsmError, CIMError):
+ cim_dev_mg_path = self._check_exist_cim_dev_mg(
+ name, cim_gmms_path, cim_vol_path, vol_id)
+ if cim_dev_mg_path is None:
+ raise

return cim_dev_mg_path

@@ -954,21 +927,14 @@ def _cim_tgt_mg_path_create(self, cim_sys_path, cim_gmms_path, name,

cim_tgt_mg_path = None
try:
- (rc, out) = self._c.InvokeMethod('CreateGroup', cim_gmms_path,
- **in_params)
- except CIMError as ce:
- if ce[0] == pywbem.CIM_ERR_FAILED:
- cim_tgt_mg_path = self._check_exist_cim_tgt_mg(name)
- if cim_tgt_mg_path is None:
- raise
- else:
+ cim_tgt_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms_path, in_params,
+ out_key='MaskingGroup', expect_class='CIM_TargetMaskingGroup')
+ except (LsmError, CIMError):
+ cim_tgt_mg_path = self._check_exist_cim_tgt_mg(name)
+ if cim_tgt_mg_path is None:
raise

- if cim_tgt_mg_path is None:
- cim_tgt_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
- expect_class='CIM_TargetMaskingGroup')
-
return cim_tgt_mg_path

def _cim_spc_path_create(self, cim_gmms_path, cim_init_mg_path,
@@ -980,11 +946,9 @@ def _cim_spc_path_create(self, cim_gmms_path, cim_init_mg_path,
'DeviceMaskingGroup': cim_dev_mg_path,
}

- (rc, out) = self._c.InvokeMethod('CreateMaskingView', cim_gmms_path,
- **in_params)
-
- return self._wait_invoke(
- rc, out, out_key='ProtocolController',
+ return self._c.invoke_method_wait(
+ 'CreateMaskingView', cim_gmms_path, in_params,
+ out_key='ProtocolController',
expect_class='CIM_SCSIProtocolController')

def _volume_mask_group(self, access_group, volume, flags=0):
@@ -1066,10 +1030,8 @@ def _volume_mask_group(self, access_group, volume, flags=0):
'MaskingGroup': cim_dev_mg_path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'AddMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'AddMembers', cim_gmms.path, in_params)
return None

@handle_cim_errors
@@ -1107,10 +1069,7 @@ def _volume_mask_old(self, access_group, volume, flags):
'ProtocolControllers': [cim_spc_path],
'DeviceAccesses': [dmtf.CTRL_CONF_SRV_DA_RW]}

- (rc, out) = self._c.InvokeMethod(
- 'ExposePaths',
- cim_ccs.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('ExposePaths', cim_ccs.path, in_params)
return None

def _volume_unmask_group(self, access_group, volume):
@@ -1193,19 +1152,15 @@ def _volume_unmask_group(self, access_group, volume):
in_params = {
'ProtocolController': cim_spc_path,
}
- (rc, out) = self._c.InvokeMethod(
- 'DeleteMaskingView',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'DeleteMaskingView', cim_gmms.path, in_params)

in_params = {
'MaskingGroup': cim_dev_mg_path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'RemoveMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait(
+ 'RemoveMembers', cim_gmms.path, in_params)

return None

@@ -1233,10 +1188,7 @@ def _volume_unmask_old(self, access_group, volume):
hide_params = {'LUNames': [cim_vol['Name']],
'ProtocolControllers': [cim_spc_path]}

- (rc, out) = self._c.InvokeMethod('HidePaths', cim_ccs.path,
- **hide_params)
- self._wait_invoke(rc, out)
-
+ self._c.invoke_method_wait('HidePaths', cim_ccs.path, hide_params)
return None

def _is_access_group(self, cim_spc):
@@ -1474,12 +1426,10 @@ def _cim_init_path_create(self, system_id, init_id, dmtf_id_type):
in_params = {'StorageID': init_id,
'IDType': pywbem.Uint16(dmtf_id_type)}

- (rc, out) = self._c.InvokeMethod('CreateStorageHardwareID',
- cim_hwms.path, **in_params)
# CreateStorageHardwareID does not allow ASYNC
- return self._wait_invoke(
- rc, out, out_key='HardwareID',
- expect_class='CIM_StorageHardwareID')
+ return self._c.invoke_method_wait(
+ 'CreateStorageHardwareID', cim_hwms.path, in_params,
+ out_key='HardwareID', expect_class='CIM_StorageHardwareID')

def _ag_init_add_group(self, access_group, init_id, init_type):
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)
@@ -1510,12 +1460,10 @@ def _ag_init_add_group(self, access_group, init_id, init_type):
'MaskingGroup': cim_init_mg_path,
'Members': [cim_init_path],
}
- (rc, out) = self._c.InvokeMethod('AddMembers',
- cim_gmms.path, **in_params)

- new_cim_init_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
- expect_class='CIM_InitiatorMaskingGroup')
+ new_cim_init_mg_path = self._c.invoke_method_wait(
+ 'AddMembers', cim_gmms.path, in_params,
+ out_key='MaskingGroup', expect_class='CIM_InitiatorMaskingGroup')
cim_init_mg_pros = smis_ag.cim_init_mg_pros()
new_cim_init_mg = self._c.GetInstance(
new_cim_init_mg_path, PropertyList=cim_init_mg_pros,
@@ -1567,10 +1515,9 @@ def _ag_init_add_old(self, access_group, init_id, init_type):
in_params = {'InitiatorPortIDs': [init_id],
'ProtocolControllers': [cim_spc_path]}

- (rc, out) = self._c.InvokeMethod('ExposePaths',
- cim_ccs.path, **in_params)
- cim_spc_path = self._wait_invoke(
- rc, out, out_key='ProtocolControllers', flag_out_array=True,
+ cim_spc_path = self._c.invoke_method_wait(
+ 'ExposePaths', cim_ccs.path, in_params,
+ out_key='ProtocolControllers', flag_out_array=True,
expect_class='CIM_SCSIProtocolController')

cim_spc_pros = smis_ag.cim_spc_pros()
@@ -1613,10 +1560,7 @@ def _ag_init_del_group(self, access_group, init_id):
'Members': [cim_init.path],
}

- (rc, out) = self._c.InvokeMethod(
- 'RemoveMembers',
- cim_gmms.path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('RemoveMembers', cim_gmms.path, in_params)

cim_init_mg_pros = smis_ag.cim_init_mg_pros()
cim_init_mg = self._c.GetInstance(
@@ -1650,10 +1594,8 @@ def _ag_init_del_old(self, access_group, init_id):

hide_params = {'InitiatorPortIDs': [init_id],
'ProtocolControllers': [cim_spc_path]}
- (rc, out) = self._c.InvokeMethod(
- 'HidePaths', cim_ccs.path, **hide_params)
+ self._c.invoke_method_wait('HidePaths', cim_ccs.path, hide_params)

- self._wait_invoke(rc, out)
return None

@handle_cim_errors
@@ -2042,71 +1984,6 @@ def target_ports(self, search_key=None, search_value=None, flags=0):

return search_property(rc, search_key, search_value)

- def _wait_invoke(self, rc, out, out_key=None, expect_class=None,
- flag_out_array=False,):
- """
- Return out[out_key] if found rc == SmisCommon.SNIA_INVOKE_OK.
- For rc == SmisCommon.SNIA_INVOKE_ASYNC, we check every
- Smis._INVOKE_CHECK_INTERVAL
- seconds until done. Then return association via CIM_AffectedJobElement
- Return CIM_InstanceName
- Assuming only one CIM_InstanceName will get.
- """
- if rc == SmisCommon.SNIA_INVOKE_OK:
- if out_key is None:
- return None
- if out_key in out:
- if flag_out_array:
- if len(out[out_key]) != 1:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(), %s is not length 1: %s"
- % (out_key, out.items()))
- return out[out_key][0]
- return out[out_key]
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(), %s not exist in out %s" %
- (out_key, out.items()))
- elif rc == SmisCommon.SNIA_INVOKE_ASYNC:
- cim_job_path = out['Job']
- loop_counter = 0
- job_pros = ['JobState', 'PercentComplete', 'ErrorDescription',
- 'OperationalStatus']
- cim_xxxs_path = []
- while(loop_counter <= Smis._INVOKE_MAX_LOOP_COUNT):
- cim_job = self._c.GetInstance(cim_job_path,
- PropertyList=job_pros,
- LocalOnly=False)
- job_state = cim_job['JobState']
- if job_state in (dmtf.JOB_STATE_NEW, dmtf.JOB_STATE_STARTING,
- dmtf.JOB_STATE_RUNNING):
- loop_counter += 1
- time.sleep(Smis._INVOKE_CHECK_INTERVAL)
- continue
- elif job_state == dmtf.JOB_STATE_COMPLETED:
- if expect_class is None:
- return None
- cim_xxxs_path = self._c.AssociatorNames(
- cim_job.path,
- AssocClass='CIM_AffectedJobElement',
- ResultClass=expect_class)
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): Got unknown job state "
- "%d: %s" % (job_state, cim_job.items()))
- if len(cim_xxxs_path) != 1:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): got unexpected(not 1) "
- "return from CIM_AffectedJobElement: "
- "%s, out: %s, job: %s" %
- (cim_xxxs_path, out.items(),
- cim_job.items()))
- return cim_xxxs_path[0]
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "_wait_invoke(): Got unexpected rc code "
- "%d, out: %s" % (rc, out.items()))
-
def _cim_pep_path_of_fc_tgt(self, cim_fc_tgt_path):
"""
Return CIMInstanceName of CIM_SCSIProtocolEndpoint of CIM_FCPort
@@ -2168,10 +2045,7 @@ def _check_exist_cim_dev_mg(self, name, cim_gmms_path, cim_vol_path,
'MaskingGroup': cim_dev_mg.path,
'Members': [cim_vol_path],
}
- (rc, out) = self._c.InvokeMethod(
- 'AddMembers',
- cim_gmms_path, **in_params)
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('AddMembers', cim_gmms_path, in_params)
return cim_dev_mg.path

return None
@@ -2238,13 +2112,10 @@ def access_group_create(self, name, init_id, init_type, system,
cim_init_mg_pros = smis_ag.cim_init_mg_pros()

try:
- (rc, out) = self._c.InvokeMethod(
- 'CreateGroup', cim_gmms.path, **in_params)
-
- cim_init_mg_path = self._wait_invoke(
- rc, out, out_key='MaskingGroup',
+ cim_init_mg_path = self._c.invoke_method_wait(
+ 'CreateGroup', cim_gmms.path, in_params,
+ out_key='MaskingGroup',
expect_class='CIM_InitiatorMaskingGroup')
-
except (LsmError, CIMError):
# Check possible failure
# 1. Initiator already exist in other group.
@@ -2317,8 +2188,5 @@ def access_group_delete(self, access_group, flags=0):
'Force': True,
}

- (rc, out) = self._c.InvokeMethod('DeleteGroup', cim_gmms.path,
- **in_params)
-
- self._wait_invoke(rc, out)
+ self._c.invoke_method_wait('DeleteGroup', cim_gmms.path, in_params)
return None
diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
index b1fb54d..e922cbf 100644
--- a/plugin/smispy/smis_common.py
+++ b/plugin/smispy/smis_common.py
@@ -27,6 +27,7 @@
import traceback
import os
import datetime
+import time

import dmtf
from lsm import LsmError, ErrorNumber, md5
@@ -179,6 +180,9 @@ class SmisCommon(object):
IAAN_WBEM_HTTP_PORT = 5988
IAAN_WBEM_HTTPS_PORT = 5989

+ _INVOKE_MAX_LOOP_COUNT = 60
+ _INVOKE_CHECK_INTERVAL = 5
+
def __init__(self, url, username, password,
namespace=dmtf.DEFAULT_NAMESPACE,
no_ssl_verify=False, debug_path=None, system_list=None):
@@ -459,6 +463,104 @@ def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
else:
raise

+ def invoke_method_wait(self, cmd, cim_path, in_params,
+ out_key=None, expect_class=None,
+ flag_out_array=False):
+ """
+ InvokeMethod and wait it until done.
+ Return a CIMInstanceName from out[out_key] or from cim_job:
+ CIM_ConcreteJob
+ |
+ | CIM_AffectedJobElement
+ v
+ CIMInstanceName # expect_class
+ If flag_out_array is True, return the first element of out[out_key].
+ """
+ (rc, out) = self.InvokeMethod(cmd, cim_path, **in_params)
+
+ if rc == SmisCommon.SNIA_INVOKE_OK:
+ if out_key is None:
+ return None
+ if out_key in out:
+ if flag_out_array:
+ if len(out[out_key]) == 1:
+ return out[out_key][0]
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(), output contains %d " %
+ len(out[out_key]) +
+ "elements: %s" % out[out_key])
+ return out[out_key]
+ else:
+ raise LsmError(ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(), %s not exist in out %s" %
+ (out_key, out.items()))
+
+ elif rc == SmisCommon.SNIA_INVOKE_ASYNC:
+ cim_job_path = out['Job']
+ loop_counter = 0
+ job_pros = ['JobState', 'ErrorDescription',
+ 'OperationalStatus']
+ cim_xxxs_path = []
+ while(loop_counter <= SmisCommon._INVOKE_MAX_LOOP_COUNT):
+ cim_job = self.GetInstance(cim_job_path,
+ PropertyList=job_pros)
+ job_state = cim_job['JobState']
+ if job_state in (dmtf.JOB_STATE_NEW, dmtf.JOB_STATE_STARTING,
+ dmtf.JOB_STATE_RUNNING):
+ loop_counter += 1
+ time.sleep(SmisCommon._INVOKE_CHECK_INTERVAL)
+ continue
+ elif job_state == dmtf.JOB_STATE_COMPLETED:
+ if not SmisCommon.cim_job_completed_ok(cim_job):
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ str(cim_job['ErrorDescription']))
+ if expect_class is None:
+ return None
+ cim_xxxs_path = self.AssociatorNames(
+ cim_job.path,
+ AssocClass='CIM_AffectedJobElement',
+ ResultClass=expect_class)
+ break
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): Got unknown job state "
+ "%d: %s" % (job_state, cim_job.items()))
+
+ if loop_counter > SmisCommon._INVOKE_MAX_LOOP_COUNT:
+ raise LsmError(
+ ErrorNumber.TIMEOUT,
+ "The job generated by %s() failed to finish in %ds" %
+ (cmd,
+ SmisCommon._INVOKE_CHECK_INTERVAL *
+ SmisCommon._INVOKE_MAX_LOOP_COUNT))
+
+ if len(cim_xxxs_path) == 1:
+ return cim_xxxs_path[0]
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): got unexpected(not 1) "
+ "return from CIM_AffectedJobElement: "
+ "%s, out: %s, job: %s" %
+ (cim_xxxs_path, out.items(), cim_job.items()))
+ else:
+ try:
+ self._dump_wbem_xml(cmd)
+ except Exception:
+ tb = traceback.format_exc()
+ raise LsmError(ErrorNumber.PLUGIN_BUG,
+ "Error: %s rc= %s" % (cmd, str(rc)) +
+ " Debug data exception: %s" % str(tb))
+
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "invoke_method_wait(): Got unexpected rc code "
+ "%d, out: %s" % (rc, out.items()))
+
def _cim_srv_of_sys_id(self, srv_name, sys_id, raise_error):
property_list = ['SystemName']
--
1.8.3.1
Gris Ge
2014-11-20 06:59:01 UTC
Permalink
* Move/Merge smis.Smis._cim_init_path_check_or_create() and
smis.Smis._cim_init_path_create() into
smis_ag.cim_init_path_check_or_create()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 57daac0..9bae9cf 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -1390,41 +1390,6 @@ def access_groups(self, search_key=None, search_value=None, flags=0):

return search_property(rc, search_key, search_value)

- def _cim_init_path_check_or_create(self, system_id, init_id, init_type):
- """
- Check whether CIM_StorageHardwareID exists, if not, create new one.
- """
- cim_init = self._get_cim_instance_by_id(
- 'Initiator', init_id, raise_error=False)
-
- if cim_init:
- return cim_init.path
-
- # Create new one
- dmtf_id_type = _lsm_init_type_to_dmtf(init_type)
- if dmtf_id_type is None:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "SMI-S Plugin does not support init_type %d" %
- init_type)
-
- return self._cim_init_path_create(system_id, init_id, dmtf_id_type)
-
- def _cim_init_path_create(self, system_id, init_id, dmtf_id_type):
- """
- Create a CIM_StorageHardwareID.
- Return CIMInstanceName
- Raise error if failed. Return if pass.
- """
- cim_hwms = self._c.cim_hwms_of_sys_id(system_id)
-
- in_params = {'StorageID': init_id,
- 'IDType': pywbem.Uint16(dmtf_id_type)}
-
- # CreateStorageHardwareID does not allow ASYNC
- return self._c.invoke_method_wait(
- 'CreateStorageHardwareID', cim_hwms.path, in_params,
- out_key='HardwareID', expect_class='CIM_StorageHardwareID')
-
def _ag_init_add_group(self, access_group, init_id, init_type):
cim_sys = smis_sys.cim_sys_of_sys_id(self._c, access_group.system_id)

@@ -1445,8 +1410,8 @@ def _ag_init_add_group(self, access_group, init_id, init_type):
if smis_ag.init_id_of_cim_init(exist_cim_init) == init_id:
return copy.deepcopy(access_group)

- cim_init_path = self._cim_init_path_check_or_create(
- access_group.system_id, init_id, init_type)
+ cim_init_path = smis_ag.cim_init_path_check_or_create(
+ self._c, access_group.system_id, init_id, init_type)

cim_gmms = self._c.cim_gmms_of_sys_id(access_group.system_id)

@@ -1501,8 +1466,8 @@ def _ag_init_add_old(self, access_group, init_id, init_type):
# Check to see if we have this initiator already, if not we
# create it and then add to the view.

- self._cim_init_path_check_or_create(
- access_group.system_id, init_id, init_type)
+ smis_ag.cim_init_path_check_or_create(
+ self._c, access_group.system_id, init_id, init_type)

cim_ccs = self._c.cim_ccs_of_sys_id(access_group.system_id)

@@ -2093,8 +2058,8 @@ def access_group_create(self, name, init_id, init_type, system,
"iSCSI target port, which not allow creating "
"iSCSI IQN access group")

- cim_init_path = self._cim_init_path_check_or_create(
- system.id, init_id, init_type)
+ cim_init_path = smis_ag.cim_init_path_check_or_create(
+ self._c, system.id, init_id, init_type)

# Create CIM_InitiatorMaskingGroup
cim_gmms = self._c.cim_gmms_of_sys_id(system.id)
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index e93956d..7b9042d 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -215,3 +215,38 @@ def init_id_of_cim_init(cim_init):
ErrorNumber.PLUGIN_BUG,
"init_id_of_cim_init() got cim_init without 'StorageID' %s: %s" %
(cim_init.path, cim_init.items()))
+
+
+def cim_init_path_check_or_create(smis_common, system_id, init_id, init_type):
+ """
+ Check whether CIM_StorageHardwareID exists, if not, create new one.
+ """
+ cim_inits = smis_common.EnumerateInstances(
+ 'CIM_StorageHardwareID',
+ PropertyList=_CIM_INIT_PROS)
+
+ if len(cim_inits):
+ for cim_init in cim_inits:
+ if init_id_of_cim_init(cim_init) == init_id:
+ return cim_init.path
+
+ # Create new one
+ dmtf_id_type = None
+ if init_type == AccessGroup.INIT_TYPE_WWPN:
+ dmtf_id_type = dmtf.ID_TYPE_WWPN
+ elif init_type == AccessGroup.INIT_TYPE_ISCSI_IQN:
+ dmtf_id_type = dmtf.ID_TYPE_ISCSI
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "cim_init_path_check_or_create(): Got invalid init_type: %d" %
+ init_type)
+
+ cim_hwms = smis_common.cim_hwms_of_sys_id(system_id)
+ in_params = {
+ 'StorageID': init_id,
+ 'IDType': dmtf_id_type,
+ }
+ return smis_common.invoke_method_wait(
+ 'CreateStorageHardwareID', cim_hwms.path, in_params,
+ out_key='HardwareID', expect_class='CIM_StorageHardwareID')
--
1.8.3.1
Gris Ge
2014-11-20 06:59:02 UTC
Permalink
* Removed these methods from smis.py since not used by others:
* _lsm_init_type_to_dmtf()
* _get_cim_instance_by_id()
* _cim_class_name_of()
* _not_found_error_of_class()
* _property_list_of_id()
* _sys_id_child()
* _id()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 9bae9cf..3ba2805 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -114,15 +114,6 @@ def _lsm_tgt_port_type_of_cim_fc_tgt(cim_fc_tgt):
return TargetPort.TYPE_FC


-def _lsm_init_type_to_dmtf(init_type):
- if init_type == AccessGroup.INIT_TYPE_WWPN:
- return dmtf.ID_TYPE_WWPN
- if init_type == AccessGroup.INIT_TYPE_ISCSI_IQN:
- return dmtf.ID_TYPE_ISCSI
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Does not support provided init_type: %d" % init_type)
-
-
class Smis(IStorageAreaNetwork):
"""
SMI-S plug-ing which exposes a small subset of the overall provided
@@ -137,37 +128,6 @@ def __init__(self):
self._c = None
self.tmo = 0

- def _get_cim_instance_by_id(self, class_type, requested_id,
- property_list=None, raise_error=True):
- """
- Find out the CIM_XXXX Instance which holding the requested_id
- Return None when error and raise_error is False
- """
- class_name = Smis._cim_class_name_of(class_type)
- error_number = Smis._not_found_error_of_class(class_type)
- id_pros = Smis._property_list_of_id(class_type, property_list)
-
- if property_list is None:
- property_list = id_pros
- else:
- property_list = merge_list(property_list, id_pros)
-
- cim_xxxs = self._c.EnumerateInstances(
- class_name, PropertyList=property_list)
- org_requested_id = requested_id
- if class_type == 'Job':
- (requested_id, ignore, ignore) = SmisCommon.parse_job_id(
- requested_id)
- for cim_xxx in cim_xxxs:
- if self._id(class_type, cim_xxx) == requested_id:
- return cim_xxx
- if raise_error is False:
- return None
-
- raise LsmError(error_number,
- "Cannot find %s Instance with " % class_name +
- "%s ID '%s'" % (class_type, org_requested_id))
-
@handle_cim_errors
def plugin_register(self, uri, password, timeout, flags=0):
"""
@@ -295,105 +255,6 @@ def job_status(self, job_id, flags=0):
raise
return status, percent_complete, completed_item

- @staticmethod
- def _cim_class_name_of(class_type):
- if class_type == 'Volume':
- return 'CIM_StorageVolume'
- if class_type == 'Pool':
- return 'CIM_StoragePool'
- if class_type == 'Disk':
- return 'CIM_DiskDrive'
- if class_type == 'Job':
- return 'CIM_ConcreteJob'
- if class_type == 'AccessGroup':
- return 'CIM_SCSIProtocolController'
- if class_type == 'Initiator':
- return 'CIM_StorageHardwareID'
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Smis._cim_class_name_of() got unknown " +
- "class_type %s" % class_type)
-
- @staticmethod
- def _not_found_error_of_class(class_type):
- if class_type == 'Volume':
- return ErrorNumber.NOT_FOUND_VOLUME
- if class_type == 'Pool':
- return ErrorNumber.NOT_FOUND_POOL
- if class_type == 'Job':
- return ErrorNumber.NOT_FOUND_JOB
- if class_type == 'AccessGroup':
- return ErrorNumber.NOT_FOUND_ACCESS_GROUP
- if class_type == 'Initiator':
- return ErrorNumber.INVALID_ARGUMENT
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Smis._cim_class_name_of() got unknown " +
- "class_type %s" % class_type)
-
- @staticmethod
- def _property_list_of_id(class_type, extra_properties=None):
- """
- Return a PropertyList which the ID of current class is basing on
- """
- rc = []
- if class_type == 'Volume':
- rc = ['SystemName', 'DeviceID']
- elif class_type == 'SystemChild':
- rc = ['SystemName']
- elif class_type == 'Disk':
- rc = ['SystemName', 'DeviceID']
- elif class_type == 'Job':
- rc = ['InstanceID']
- elif class_type == 'Initiator':
- rc = ['StorageID']
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Smis._property_list_of_id() got unknown " +
- "class_type %s" % class_type)
-
- if extra_properties:
- rc = merge_list(rc, extra_properties)
- return rc
-
- def _sys_id_child(self, cim_xxx):
- """
- Find out the system id of Pool/Volume/Disk/AccessGroup/Initiator
- Currently, we just use SystemName of cim_xxx
- """
- return self._id('SystemChild', cim_xxx)
-
- def _id(self, class_type, cim_xxx):
- """
- Return the ID of certain class.
- When ID is based on two or more properties, we use MD5 hash of them.
- If not, return the property value.
- """
- property_list = Smis._property_list_of_id(class_type)
- for key in property_list:
- if key not in cim_xxx:
- cim_xxx = self._c.GetInstance(cim_xxx.path,
- PropertyList=property_list,
- LocalOnly=False)
- break
-
- id_str = ''
- for key in property_list:
- if key not in cim_xxx:
- cim_class_name = ''
- if class_type == 'SystemChild':
- cim_class_name = str(cim_xxx.classname)
- else:
- cim_class_name = Smis._cim_class_name_of(class_type)
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "%s %s " % (cim_class_name, cim_xxx.path) +
- "does not have property %s " % str(key) +
- "calculate out %s id" % class_type)
- else:
- id_str += cim_xxx[key]
- if len(property_list) == 1 and class_type != 'Job':
- return id_str
- else:
- return md5(id_str)
-
def _new_vol_from_name(self, out):
"""
Given a volume by CIMInstanceName, return a lsm Volume object
--
1.8.3.1
Gris Ge
2014-11-20 06:59:03 UTC
Permalink
* Replace smis._lsm_init_id_to_snia() with smis_ag.lsm_init_id_to_snia()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 3ba2805..a46dfdd 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -85,18 +85,6 @@
# Group M&M SNIA SMI-S 'Group Masking and Mapping' profile


-def _lsm_init_id_to_snia(lsm_init_id):
- """
- If lsm_init_id is a WWPN, convert it to SNIA format:
- [0-9A-F]{16}
- If not, return directly.
- """
- val, init_type, init_id = AccessGroup.initiator_id_verify(lsm_init_id)
- if val and init_type == AccessGroup.INIT_TYPE_WWPN:
- return lsm_init_id.replace(':', '').upper()
- return lsm_init_id
-
-
def _lsm_tgt_port_type_of_cim_fc_tgt(cim_fc_tgt):
"""
We are assuming we got CIM_FCPort. Caller should make sure of that.
@@ -1294,7 +1282,7 @@ def _ag_init_add_group(self, access_group, init_id, init_type):
@handle_cim_errors
def access_group_initiator_add(self, access_group, init_id, init_type,
flags=0):
- init_id = _lsm_init_id_to_snia(init_id)
+ init_id = smis_ag.lsm_init_id_to_snia(init_id)
mask_type = smis_cap.mask_type(self._c, raise_error=True)

if mask_type == smis_cap.MASK_TYPE_GROUP:
@@ -1399,7 +1387,7 @@ def access_group_initiator_delete(self, access_group, init_id, init_type,
raise LsmError(ErrorNumber.NO_SUPPORT,
"SMI-S plugin does not support "
"access_group_initiator_delete() against NetApp-E")
- init_id = _lsm_init_id_to_snia(init_id)
+ init_id = smis_ag.lsm_init_id_to_snia(init_id)
mask_type = smis_cap.mask_type(self._c, raise_error=True)

if mask_type == smis_cap.MASK_TYPE_GROUP:
@@ -1882,7 +1870,7 @@ def access_group_create(self, name, init_id, init_type, system,
1. Create CIM_InitiatorMaskingGroup
"""
org_init_id = init_id
- init_id = _lsm_init_id_to_snia(init_id)
+ init_id = smis_ag.lsm_init_id_to_snia(init_id)

self._c.profile_check(SmisCommon.SNIA_GROUP_MASK_PROFILE,
SmisCommon.SMIS_SPEC_VER_1_5,
diff --git a/plugin/smispy/smis_ag.py b/plugin/smispy/smis_ag.py
index 7b9042d..9a6d81b 100644
--- a/plugin/smispy/smis_ag.py
+++ b/plugin/smispy/smis_ag.py
@@ -217,6 +217,18 @@ def init_id_of_cim_init(cim_init):
(cim_init.path, cim_init.items()))


+def lsm_init_id_to_snia(lsm_init_id):
+ """
+ If lsm_init_id is a WWPN, convert it to SNIA format:
+ [0-9A-F]{16}
+ If not, return original directly.
+ """
+ val, init_type, init_id = AccessGroup.initiator_id_verify(lsm_init_id)
+ if val and init_type == AccessGroup.INIT_TYPE_WWPN:
+ return lsm_init_id.replace(':', '').upper()
+ return lsm_init_id
+
+
def cim_init_path_check_or_create(smis_common, system_id, init_id, init_type):
"""
Check whether CIM_StorageHardwareID exists, if not, create new one.
--
1.8.3.1
Gris Ge
2014-11-20 06:59:04 UTC
Permalink
* Removed these methods and properties as they are not used out side of
SmisCommon:
InvokeMethod()
last_request
last_reply

* For SmisCommon internal use of these methods and properties, changed them
to use self._wbem_conn instead.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis_common.py | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
index e922cbf..d58e263 100644
--- a/plugin/smispy/smis_common.py
+++ b/plugin/smispy/smis_common.py
@@ -297,23 +297,12 @@ def GetInstance(self, InstanceName, **params):
params['LocalOnly'] = False
return self._wbem_conn.GetInstance(InstanceName, **params)

- def InvokeMethod(self, MethodName, ObjectName, **params):
- return self._wbem_conn.InvokeMethod(MethodName, ObjectName, **params)
-
def DeleteInstance(self, InstanceName, **params):
return self._wbem_conn.DeleteInstance(InstanceName, **params)

def References(self, ObjectName, **params):
return self._wbem_conn.References(ObjectName, **params)

- @property
- def last_request(self):
- return self._wbem_conn.last_request
-
- @property
- def last_reply(self):
- return self._wbem_conn.last_reply
-
def is_megaraid(self):
return self._vendor_product == SmisCommon._PRODUCT_MEGARAID

@@ -393,7 +382,8 @@ def _dump_wbem_xml(self, file_prefix):
# Dump the request & reply to a file
with open(debug_full, 'w') as d:
d.write("REQ:\n%s\n\nREPLY:\n%s\n" %
- (self.last_request, self.last_reply))
+ (self._wbem_conn.last_request,
+ self._wbem_conn.last_reply))


def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
@@ -427,7 +417,8 @@ def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
if retrieve_data is None:
retrieve_data = SmisCommon.JOB_RETRIEVE_NONE
try:
- (rc, out) = self.InvokeMethod(cmd, cim_path, **in_params)
+ (rc, out) = self._wbem_conn.InvokeMethod(
+ cmd, cim_path, **in_params)

# Check to see if operation is done
if rc == SmisCommon.SNIA_INVOKE_OK:
@@ -476,7 +467,7 @@ def invoke_method_wait(self, cmd, cim_path, in_params,
CIMInstanceName # expect_class
If flag_out_array is True, return the first element of out[out_key].
"""
- (rc, out) = self.InvokeMethod(cmd, cim_path, **in_params)
+ (rc, out) = self._wbem_conn.InvokeMethod(cmd, cim_path, **in_params)

if rc == SmisCommon.SNIA_INVOKE_OK:
if out_key is None:
--
1.8.3.1
Gris Ge
2014-11-20 06:59:05 UTC
Permalink
* Fixed these PEP8 errors:
smis_common.py:222:19: E121 continuation line indentation is not a multiple
of four
smis_common.py:389:5: E303 too many blank lines (2)

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis_common.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
index d58e263..4ac19ce 100644
--- a/plugin/smispy/smis_common.py
+++ b/plugin/smispy/smis_common.py
@@ -219,7 +219,7 @@ def __init__(self, url, username, password,
SmisCommon.SNIA_BLK_ROOT_PROFILE: SmisCommon.SMIS_SPEC_VER_1_4,
SmisCommon.SNIA_BLK_SRVS_PROFILE: SmisCommon.SMIS_SPEC_VER_1_4,
SmisCommon.SNIA_DISK_LITE_PROFILE:
- SmisCommon.SMIS_SPEC_VER_1_4,
+ SmisCommon.SMIS_SPEC_VER_1_4,
}
self._vendor_product = SmisCommon._PRODUCT_MEGARAID
else:
@@ -385,7 +385,6 @@ def _dump_wbem_xml(self, file_prefix):
(self._wbem_conn.last_request,
self._wbem_conn.last_reply))

-
def invoke_method(self, cmd, cim_path, in_params, out_handler=None,
error_handler=None, retrieve_data=None,
method_data=None):
--
1.8.3.1
Tony Asleson
2014-11-21 01:22:27 UTC
Permalink
Patch set looks good. I ran across all boxes in lab with no regressions
appearing.

Patch set committed.

Thanks!
Tony
'[PATCH 0/7] Improve access group related methods.'
* Patches in V1 about access group error has been removed.
They will have their own patch set along with other plugin update and
new test case.
* Spliced V1 patch to fit the 'one patch for one small thing' rule.
* Improved invoke_method_wait() in patch 3/8 to support debug_path URI
parameter.
* Removed some unused methods from SmisCommon.
* EMC VMAX
* EMC VNX
* HP 3PAR
SMI-S Plugin: Move Smis._job_completed_ok to SmisCommon
SMI-S Plugin: Move WBEM xml dump code to _dump_wbem_xml() method.
SMI-S Plugin: Move job wait method to SmisCommon.
SMI-S Plugin: Move smis.Smis._init_id() into smis_ag.py
SMI-S Plugin: Move cim_init check into smis_ag.py
SMI-S Plugin: Remove unused methods from smis.py
SMI-S Plugin: Move lsm init_id to SNIA converter
SMI-S Plugin: Remove unused methods from SmisCommon
SMI-S Plugin: PEP8 fix for smis_common.py
plugin/smispy/smis.py | 453 ++++++-------------------------------------
plugin/smispy/smis_ag.py | 63 +++++-
plugin/smispy/smis_common.py | 176 ++++++++++++++---
3 files changed, 262 insertions(+), 430 deletions(-)
Loading...