Discussion:
[Libstoragemgmt-devel] [PATCH 0/7] SMI-S plugin reorganize and improve performance of system()
Gris Ge
2014-09-15 12:16:50 UTC
Permalink
New layout of smis.py:
1. DMTF
# Store DMTF CIM constants
2. SmisCommon
# Store SNIA constants and share modules for SmisSys and etc.
3. SmisSys # Coming SmisVol, SmisPool, SmisDisk and etc.
# All lsm.System related methods.
4. Smis, MegaRAID, ESeries
# Plugin
5. SmisProxy
# Proxy for MegaRAID, ESeries and Smis plugin.

Module dependency rule: a module can only depend on lower(smaller number)
layer modules.

Performance improvements:
1. Improve the lsm.Client.systems() on MegaRAID:

Remote LSI SMI-Provider:
Before: 5.07 seconds
Now: 2.02 seconds
Local LSI SMI-Provider:
Before: 0.42 seconds
Now: 0.31 seconds

2. Improve cim_sys retrieving by skip leaf CIM_ComputerSystem.

Before: self._get_cim_instance_by_id('System', system.id)
# It enumerate all CIM_ComputerSystem,
# 1 enumerate call.
Now: self._cim_sys_of_sys_id(system.id)
# Only search in root CIM_ComputerSystem,
# 1 assocication call
No actually performance improvement found as it just save some
data CIM transfer.

Gris Ge (7):
SMI-S plugin: Add MegaRAID module for vendor specific codes
SMI-S plugin: Add SmisCommon module for shared methods in Smis and
Smis sub modules
SMI-S plugin: Add SmisSys module for all lsm.System related methods
Packaging: Include smis_common.py, smis_sys.py and megaraid.py into
-smis-plugin package
SMI-S plugin: Expose DMTF.dmtf_op_status_list_conv(), add
"root/PG_interop" namespace and move cim_sys things to SmisSys
module
SMI-S plugin: Use runtime module loading in SmisProxy
SMI-S plugin: Improve profile register check and systems() query

packaging/libstoragemgmt.spec.in | 3 +
plugin/Makefile.am | 3 +
plugin/smispy/dmtf.py | 38 +--
plugin/smispy/megaraid.py | 49 ++++
plugin/smispy/smis.py | 586 +++++++++------------------------------
plugin/smispy/smis_common.py | 159 +++++++++++
plugin/smispy/smis_sys.py | 125 +++++++++
plugin/smispy/smisproxy.py | 5 +-
8 files changed, 487 insertions(+), 481 deletions(-)
create mode 100644 plugin/smispy/megaraid.py
create mode 100644 plugin/smispy/smis_common.py
create mode 100644 plugin/smispy/smis_sys.py
--
1.8.3.1
Gris Ge
2014-09-15 12:16:51 UTC
Permalink
* Skip profile register checking and provide known support status directly.
* Overide _root_cim_syss() as we have not set self._root_blk_cim_rp yet.
Use enumerate instead as MegaRAID does not support 'Multiple Computer
System' profile.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/megaraid.py | 49 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 plugin/smispy/megaraid.py

diff --git a/plugin/smispy/megaraid.py b/plugin/smispy/megaraid.py
new file mode 100644
index 0000000..b120671
--- /dev/null
+++ b/plugin/smispy/megaraid.py
@@ -0,0 +1,49 @@
+# Copyright (C) 2014 Red Hat, Inc.
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Author: Gris Ge <***@redhat.com>
+
+from smis import Smis
+from dmtf import DMTF
+from lsm import Pool
+from smis_common import SmisCommon, merge_list
+from smis_sys import SmisSys
+
+class MegaRAID(Smis):
+ def _plugin_register_vendor_workaround(self):
+ self._profile_dict = {
+ SmisCommon.SNIA_BLK_ROOT_PROFILE: SmisCommon.SMIS_SPEC_VER_1_4,
+ SmisCommon.SNIA_DISK_LITE_PROFILE: SmisCommon.SMIS_SPEC_VER_1_4,
+ }
+ self.vendor_namespace = 'root/LsiMr13'
+ return None
+
+ @staticmethod
+ def _profile_register_load(wbem_conn):
+ return {}, None
+
+ # MegaRAID still using 1.2 SMI-S, no need to bother the profile register.
+ SmisCommon.profile_register_load= _profile_register_load
+
+ def _root_cim_syss(self, property_list=None):
+ cim_sys_id_pros = SmisSys.cim_sys_id_pros()
+ if property_list is None:
+ property_list = cim_sys_id_pros
+ else:
+ property_list = merge_list(property_list, cim_sys_id_pros)
+
+ return self._c.EnumerateInstances(
+ 'CIM_ComputerSystem',
+ PropertyList=property_list)
--
1.8.3.1
Andy Grover
2014-09-15 19:14:08 UTC
Permalink
Post by Gris Ge
* Skip profile register checking and provide known support status directly.
* Overide _root_cim_syss() as we have not set self._root_blk_cim_rp yet.
Use enumerate instead as MegaRAID does not support 'Multiple Computer
System' profile.
If you are refactoring, you should just refactor and put other changes
in a separate patch.

Also, this patch depends on patch 2 (smis_common). Try to move dependent
patches after the things they depend on.

Finally, in contrast to Java, small classes do not get their own files.
Only put things in their own files when they make sense to import
separately, or if the one file is truly getting too big.

Regards -- Andy
Post by Gris Ge
---
plugin/smispy/megaraid.py | 49 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 plugin/smispy/megaraid.py
diff --git a/plugin/smispy/megaraid.py b/plugin/smispy/megaraid.py
new file mode 100644
index 0000000..b120671
--- /dev/null
+++ b/plugin/smispy/megaraid.py
@@ -0,0 +1,49 @@
+# Copyright (C) 2014 Red Hat, Inc.
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+
+from smis import Smis
+from dmtf import DMTF
+from lsm import Pool
+from smis_common import SmisCommon, merge_list
+from smis_sys import SmisSys
+
+ self._profile_dict = {
+ SmisCommon.SNIA_BLK_ROOT_PROFILE: SmisCommon.SMIS_SPEC_VER_1_4,
+ SmisCommon.SNIA_DISK_LITE_PROFILE: SmisCommon.SMIS_SPEC_VER_1_4,
+ }
+ self.vendor_namespace = 'root/LsiMr13'
+ return None
+
+ return {}, None
+
+ # MegaRAID still using 1.2 SMI-S, no need to bother the profile register.
+ SmisCommon.profile_register_load= _profile_register_load
+
+ cim_sys_id_pros = SmisSys.cim_sys_id_pros()
+ property_list = cim_sys_id_pros
+ property_list = merge_list(property_list, cim_sys_id_pros)
+
+ return self._c.EnumerateInstances(
+ 'CIM_ComputerSystem',
+ PropertyList=property_list)
Gris Ge
2014-09-16 09:18:52 UTC
Permalink
Post by Andy Grover
Post by Gris Ge
* Skip profile register checking and provide known support status directly.
* Overide _root_cim_syss() as we have not set self._root_blk_cim_rp yet.
Use enumerate instead as MegaRAID does not support 'Multiple Computer
System' profile.
If you are refactoring, you should just refactor and put other
changes in a separate patch.
Also, this patch depends on patch 2 (smis_common). Try to move
dependent patches after the things they depend on.
Sure. Will do.
Post by Andy Grover
Finally, in contrast to Java, small classes do not get their own
files. Only put things in their own files when they make sense to
import separately, or if the one file is truly getting too big.
MegaRAID only implement old version(1.2) of SMI-S while other storage
array like EMC or NetApp are using SMI-S 1.4+.

Splitting MegaRAID methods into a separate file could saving me from
coding too much workaround in smis.py(already 4000+ lines).
Post by Andy Grover
Regards -- Andy
--
Gris Ge
Gris Ge
2014-09-15 12:16:52 UTC
Permalink
* Rewrite profile register check in SmisCommon:
SmisCommon.profile_register_load()
# Return a dictionary containing highest version of profiles.
# Also return a root profile('Array' profile).
SmisCommon.profile_check()
# Take the dictionary generated by profile_register_load() to check
# profile support status.
# Removed 'strict=False' parameter, as no methods is using it.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis_common.py | 159 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 159 insertions(+)
create mode 100644 plugin/smispy/smis_common.py

diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
new file mode 100644
index 0000000..49ee757
--- /dev/null
+++ b/plugin/smispy/smis_common.py
@@ -0,0 +1,159 @@
+# Copyright (C) 2014 Red Hat, Inc.
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+# Author: Gris Ge <***@redhat.com>
+
+import pywbem
+from pywbem import CIMError
+
+from dmtf import DMTF
+from lsm import LsmError, ErrorNumber
+
+
+def merge_list(list_a, list_b):
+ return list(set(list_a + list_b))
+
+
+class SmisCommon(object):
+ """
+ This class hold the shared methods between SmisSys and SmisVol and etc.
+ To simply the complex SMI-S, all functions in this class are static.
+ """
+ SNIA_BLK_ROOT_PROFILE = 'Array'
+ SNIA_BLK_SRVS_PROFILE = 'Block Services'
+ SNIA_DISK_LITE_PROFILE = 'Disk Drive Lite'
+ SNIA_MULTI_SYS_PROFILE = 'Multiple Computer System'
+ SNIA_MASK_PROFILE = 'Masking and Mapping'
+ SNIA_GROUP_MASK_PROFILE = 'Group Masking and Mapping'
+ SNIA_FC_TGT_PORT_PROFILE = 'FC Target Ports'
+ SNIA_ISCSI_TGT_PORT_PROFILE = 'iSCSI Target Ports'
+ SMIS_SPEC_VER_1_2 = '1.2'
+ SMIS_SPEC_VER_1_4 = '1.4'
+ SMIS_SPEC_VER_1_5 = '1.5'
+ SMIS_SPEC_VER_1_6 = '1.6'
+ SNIA_REG_ORG_CODE = pywbem.Uint16(11)
+ IAAN_WBEM_HTTP_PORT = 5988
+ IAAN_WBEM_HTTPS_PORT = 5989
+ SMIS_DEFAULT_NAMESPACE = 'interop'
+ SNIA_REG_ORG_CODE = pywbem.Uint16(11)
+
+ @staticmethod
+ def profile_register_load(wbem_conn):
+ """
+ Check CIM_RegisteredProfile in interop namespace.
+ Return (profile_dict, root_blk_cim_rp)
+ The 'profile_dict' is a dictionary like this:
+ {
+ # profile_name: max_version
+ 'Array': 1.4,
+ 'Block Service Profile': 1.4,
+ }
+ The 'root_blk_cim_rp' is the 'Array' profile of CIM_RegisteredProfile
+ with hightest version number.
+ """
+ profile_dict = {}
+ root_blk_cim_rp = None
+ namespace_check_list = DMTF.INTEROP_NAMESPACES
+
+ cim_rps = []
+ for namespace in namespace_check_list:
+ try:
+ cim_rps = wbem_conn.EnumerateInstances(
+ 'CIM_RegisteredProfile',
+ namespace=namespace,
+ PropertyList=['RegisteredName', 'RegisteredVersion',
+ 'RegisteredOrganization'],
+ LocalOnly=False)
+ except CIMError as e:
+ if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
+ e[0] == pywbem.CIM_ERR_INVALID_NAMESPACE or \
+ e[0] == pywbem.CIM_ERR_INVALID_CLASS:
+ pass
+ else:
+ raise
+ if len(cim_rps) != 0:
+ break
+
+ if len(cim_rps) >= 1:
+ for cim_rp in cim_rps:
+ if cim_rp['RegisteredOrganization'] != \
+ SmisCommon.SNIA_REG_ORG_CODE:
+ continue
+ profile_name = cim_rp['RegisteredName']
+ profile_ver = cim_rp['RegisteredVersion']
+ profile_ver_num = SmisCommon._profile_spec_ver_to_num(
+ profile_ver)
+ if profile_name in profile_dict.keys():
+ exist_ver_num = SmisCommon._profile_spec_ver_to_num(
+ profile_dict[profile_name])
+ if exist_ver_num >= profile_ver_num:
+ continue
+ if profile_name == SmisCommon.SNIA_BLK_ROOT_PROFILE:
+ root_blk_cim_rp = cim_rp
+ profile_dict[profile_name] = profile_ver
+
+ return profile_dict, root_blk_cim_rp
+
+ @staticmethod
+ def profile_check(profile_dict, profile_name, spec_ver, raise_error=False):
+ """
+ Check whether we support certain profile at certain SNIA
+ specification version.
+ Profile spec version later or equal than require spec_ver will also be
+ consider as found.
+ Require profile_dict provided by SmisCommon.profile_register_load()
+ Will raise LsmError(ErrorNumber.NO_SUPPORT, 'xxx') if raise_error
+ is True when nothing found.
+ """
+ request_ver_num = SmisCommon._profile_spec_ver_to_num(spec_ver)
+ if profile_name not in profile_dict.keys():
+ if raise_error:
+ raise LsmError(
+ ErrorNumber.NO_SUPPORT,
+ "SNIA SMI-S %s '%s' profile is not supported by " %
+ (profile_name, spec_ver) +
+ "target SMI-S provider")
+ return False
+
+ support_ver_num = SmisCommon._profile_spec_ver_to_num(
+ profile_dict[profile_name])
+ if support_ver_num < request_ver_num:
+ if raise_error:
+ raise LsmError(
+ ErrorNumber.NO_SUPPORT,
+ "SNIA SMI-S %s '%s' profile is not supported by " %
+ (profile_name, spec_ver) +
+ "target SMI-S provider. Only version %s is supported" %
+ profile_dict[profile_name])
+ return True
+
+ @staticmethod
+ def _profile_spec_ver_to_num(spec_ver_str):
+ """
+ Convert version string stored in CIM_RegisteredProfile to a integer.
+ Example:
+ "1.5.1" -> 1,005,001
+ """
+ tmp_list = [0, 0, 0]
+ tmp_list = spec_ver_str.split(".")
+ if len(tmp_list) == 2:
+ tmp_list.extend([0])
+ if len(tmp_list) == 3:
+ return (int(tmp_list[0]) * 10 ** 6 +
+ int(tmp_list[1]) * 10 ** 3 +
+ int(tmp_list[2]))
+ return None
+
+
--
1.8.3.1
Andy Grover
2014-09-15 19:15:36 UTC
Permalink
Post by Gris Ge
SmisCommon.profile_register_load()
# Return a dictionary containing highest version of profiles.
# Also return a root profile('Array' profile).
SmisCommon.profile_check()
# Take the dictionary generated by profile_register_load() to check
# profile support status.
# Removed 'strict=False' parameter, as no methods is using it.
Refactoring patches should be separate from functionality patches.
Post by Gris Ge
---
plugin/smispy/smis_common.py | 159 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 159 insertions(+)
create mode 100644 plugin/smispy/smis_common.py
diff --git a/plugin/smispy/smis_common.py b/plugin/smispy/smis_common.py
new file mode 100644
index 0000000..49ee757
--- /dev/null
+++ b/plugin/smispy/smis_common.py
@@ -0,0 +1,159 @@
+# Copyright (C) 2014 Red Hat, Inc.
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+
+import pywbem
+from pywbem import CIMError
+
+from dmtf import DMTF
+from lsm import LsmError, ErrorNumber
+
+
+ return list(set(list_a + list_b))
Probably should go in a utils.py or something. If it's only used a few
times then I wouldn't even bother with it.
Post by Gris Ge
+
+
Eliminate this class. Everything here should just be de-indented and
live at the top level of the module.

Regards -- Andy
Post by Gris Ge
+ """
+ This class hold the shared methods between SmisSys and SmisVol and etc.
+ To simply the complex SMI-S, all functions in this class are static.
+ """
+ SNIA_BLK_ROOT_PROFILE = 'Array'
+ SNIA_BLK_SRVS_PROFILE = 'Block Services'
+ SNIA_DISK_LITE_PROFILE = 'Disk Drive Lite'
+ SNIA_MULTI_SYS_PROFILE = 'Multiple Computer System'
+ SNIA_MASK_PROFILE = 'Masking and Mapping'
+ SNIA_GROUP_MASK_PROFILE = 'Group Masking and Mapping'
+ SNIA_FC_TGT_PORT_PROFILE = 'FC Target Ports'
+ SNIA_ISCSI_TGT_PORT_PROFILE = 'iSCSI Target Ports'
+ SMIS_SPEC_VER_1_2 = '1.2'
+ SMIS_SPEC_VER_1_4 = '1.4'
+ SMIS_SPEC_VER_1_5 = '1.5'
+ SMIS_SPEC_VER_1_6 = '1.6'
+ SNIA_REG_ORG_CODE = pywbem.Uint16(11)
+ IAAN_WBEM_HTTP_PORT = 5988
+ IAAN_WBEM_HTTPS_PORT = 5989
+ SMIS_DEFAULT_NAMESPACE = 'interop'
+ SNIA_REG_ORG_CODE = pywbem.Uint16(11)
+
+ """
+ Check CIM_RegisteredProfile in interop namespace.
+ Return (profile_dict, root_blk_cim_rp)
+ {
+ # profile_name: max_version
+ 'Array': 1.4,
+ 'Block Service Profile': 1.4,
+ }
+ The 'root_blk_cim_rp' is the 'Array' profile of CIM_RegisteredProfile
+ with hightest version number.
+ """
+ profile_dict = {}
+ root_blk_cim_rp = None
+ namespace_check_list = DMTF.INTEROP_NAMESPACES
+
+ cim_rps = []
+ cim_rps = wbem_conn.EnumerateInstances(
+ 'CIM_RegisteredProfile',
+ namespace=namespace,
+ PropertyList=['RegisteredName', 'RegisteredVersion',
+ 'RegisteredOrganization'],
+ LocalOnly=False)
+ if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
+ e[0] == pywbem.CIM_ERR_INVALID_NAMESPACE or \
+ pass
+ raise
+ break
+
+ if cim_rp['RegisteredOrganization'] != \
+ continue
+ profile_name = cim_rp['RegisteredName']
+ profile_ver = cim_rp['RegisteredVersion']
+ profile_ver_num = SmisCommon._profile_spec_ver_to_num(
+ profile_ver)
+ exist_ver_num = SmisCommon._profile_spec_ver_to_num(
+ profile_dict[profile_name])
+ continue
+ root_blk_cim_rp = cim_rp
+ profile_dict[profile_name] = profile_ver
+
+ return profile_dict, root_blk_cim_rp
+
+ """
+ Check whether we support certain profile at certain SNIA
+ specification version.
+ Profile spec version later or equal than require spec_ver will also be
+ consider as found.
+ Require profile_dict provided by SmisCommon.profile_register_load()
+ Will raise LsmError(ErrorNumber.NO_SUPPORT, 'xxx') if raise_error
+ is True when nothing found.
+ """
+ request_ver_num = SmisCommon._profile_spec_ver_to_num(spec_ver)
+ raise LsmError(
+ ErrorNumber.NO_SUPPORT,
+ "SNIA SMI-S %s '%s' profile is not supported by " %
+ (profile_name, spec_ver) +
+ "target SMI-S provider")
+ return False
+
+ support_ver_num = SmisCommon._profile_spec_ver_to_num(
+ profile_dict[profile_name])
+ raise LsmError(
+ ErrorNumber.NO_SUPPORT,
+ "SNIA SMI-S %s '%s' profile is not supported by " %
+ (profile_name, spec_ver) +
+ "target SMI-S provider. Only version %s is supported" %
+ profile_dict[profile_name])
+ return True
+
+ """
+ Convert version string stored in CIM_RegisteredProfile to a integer.
+ "1.5.1" -> 1,005,001
+ """
+ tmp_list = [0, 0, 0]
+ tmp_list = spec_ver_str.split(".")
+ tmp_list.extend([0])
+ return (int(tmp_list[0]) * 10 ** 6 +
+ int(tmp_list[1]) * 10 ** 3 +
+ int(tmp_list[2]))
+ return None
+
+
Gris Ge
2014-09-16 09:26:36 UTC
Permalink
Post by Andy Grover
Refactoring patches should be separate from functionality patches.
Sure. Will do.
Post by Andy Grover
Post by Gris Ge
+ return list(set(list_a + list_b))
Probably should go in a utils.py or something. If it's only used a
few times then I wouldn't even bother with it.
It will be used by many methods to merge PropertyList argument.
I will save it into utils.py.
Post by Andy Grover
Post by Gris Ge
+
+
Eliminate this class. Everything here should just be de-indented and
live at the top level of the module.
Sure. Will do.
Post by Andy Grover
Regards -- Andy
--
Gris Ge
Gris Ge
2014-09-15 12:16:53 UTC
Permalink
* Move lsm.System.status and lsm.System.status_info checking method from
DMTF to SmisSys.sys_status_of_cim_sys().
* Move _root_cim_sys() from smis.py to SmisSys.root_cim_sys() with these
changes:
1. Not allow enumerate any more.
Actually, all vendors(including LSI) I got support profile register.
This should not be a problem. Tested in SNIA lab and NetApp-E.
* Move properties generator from smis.py to SmisSys.cim_sys_id_pros() and
SmisSys.cim_sys_pros().
* Move sys_id generator from smis.py to SmisSys.sys_id_of_cim_sys().

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis_sys.py | 125 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
create mode 100644 plugin/smispy/smis_sys.py

diff --git a/plugin/smispy/smis_sys.py b/plugin/smispy/smis_sys.py
new file mode 100644
index 0000000..e0bef86
--- /dev/null
+++ b/plugin/smispy/smis_sys.py
@@ -0,0 +1,125 @@
+# Copyright (C) 2014 Red Hat, Inc.
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+# Author: tasleson
+# Gris Ge <***@redhat.com>
+
+import pywbem
+from pywbem import CIMError
+
+from dmtf import DMTF
+from lsm import LsmError, ErrorNumber, System
+from smis_common import SmisCommon, merge_list
+
+class SmisSys(object):
+ """
+ To simply the complex SMI-S, all functions in this class are static.
+ This class handle all lsm.System related methods and constants.
+ We are using CIM_ComputerSystem for lsm.System from SMI-S 1.4+ 'Block
+ Service Pacakge' profile.
+ """
+ @staticmethod
+ def cim_sys_id_pros():
+ """
+ Return the property of CIM_ComputerSystem required to gernarate
+ lsm.System.id
+ """
+ return ['Name']
+
+ @staticmethod
+ def sys_id_of_cim_sys(cim_sys):
+ if 'Name' in cim_sys:
+ return cim_sys['Name']
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "sys_id_of_cim_sys(): Got a CIM_ComputerSystem does not have "
+ "'Name' property: %s, %s" % (cim_sys.items(), cim_sys.path))
+
+ @staticmethod
+ def cim_sys_pros():
+ cim_sys_pros = ['ElementName', 'OperationalStatus']
+ cim_sys_pros.extend(SmisSys.cim_sys_id_pros())
+ return cim_sys_pros
+
+ @staticmethod
+ def cim_sys_to_lsm_sys(cim_sys):
+ status = System.STATUS_UNKNOWN
+ status_info = ''
+
+ if 'OperationalStatus' in cim_sys:
+ (status, status_info) = SmisSys.sys_status_of_cim_sys(cim_sys)
+
+ sys_id = SmisSys.sys_id_of_cim_sys(cim_sys)
+ sys_name = cim_sys['ElementName']
+
+ return System(sys_id, sys_name, status, status_info)
+
+ @staticmethod
+ def root_cim_sys(wbem_conn, cim_root_rp, sys_filter, property_list=None):
+ """
+ Use this association to find out the root CIM_ComputerSystem:
+ CIM_RegisteredProfile # Root Profile('Array') in interop
+ |
+ | CIM_ElementConformsToProfile
+ v
+ CIM_ComputerSystem # vendor namespace
+ """
+ cim_scss_path = []
+ id_pros = SmisSys.cim_sys_id_pros()
+ if property_list is None:
+ property_list = id_pros
+ else:
+ property_list = merge_list(property_list, id_pros)
+
+ cim_syss = wbem_conn.Associators(
+ cim_root_rp.path,
+ ResultClass='CIM_ComputerSystem',
+ AssocClass='CIM_ElementConformsToProfile',
+ PropertyList=property_list)
+
+ if len(cim_syss) == 0:
+ raise LsmError(ErrorNumber.NO_SUPPORT,
+ "Current SMI-S provider does not provide "
+ "the root CIM_ComputerSystem associated "
+ "to 'Array' CIM_RegisteredProfile.")
+
+ # System URI Filtering
+ if sys_filter:
+ needed_cim_syss = []
+ for cim_sys in cim_syss:
+ if SmisSys.sys_id_of_cim_sys(cim_sys) in sys_filter:
+ needed_cim_syss.extend([cim_sys])
+ return needed_cim_syss
+ else:
+ return cim_syss
+
+ _LSM_SYS_OP_STATUS_CONV = {
+ DMTF.OP_STATUS_OK: System.STATUS_OK,
+ DMTF.OP_STATUS_ERROR: System.STATUS_ERROR,
+ DMTF.OP_STATUS_DEGRADED: System.STATUS_DEGRADED,
+ DMTF.OP_STATUS_NON_RECOVERABLE_ERROR: System.STATUS_ERROR,
+ DMTF.OP_STATUS_PREDICTIVE_FAILURE: System.STATUS_PREDICTIVE_FAILURE,
+ DMTF.OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: System.STATUS_ERROR,
+ }
+
+ @staticmethod
+ def sys_status_of_cim_sys(cim_sys):
+ """
+ Convert CIM_ComputerSystem['OperationalStatus']
+ """
+ return DMTF.dmtf_op_status_list_conv(
+ SmisSys._LSM_SYS_OP_STATUS_CONV, cim_sys['OperationalStatus'],
+ System.STATUS_UNKNOWN, System.STATUS_OTHER)
--
1.8.3.1
Andy Grover
2014-09-15 19:15:10 UTC
Permalink
Post by Gris Ge
* Move lsm.System.status and lsm.System.status_info checking method from
DMTF to SmisSys.sys_status_of_cim_sys().
* Move _root_cim_sys() from smis.py to SmisSys.root_cim_sys() with these
1. Not allow enumerate any more.
Actually, all vendors(including LSI) I got support profile register.
This should not be a problem. Tested in SNIA lab and NetApp-E.
* Move properties generator from smis.py to SmisSys.cim_sys_id_pros() and
SmisSys.cim_sys_pros().
* Move sys_id generator from smis.py to SmisSys.sys_id_of_cim_sys().
---
plugin/smispy/smis_sys.py | 125 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
create mode 100644 plugin/smispy/smis_sys.py
diff --git a/plugin/smispy/smis_sys.py b/plugin/smispy/smis_sys.py
new file mode 100644
index 0000000..e0bef86
--- /dev/null
+++ b/plugin/smispy/smis_sys.py
@@ -0,0 +1,125 @@
+# Copyright (C) 2014 Red Hat, Inc.
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+# Author: tasleson
+
+import pywbem
+from pywbem import CIMError
+
+from dmtf import DMTF
+from lsm import LsmError, ErrorNumber, System
+from smis_common import SmisCommon, merge_list
+
If you have an object that only contains data and static methods, and no
__init__, then it shouldn't be a class. The class is just being used as
a namespace. Just put everything at the top level of the module, the
module is the namespace.

Regards -- Andy
Post by Gris Ge
+ """
+ To simply the complex SMI-S, all functions in this class are static.
+ This class handle all lsm.System related methods and constants.
+ We are using CIM_ComputerSystem for lsm.System from SMI-S 1.4+ 'Block
+ Service Pacakge' profile.
+ """
+ """
+ Return the property of CIM_ComputerSystem required to gernarate
+ lsm.System.id
+ """
+ return ['Name']
+
+ return cim_sys['Name']
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "sys_id_of_cim_sys(): Got a CIM_ComputerSystem does not have "
+ "'Name' property: %s, %s" % (cim_sys.items(), cim_sys.path))
+
+ cim_sys_pros = ['ElementName', 'OperationalStatus']
+ cim_sys_pros.extend(SmisSys.cim_sys_id_pros())
+ return cim_sys_pros
+
+ status = System.STATUS_UNKNOWN
+ status_info = ''
+
+ (status, status_info) = SmisSys.sys_status_of_cim_sys(cim_sys)
+
+ sys_id = SmisSys.sys_id_of_cim_sys(cim_sys)
+ sys_name = cim_sys['ElementName']
+
+ return System(sys_id, sys_name, status, status_info)
+
+ """
+ CIM_RegisteredProfile # Root Profile('Array') in interop
+ |
+ | CIM_ElementConformsToProfile
+ v
+ CIM_ComputerSystem # vendor namespace
+ """
+ cim_scss_path = []
+ id_pros = SmisSys.cim_sys_id_pros()
+ property_list = id_pros
+ property_list = merge_list(property_list, id_pros)
+
+ cim_syss = wbem_conn.Associators(
+ cim_root_rp.path,
+ ResultClass='CIM_ComputerSystem',
+ AssocClass='CIM_ElementConformsToProfile',
+ PropertyList=property_list)
+
+ raise LsmError(ErrorNumber.NO_SUPPORT,
+ "Current SMI-S provider does not provide "
+ "the root CIM_ComputerSystem associated "
+ "to 'Array' CIM_RegisteredProfile.")
+
+ # System URI Filtering
+ needed_cim_syss = []
+ needed_cim_syss.extend([cim_sys])
+ return needed_cim_syss
+ return cim_syss
+
+ _LSM_SYS_OP_STATUS_CONV = {
+ DMTF.OP_STATUS_OK: System.STATUS_OK,
+ DMTF.OP_STATUS_ERROR: System.STATUS_ERROR,
+ DMTF.OP_STATUS_DEGRADED: System.STATUS_DEGRADED,
+ DMTF.OP_STATUS_NON_RECOVERABLE_ERROR: System.STATUS_ERROR,
+ DMTF.OP_STATUS_PREDICTIVE_FAILURE: System.STATUS_PREDICTIVE_FAILURE,
+ DMTF.OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: System.STATUS_ERROR,
+ }
+
+ """
+ Convert CIM_ComputerSystem['OperationalStatus']
+ """
+ return DMTF.dmtf_op_status_list_conv(
+ SmisSys._LSM_SYS_OP_STATUS_CONV, cim_sys['OperationalStatus'],
+ System.STATUS_UNKNOWN, System.STATUS_OTHER)
Gris Ge
2014-09-16 09:19:59 UTC
Permalink
Post by Andy Grover
If you have an object that only contains data and static methods,
and no __init__, then it shouldn't be a class. The class is just
being used as a namespace. Just put everything at the top level of
the module, the module is the namespace.
Regards -- Andy
Thank you for the suggestion.
I will use file name as namespace.

Best regards.
--
Gris Ge
Gris Ge
2014-09-15 12:16:54 UTC
Permalink
* Add megaraid.py, smis_sys.py and smis_common.py into
libstoragemgmt-smis-plugin package.

Signed-off-by: Gris Ge <***@redhat.com>
---
packaging/libstoragemgmt.spec.in | 3 +++
plugin/Makefile.am | 3 +++
2 files changed, 6 insertions(+)

diff --git a/packaging/libstoragemgmt.spec.in b/packaging/libstoragemgmt.spec.in
index 5e0871e..3848a23 100644
--- a/packaging/libstoragemgmt.spec.in
+++ b/packaging/libstoragemgmt.spec.in
@@ -316,6 +316,9 @@ fi
%{python_sitelib}/lsm/plugin/smispy/smis.*
%{python_sitelib}/lsm/plugin/smispy/smisproxy.*
%{python_sitelib}/lsm/plugin/smispy/dmtf.*
+%{python_sitelib}/lsm/plugin/smispy/megaraid.*
+%{python_sitelib}/lsm/plugin/smispy/smis_common.*
+%{python_sitelib}/lsm/plugin/smispy/smis_sys.*
%{_bindir}/smispy_lsmplugin

%files netapp-plugin
diff --git a/plugin/Makefile.am b/plugin/Makefile.am
index 5a4dca2..08ae9c6 100644
--- a/plugin/Makefile.am
+++ b/plugin/Makefile.am
@@ -27,6 +27,9 @@ smispy_PYTHON = \
smispy/smis.py \
smispy/smisproxy.py \
smispy/eseries.py \
+ smispy/smis_common.py \
+ smispy/megaraid.py \
+ smispy/smis_sys.py \
smispy/dmtf.py

nstordir = $(plugindir)/nstor
--
1.8.3.1
Gris Ge
2014-09-15 12:16:55 UTC
Permalink
* Make _dmtf_op_status_list_conv() public in order to allowing moving
status converter to SmisXXX modules.
* Remove cim_sys_status_of() and related constants.
It has been moved to SmisSys module.
* Add 'root/PG_interop' namespace to interop check list.
Some provider like LSI/Avago MegaRAID and HP 3PAR are still using that one.

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

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index 5edbfc1..0fdcd3b 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2011-2014 Red Hat, Inc.
+# Copyright (C) 2014 Red Hat, Inc.
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
# License as published by the Free Software Foundation; either
@@ -18,11 +18,15 @@

# This class handle DMTF CIM constants and convert to LSM type.

-from lsm import (System, Pool, Disk)
+from lsm import (System, Pool, Disk, LsmError, ErrorNumber)
from pywbem import Uint16


class DMTF(object):
+ """
+ DMTF class contains DMTF CIM constants and their conversion.
+ To simplify things, all functions are staticmethod.
+ """
# CIM_ManagedSystemElement['OperationalStatus']
OP_STATUS_UNKNOWN = 0
OP_STATUS_OTHER = 1
@@ -77,17 +81,8 @@ class DMTF(object):
except KeyError:
return ''

- _LSM_SYS_OP_STATUS_CONV = {
- OP_STATUS_OK: System.STATUS_OK,
- OP_STATUS_ERROR: System.STATUS_ERROR,
- OP_STATUS_DEGRADED: System.STATUS_DEGRADED,
- OP_STATUS_NON_RECOVERABLE_ERROR: System.STATUS_ERROR,
- OP_STATUS_PREDICTIVE_FAILURE: System.STATUS_PREDICTIVE_FAILURE,
- OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: System.STATUS_ERROR,
- }
-
@staticmethod
- def _dmtf_op_status_list_conv(conv_dict, dmtf_op_status_list,
+ def dmtf_op_status_list_conv(conv_dict, dmtf_op_status_list,
unknown_value, other_value):
status = 0
status_info_list = []
@@ -104,15 +99,6 @@ class DMTF(object):
status = unknown_value
return status, " ".join(status_info_list)

- @staticmethod
- def cim_sys_status_of(dmtf_op_status_list):
- """
- Convert CIM_ComputerSystem['OperationalStatus']
- """
- return DMTF._dmtf_op_status_list_conv(
- DMTF._LSM_SYS_OP_STATUS_CONV, dmtf_op_status_list,
- System.STATUS_UNKNOWN, System.STATUS_OTHER)
-
_LSM_POOL_OP_STATUS_CONV = {
OP_STATUS_OK: Pool.STATUS_OK,
OP_STATUS_ERROR: Pool.STATUS_ERROR,
@@ -126,7 +112,7 @@ class DMTF(object):
"""
Convert CIM_StoragePool['OperationalStatus'] to LSM
"""
- return DMTF._dmtf_op_status_list_conv(
+ return DMTF.dmtf_op_status_list_conv(
DMTF._LSM_POOL_OP_STATUS_CONV, dmtf_op_status_list,
Pool.STATUS_UNKNOWN, Pool.STATUS_OTHER)

@@ -149,7 +135,7 @@ class DMTF(object):
Convert CIM_DiskDrive['OperationalStatus'] to LSM
Only return status, no status_info
"""
- return DMTF._dmtf_op_status_list_conv(
+ return DMTF.dmtf_op_status_list_conv(
DMTF._LSM_DISK_OP_STATUS_CONV, dmtf_op_status_list,
Disk.STATUS_UNKNOWN, Disk.STATUS_OTHER)[0]

@@ -205,3 +191,9 @@ class DMTF(object):
SCS_CAP_VOLUME_CREATE = Uint16(5)
SCS_CAP_VOLUME_DELETE = Uint16(6)
SCS_CAP_VOLUME_MODIFY = Uint16(7)
+
+ # DSP 1033 Profile Registration
+ # 'root/PG_interop' is not standard by DMTF but just workaround for
+ # HP 3PAR and LSI MegaRAID.
+ INTEROP_NAMESPACES = ['interop', 'root/interop', 'root/PG_interop']
+
--
1.8.3.1
Andy Grover
2014-09-15 19:15:45 UTC
Permalink
Post by Gris Ge
* Make _dmtf_op_status_list_conv() public in order to allowing moving
status converter to SmisXXX modules.
* Remove cim_sys_status_of() and related constants.
It has been moved to SmisSys module.
* Add 'root/PG_interop' namespace to interop check list.
Some provider like LSI/Avago MegaRAID and HP 3PAR are still using that one.
---
plugin/smispy/dmtf.py | 38 +++++++++++++++-----------------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index 5edbfc1..0fdcd3b 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2011-2014 Red Hat, Inc.
+# Copyright (C) 2014 Red Hat, Inc.
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
# License as published by the Free Software Foundation; either
@@ -18,11 +18,15 @@
# This class handle DMTF CIM constants and convert to LSM type.
-from lsm import (System, Pool, Disk)
+from lsm import (System, Pool, Disk, LsmError, ErrorNumber)
from pywbem import Uint16
Yet another case where this class is not needed, remove it.

Regards -- Andy
Post by Gris Ge
+ """
+ DMTF class contains DMTF CIM constants and their conversion.
+ To simplify things, all functions are staticmethod.
+ """
# CIM_ManagedSystemElement['OperationalStatus']
OP_STATUS_UNKNOWN = 0
OP_STATUS_OTHER = 1
return ''
- _LSM_SYS_OP_STATUS_CONV = {
- OP_STATUS_OK: System.STATUS_OK,
- OP_STATUS_ERROR: System.STATUS_ERROR,
- OP_STATUS_DEGRADED: System.STATUS_DEGRADED,
- OP_STATUS_NON_RECOVERABLE_ERROR: System.STATUS_ERROR,
- OP_STATUS_PREDICTIVE_FAILURE: System.STATUS_PREDICTIVE_FAILURE,
- OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: System.STATUS_ERROR,
- }
-
@staticmethod
- def _dmtf_op_status_list_conv(conv_dict, dmtf_op_status_list,
+ def dmtf_op_status_list_conv(conv_dict, dmtf_op_status_list,
status = 0
status_info_list = []
status = unknown_value
return status, " ".join(status_info_list)
- """
- Convert CIM_ComputerSystem['OperationalStatus']
- """
- return DMTF._dmtf_op_status_list_conv(
- DMTF._LSM_SYS_OP_STATUS_CONV, dmtf_op_status_list,
- System.STATUS_UNKNOWN, System.STATUS_OTHER)
-
_LSM_POOL_OP_STATUS_CONV = {
OP_STATUS_OK: Pool.STATUS_OK,
OP_STATUS_ERROR: Pool.STATUS_ERROR,
"""
Convert CIM_StoragePool['OperationalStatus'] to LSM
"""
- return DMTF._dmtf_op_status_list_conv(
+ return DMTF.dmtf_op_status_list_conv(
DMTF._LSM_POOL_OP_STATUS_CONV, dmtf_op_status_list,
Pool.STATUS_UNKNOWN, Pool.STATUS_OTHER)
Convert CIM_DiskDrive['OperationalStatus'] to LSM
Only return status, no status_info
"""
- return DMTF._dmtf_op_status_list_conv(
+ return DMTF.dmtf_op_status_list_conv(
DMTF._LSM_DISK_OP_STATUS_CONV, dmtf_op_status_list,
Disk.STATUS_UNKNOWN, Disk.STATUS_OTHER)[0]
SCS_CAP_VOLUME_CREATE = Uint16(5)
SCS_CAP_VOLUME_DELETE = Uint16(6)
SCS_CAP_VOLUME_MODIFY = Uint16(7)
+
+ # DSP 1033 Profile Registration
+ # 'root/PG_interop' is not standard by DMTF but just workaround for
+ # HP 3PAR and LSI MegaRAID.
+ INTEROP_NAMESPACES = ['interop', 'root/interop', 'root/PG_interop']
+
Gris Ge
2014-09-16 09:28:06 UTC
Permalink
Post by Andy Grover
Yet another case where this class is not needed, remove it.
Sure. Will do.

Thank you for the review and suggestions.
Best regards.
Post by Andy Grover
Regards -- Andy
--
Gris Ge
Gris Ge
2014-09-15 12:16:56 UTC
Permalink
* The Smis module has been divided in to SmisCommon and SmisSys,
ESeries and MegaRAID has to override method like this:

class MegaRAID(Smis):
def _some_method(self, xxx):
pass
SmisCommon.some_method = _some_method

In this way, if we load MegaRAID module, Smis class will be changed also.
Hence we have to use runtime module loading.

Please kindly let me know if you have better way.

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

diff --git a/plugin/smispy/smisproxy.py b/plugin/smispy/smisproxy.py
index 19a32ce..162111d 100644
--- a/plugin/smispy/smisproxy.py
+++ b/plugin/smispy/smisproxy.py
@@ -17,7 +17,6 @@

from smis import Smis
from lsm import Proxy, VERSION
-import eseries

#The unfortunate truth is that each of the vendors implements functionality
#slightly differently so we will need to have some special code for these
@@ -44,7 +43,9 @@ class SmisProxy(Proxy):

#TODO We need to do a better job at check for this.
if 'root/lsiarray13' in uri.lower():
- self.proxied_obj = eseries.ESeries()
+ self.proxied_obj = __import__('eseries').ESeries()
+ elif 'root/lsimr13' in uri.lower():
+ self.proxied_obj = __import__('megaraid').MegaRAID()
else:
self.proxied_obj = Smis()
--
1.8.3.1
Andy Grover
2014-09-15 19:16:08 UTC
Permalink
Post by Gris Ge
* The Smis module has been divided in to SmisCommon and SmisSys,
pass
SmisCommon.some_method = _some_method
In this way, if we load MegaRAID module, Smis class will be changed also.
Hence we have to use runtime module loading.
Please kindly let me know if you have better way.
You just want to override some behavior for some types of array that
don't exactly follow SMI-S right?

Use inheritance. You shouldn't be monkey-patching the base class,
instead put the different behavior in the derived classes and
instantiate them instead of the base class. See below.
Post by Gris Ge
#TODO We need to do a better job at check for this.
- self.proxied_obj = eseries.ESeries()
+ self.proxied_obj = __import__('eseries').ESeries()
+ self.proxied_obj = __import__('megaraid').MegaRAID()
self.proxied_obj = Smis()
if 'root/lsiarray13' in uri.lower():
self.proxied_obj = ESeries()
elif 'root/lsimr13' in uri.lower():
self.proxied_obj = MegaRAID()
else:
self.proxied_obj = Smis()

Here is a clear example of instantiating derived classes if needed, or
else relying on the standard base class behavior.

Regards -- Andy
Gris Ge
2014-09-16 09:50:32 UTC
Permalink
Post by Andy Grover
Post by Gris Ge
pass
SmisCommon.some_method = _some_method
You just want to override some behavior for some types of array that
don't exactly follow SMI-S right?
1. MegaRAID SMI-S is 1.2 while we are using 1.4 in this plugin.
2. SMI-S has some bad design while using MegaRAID extension is good for
performance and data accuracy. Request SNIA to improve their profile
is not easy job as submit a patch.
Example: Query disk type.
Post by Andy Grover
Use inheritance. You shouldn't be monkey-patching the base class,
instead put the different behavior in the derived classes and
instantiate them instead of the base class. See below.
Post by Gris Ge
#TODO We need to do a better job at check for this.
- self.proxied_obj = eseries.ESeries()
+ self.proxied_obj = __import__('eseries').ESeries()
+ self.proxied_obj = __import__('megaraid').MegaRAID()
self.proxied_obj = Smis()
self.proxied_obj = ESeries()
self.proxied_obj = MegaRAID()
self.proxied_obj = Smis()
Here is a clear example of instantiating derived classes if needed,
or else relying on the standard base class behavior.
Regards -- Andy
AFAIK, inheritance only works when all functions/methods are saved into
one namespace or class[1]. It cannot work if I split Smis class into many
sub class.

Example:

# File smis.py
class Smis():
def systems():
cim_syss = SmisSys.root_cim_sys()
do something against cim_syss
return lsm.System

# File smis_sys.py

def root_cim_sys():
do_something

# File megaraid.py
# Want to override the smi_sys.root_cim_sys() method.

## A: Inheritance:
class MegaRAID(Smis)
def root_cim_sys():
do something different

## B: Override

class MegaRAID(Smis)
def _root_cim_sys():
do something different

smis_sys.root_cim_sys = _root_cim_sys

In this case, A) will not work, but B) will work for runtime module
loading.

[1] Current smis.py file has 4128 lines. It's hard for me to maintain
that file now.
--
Gris Ge
Tony Asleson
2014-09-16 21:37:14 UTC
Permalink
Post by Gris Ge
Post by Andy Grover
self.proxied_obj = ESeries()
self.proxied_obj = MegaRAID()
self.proxied_obj = Smis()
Here is a clear example of instantiating derived classes if needed,
or else relying on the standard base class behavior.
AFAIK, inheritance only works when all functions/methods are saved into
one namespace or class[1]. It cannot work if I split Smis class into many
sub class.
Lets step back for a minute and think about the problems we are trying
to solve. SMI-S providers support different versions, each having
implementation bugs and vendor extensions which we avoid. Today the
plug-in code is *littered* with checks for different vendors quirks and
bug workarounds and we have this separate eseries file too.

I created the separate eseries file & class as there was some
functionality that deviated from the spec. and it seemed like a good
approach to keep from polluting the common code, but if you look back
through the commits you will see that this separate file was out of date
and broken often. It's easy to forget to go and check a separate file
that overrides methods. If you look at the eseries file today we see
the following:

1. We have copied and pasted code for plugin_register that is out of
date, doesn't support everything that the common plugin does.

2. The capabilities is using _common_capabilities which appears to have
been orphaned in the common code base. Not what I had intended.

Adding separate classes that override methods for MegaRAID and others
will likely result in more code that is broken and out of date, just
like the eseries specific code.
Post by Gris Ge
class MegaRAID(Smis)
do something different
smis_sys.root_cim_sys = _root_cim_sys
This is a hack, one that is only going to make things uglier, not better
over time. It's typically discouraged in the community. Trying to
mentally walk through the code will become impossible if class methods
are hijacked all over the place.
Post by Gris Ge
[1] Current smis.py file has 4128 lines. It's hard for me to maintain
that file now.
The SMI-S plug-in could use some clean-up, but the difficulties mainly
stem from the differences we have to deal with for vendor differences
and how the code is structured, not that there are so many lines in one
file.

In my opinion I believe we should proceed with the following:

1. Abandon the notion that writing separate class(es) that override the
common plug-in for vendor differences yields easier to maintain code.
History has shown that this is not true. Remove eseries specific file
and incorporate changes back into common plugin. Toss out the SmisProxy
class. We already have conditionals in the code checking for different
vendor quirks, continue to do that. Easier to maintain one code path
than multiple copies of code path.

2. Create separate files for:

* Constants (Snia/SMI-S & DMTF), Gris already started this, lets expand
* Move methods that are really utility functions to a separate utility file
* Create a class or function library that handles the low level CIM
operations, connect, enumerations, instance getting, invoking & job
control etc.
* Create individual classes & files or function libraries for handling
specific parts of the interface which utilizes the same low level class
for cim operations.

Thoughts on this approach?

Regards,
Tony
Andy Grover
2014-09-17 03:00:20 UTC
Permalink
Post by Tony Asleson
* Constants (Snia/SMI-S & DMTF), Gris already started this, lets expand
* Move methods that are really utility functions to a separate utility file
* Create a class or function library that handles the low level CIM
operations, connect, enumerations, instance getting, invoking & job
control etc.
* Create individual classes & files or function libraries for handling
specific parts of the interface which utilizes the same low level class
for cim operations.
Thoughts on this approach?
I think this sounds promising. As we've seen, OOP isn't always the right
tool for the job, but the techniques above are equally valid approaches
to writing good code. Python is a multi-paradigm language.

One point I'd make is that this somewhat-unpleasant plugin is a
significant part of the value of libsm! Shielding admins from crappy
SMI-S implementations is important! Differing versions and
implementations of a standard is kind of a nightmare scenario. This code
will never be pretty, but putting efforts into taming it could at least
make it maintainable.

Regards -- Andy
Gris Ge
2014-09-17 13:57:23 UTC
Permalink
Post by Tony Asleson
1. Abandon the notion that writing separate class(es) that override the
common plug-in for vendor differences yields easier to maintain code.
History has shown that this is not true. Remove eseries specific file
and incorporate changes back into common plugin. Toss out the SmisProxy
class. We already have conditionals in the code checking for different
vendor quirks, continue to do that. Easier to maintain one code path
than multiple copies of code path.
* Constants (Snia/SMI-S & DMTF), Gris already started this, lets expand
* Move methods that are really utility functions to a separate utility file
* Create a class or function library that handles the low level CIM
operations, connect, enumerations, instance getting, invoking & job
control etc.
* Create individual classes & files or function libraries for handling
specific parts of the interface which utilizes the same low level class
for cim operations.
Thoughts on this approach?
Regards,
Tony
Hi Tony,

Removing the SmisProxy is a good point.

I will do the initial work base on this.

Thanks.
--
Gris Ge
Gris Ge
2014-09-15 12:33:13 UTC
Permalink
* Profile register check in plugin_register() has been moved to
SmisCommon.profile_register_load().
* New _plugin_register_vendor_workaround() with NetApp profile register typo
fix:
NetApp: 'FC Target Port'
SMI-S: 'FC Target Ports'
* Fallback mode has been removed.
* The 'force_fallback_mode' has been changed to silently ignored.
No provider are facing issue on this.
* Updated self._root_cim_syss() to use SmisSys.root_cim_sys().
* Replaced _get_cim_instance_by_id('System', sys_id) with
self._cim_sys_of_sys_id(sys_id).
* The SNIA class has been merged into SmisCommon.
* Change _iscsi_tgt_is_supported() to check 1.2 version of 'iSCSI Target
Port' profile as it has not been changed since then and many(HP and HDS)
are still using 1.2 version string.
* These lsm.System related methods has been moved to SmisSys:
* self._sys_id() -> SmisSys.sys_id_of_cim_sys()
* self._cim_sys_to_lsm -> SmisSys.cim_sys_to_lsm_sys()
* self._cim_sys_pros() -> SmisSys.cim_sys_pros()
* self._property_list_of_id('System') -> SmisSys.cim_sys_id_pros()

Tested on these arrays for 'lsmcli list --type systems' and
'list --type target_ports':
* LSI/Avago MegaRAID
* EMC VNX
* EMC CX
* EMC VMAX
* IBM XIV
* Fujitsu DX80S2
* HP 3PAR
* Dell/Compellent Storage Center
* Dot Hill AssuredSAN 4524
* HDS AMS2100
* NetApp ONTAP
* NetApp-E

Tested on this array for 'lsmcli volume-mask', 'lsmcli volume-create' and
'lsmcli volume-unmask' for _get_cim_instance_by_id() changes:
* EMC VNX

Plugin_test PASS[1] on EMC VNX.

[1] Except plugin_test failed on IS_MASKED when deleting a volume and access
group.

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 870c6c1..c433ed0 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -35,6 +35,8 @@ from lsm import (IStorageAreaNetwork, error, uri_parse, LsmError, ErrorNumber,
search_property)

from dmtf import DMTF
+from smis_common import SmisCommon
+from smis_sys import SmisSys

## Variable Naming scheme:
# cim_xxx CIMInstance
@@ -188,21 +190,6 @@ def _lsm_init_type_to_dmtf(init_type):
"Does not support provided init_type: %d" % init_type)


-class SNIA(object):
- BLK_ROOT_PROFILE = 'Array'
- BLK_SRVS_PROFILE = 'Block Services'
- DISK_LITE_PROFILE = 'Disk Drive Lite'
- MULTI_SYS_PROFILE = 'Multiple Computer System'
- MASK_PROFILE = 'Masking and Mapping'
- GROUP_MASK_PROFILE = 'Group Masking and Mapping'
- FC_TGT_PORT_PROFILE = 'FC Target Ports'
- ISCSI_TGT_PORT_PROFILE = 'iSCSI Target Ports'
- SMIS_SPEC_VER_1_4 = '1.4'
- SMIS_SPEC_VER_1_5 = '1.5'
- SMIS_SPEC_VER_1_6 = '1.6'
- REG_ORG_CODE = pywbem.Uint16(11)
-
-
class Smis(IStorageAreaNetwork):
"""
SMI-S plug-ing which exposes a small subset of the overall provided
@@ -327,7 +314,6 @@ class Smis(IStorageAreaNetwork):
DMTF_STATUS_POWER_MODE = 18

# DSP 1033 Profile Registration
- DMTF_INTEROP_NAMESPACES = ['interop', 'root/interop']
SMIS_DEFAULT_NAMESPACE = 'interop'

IAAN_WBEM_HTTP_PORT = 5988
@@ -405,11 +391,11 @@ class Smis(IStorageAreaNetwork):
self._c = None
self.tmo = 0
self.system_list = None
- self.cim_rps = []
- self.cim_root_profile_dict = dict()
- self.fallback_mode = True # Means we cannot use profile register
self.all_vendor_namespaces = []
self.debug_path = None
+ self._profile_dict = {}
+ self._blk_root_cim_rp = None
+ self._vendor_namespace = None

def _get_cim_instance_by_id(self, class_type, requested_id,
property_list=None, raise_error=True):
@@ -583,48 +569,30 @@ class Smis(IStorageAreaNetwork):

if 'force_fallback_mode' in u['parameters'] and \
u['parameters']['force_fallback_mode'] == 'yes':
- return
+ # force_fallback_mode is not needed any more.
+ # silently ignore.
+ pass

- # Checking profile registration support status unless
- # force_fallback_mode is enabled in URI.
- namespace_check_list = Smis.DMTF_INTEROP_NAMESPACES
- if 'namespace' in u['parameters'] and \
- u['parameters']['namespace'] not in namespace_check_list:
- namespace_check_list.extend([u['parameters']['namespace']])
+ (self._profile_dict, self._blk_root_cim_rp) = \
+ SmisCommon.profile_register_load(self._c)

- for interop_namespace in Smis.DMTF_INTEROP_NAMESPACES:
- try:
- self.cim_rps = self._c.EnumerateInstances(
- 'CIM_RegisteredProfile',
- namespace=interop_namespace,
- PropertyList=['RegisteredName', 'RegisteredVersion',
- 'RegisteredOrganization'],
- LocalOnly=False)
- except CIMError as e:
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_NAMESPACE or \
- e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- pass
- else:
- raise
- if len(self.cim_rps) != 0:
- break
+ self._plugin_register_vendor_workaround()

- if len(self.cim_rps) >= 1:
- self.fallback_mode = False
- self.all_vendor_namespaces = []
- # Support 'Array' profile is step 0 for this whole plugin.
- # We find out all 'Array' CIM_RegisteredProfile and stored
- # them into self.cim_root_profile_dict
- if not self._profile_is_supported(
- SNIA.BLK_ROOT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False):
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Target SMI-S provider does not support "
- "SNIA SMI-S SPEC %s '%s' profile" %
- (SNIA.SMIS_SPEC_VER_1_4,
- SNIA.BLK_ROOT_PROFILE))
+ SmisCommon.profile_check(
+ self._profile_dict, SmisCommon.SNIA_BLK_ROOT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4, raise_error=True)
+
+ def _plugin_register_vendor_workaround(self):
+ """
+ Vendor specfic wround for plugin_register()
+ """
+ # NetApp ONTAP typo on FC Target profile:
+ # NetApp: 'FC Target Port'
+ # SMI-S: 'FC Target Ports'
+ if 'FC Target Port' in self._profile_dict.keys():
+ self._profile_dict[SmisCommon.SNIA_FC_TGT_PORT_PROFILE] = \
+ self._profile_dict['FC Target Port']
+ return None

def time_out_set(self, ms, flags=0):
self.tmo = ms
@@ -643,20 +611,6 @@ class Smis(IStorageAreaNetwork):
volume_resize()
volume_delete()
"""
- if self.fallback_mode:
- # pools() is mandatory, we will try pools() related methods first
- try:
- self._cim_pools_of(cim_sys_path)
- except CIMError as e:
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Target SMI-S provider does not support "
- "CIM_StoragePool querying which is "
- "mandatory for pools() method")
- else:
- raise
-
# CIM_StorageConfigurationService is optional.
cim_scs_path = self._get_cim_service_path(
cim_sys_path, 'CIM_StorageConfigurationService')
@@ -708,23 +662,10 @@ class Smis(IStorageAreaNetwork):
return

def _disk_cap_set(self, cim_sys_path, cap):
- if self.fallback_mode:
- try:
- # Assuming provider support disk drive when systems under it
- # support it.
- self._enumerate('CIM_DiskDrive')
- except CIMError as e:
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- return
- else:
- raise
- else:
- if not self._profile_is_supported(SNIA.DISK_LITE_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False):
- return
+ if not self._profile_is_supported(SmisCommon.SNIA_DISK_LITE_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
+ raise_error=False):
+ return

cap.set(Capabilities.DISKS)
return
@@ -807,16 +748,9 @@ class Smis(IStorageAreaNetwork):
Fallback mode means target provider does not support interop, but
they still need to follow at least SNIA SMI-S 1.4rev6
"""
- if self.fallback_mode:
- cim_ccs_path = self._get_cim_service_path(
- cim_sys_path, 'CIM_ControllerConfigurationService')
- if cim_ccs_path is None:
- return
-
- elif not self._profile_is_supported(SNIA.MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False):
+ if not self._profile_is_supported(SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
+ raise_error=False):
return

cap.set(Capabilities.ACCESS_GROUPS)
@@ -831,9 +765,9 @@ class Smis(IStorageAreaNetwork):
if cim_sys_path.classname == 'Clar_StorageSystem':
return

- if self._fc_tgt_is_supported(cim_sys_path):
+ if self._fc_tgt_is_supported():
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_WWPN)
- if self._iscsi_tgt_is_supported(cim_sys_path):
+ if self._iscsi_tgt_is_supported():
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_ISCSI_IQN)
return

@@ -845,51 +779,8 @@ class Smis(IStorageAreaNetwork):

def _tgt_cap_set(self, cim_sys_path, cap):

- # LSI MegaRAID actually not support FC Target and iSCSI target,
- # They expose empty list of CIM_FCPort
- if cim_sys_path.classname == 'LSIESG_MegaRAIDHBA':
- return
-
- flag_fc_support = False
- flag_iscsi_support = False
- if self.fallback_mode:
- flag_fc_support = True
- flag_iscsi_support = True
- # CIM_FCPort is the control class of FC Targets profile
- try:
- self._cim_fc_tgt_of(cim_sys_path)
- except CIMError as e:
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- flag_fc_support = False
-
- try:
- self._cim_iscsi_pg_of(cim_sys_path)
- except CIMError as e:
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- flag_iscsi_support = False
- else:
- flag_fc_support = self._profile_is_supported(
- SNIA.FC_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
- # One more check for NetApp Typo:
- # NetApp: 'FC Target Port'
- # SMI-S: 'FC Target Ports'
- # Bug reported.
- if not flag_fc_support:
- flag_fc_support = self._profile_is_supported(
- 'FC Target Port',
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
- flag_iscsi_support = self._profile_is_supported(
- SNIA.ISCSI_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
+ flag_fc_support = self._fc_tgt_is_supported()
+ flag_iscsi_support = self._iscsi_tgt_is_supported()

if flag_fc_support or flag_iscsi_support:
cap.set(Capabilities.TARGET_PORTS)
@@ -914,10 +805,10 @@ class Smis(IStorageAreaNetwork):
cap.set(Capabilities.ACCESS_GROUPS_GRANTED_TO_VOLUME)
cap.set(Capabilities.VOLUMES_ACCESSIBLE_BY_ACCESS_GROUP)
cap.set(Capabilities.VOLUME_MASK)
- if self._fc_tgt_is_supported(cim_sys_path):
+ if self._fc_tgt_is_supported():
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_WWPN)
cap.set(Capabilities.ACCESS_GROUP_CREATE_WWPN)
- if self._iscsi_tgt_is_supported(cim_sys_path):
+ if self._iscsi_tgt_is_supported():
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_ISCSI_IQN)
cap.set(Capabilities.ACCESS_GROUP_CREATE_ISCSI_IQN)

@@ -960,8 +851,7 @@ class Smis(IStorageAreaNetwork):
@handle_cim_errors
def capabilities(self, system, flags=0):

- cim_sys = self._get_cim_instance_by_id(
- 'System', system.id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(system.id)

cap = Capabilities()

@@ -1098,8 +988,6 @@ class Smis(IStorageAreaNetwork):
rc = []
if class_type == 'Volume':
rc = ['SystemName', 'DeviceID']
- elif class_type == 'System':
- rc = ['Name']
elif class_type == 'Pool':
rc = ['InstanceID']
elif class_type == 'SystemChild':
@@ -1126,12 +1014,6 @@ class Smis(IStorageAreaNetwork):
"""
return self._id('SystemChild', cim_xxx)

- def _sys_id(self, cim_sys):
- """
- Return CIM_ComputerSystem['Name']
- """
- return self._id('System', cim_sys)
-
def _pool_id(self, cim_pool):
"""
Return CIM_StoragePool['InstanceID']
@@ -1518,11 +1400,11 @@ class Smis(IStorageAreaNetwork):
profile.
"""
rc = []
- cim_sys_pros = self._property_list_of_id("System")
+ cim_sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)
cim_vol_pros = self._cim_vol_pros()
for cim_sys in cim_syss:
- sys_id = self._sys_id(cim_sys)
+ sys_id = SmisSys.sys_id_of_cim_sys(cim_sys)
pool_pros = self._property_list_of_id('Pool')
for cim_pool in self._cim_pools_of(cim_sys.path, pool_pros):
pool_id = self._pool_id(cim_pool)
@@ -1583,11 +1465,11 @@ class Smis(IStorageAreaNetwork):
rc = []
cim_pool_pros = self._new_pool_cim_pool_pros()

- cim_sys_pros = self._property_list_of_id("System")
+ cim_sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)

for cim_sys in cim_syss:
- system_id = self._sys_id(cim_sys)
+ system_id = SmisSys.sys_id_of_cim_sys(cim_sys)
for cim_pool in self._cim_pools_of(cim_sys.path, cim_pool_pros):
# Skip spare storage pool.
if 'Usage' in cim_pool and \
@@ -1629,12 +1511,12 @@ class Smis(IStorageAreaNetwork):
Find out the system ID for certain CIM_StoragePool.
Will return '' if failed.
"""
- sys_pros = self._property_list_of_id('System')
+ sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._c.Associators(cim_pool.path,
ResultClass='CIM_ComputerSystem',
PropertyList=sys_pros)
if len(cim_syss) == 1:
- return self._sys_id(cim_syss[0])
+ return SmisSys.sys_id_of_cim_sys(cim_syss[0])
return ''

@handle_cim_errors
@@ -1668,29 +1550,6 @@ class Smis(IStorageAreaNetwork):
total_space, free_space,
status, status_info, system_id)

- @staticmethod
- def _cim_sys_to_lsm(cim_sys):
- # In the case of systems we are assuming that the System Name is
- # unique.
- status = System.STATUS_UNKNOWN
- status_info = ''
-
- if 'OperationalStatus' in cim_sys:
- (status, status_info) = \
- DMTF.cim_sys_status_of(cim_sys['OperationalStatus'])
-
- return System(cim_sys['Name'], cim_sys['ElementName'], status,
- status_info)
-
- def _cim_sys_pros(self):
- """
- Return a list of properties required to create a LSM System
- """
- cim_sys_pros = self._property_list_of_id('System',
- ['ElementName',
- 'OperationalStatus'])
- return cim_sys_pros
-
@handle_cim_errors
def systems(self, flags=0):
"""
@@ -1700,10 +1559,10 @@ class Smis(IStorageAreaNetwork):
don't check support status here as startup() already checked 'Array'
profile.
"""
- cim_sys_pros = self._cim_sys_pros()
+ cim_sys_pros = SmisSys.cim_sys_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)

- return [Smis._cim_sys_to_lsm(s) for s in cim_syss]
+ return [SmisSys.cim_sys_to_lsm_sys(s) for s in cim_syss]

def _check_for_dupe_vol(self, volume_name, original_exception):
"""
@@ -2142,8 +2001,7 @@ class Smis(IStorageAreaNetwork):
target ports(all FC and FCoE port for access_group.init_type == WWPN,
and the same to iSCSI)
"""
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)

cim_init_mg = self._cim_init_mg_of_id(access_group.id,
raise_error=True)
@@ -2225,8 +2083,7 @@ class Smis(IStorageAreaNetwork):
mask_type = self._mask_type(raise_error=True)
# Workaround for EMC VNX/CX
if mask_type == Smis.MASK_TYPE_GROUP:
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
if cim_sys.path.classname == 'Clar_StorageSystem':
mask_type = Smis.MASK_TYPE_MASK

@@ -2245,8 +2102,7 @@ class Smis(IStorageAreaNetwork):
access_group.id +
"will not do volume_mask()")

- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
cim_css_path = self._get_cim_service_path(
cim_sys.path, 'CIM_ControllerConfigurationService')

@@ -2274,8 +2130,7 @@ class Smis(IStorageAreaNetwork):
"""
cim_vol = self._get_cim_instance_by_id(
'Volume', volume.id, raise_error=True)
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)

cim_gmm_cap = self._c.Associators(
cim_sys.path,
@@ -2369,8 +2224,7 @@ class Smis(IStorageAreaNetwork):
mask_type = self._mask_type(raise_error=True)
# Workaround for EMC VNX/CX
if mask_type == Smis.MASK_TYPE_GROUP:
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
if cim_sys.path.classname == 'Clar_StorageSystem':
mask_type = Smis.MASK_TYPE_MASK

@@ -2379,8 +2233,7 @@ class Smis(IStorageAreaNetwork):
return self._volume_unmask_old(access_group, volume)

def _volume_unmask_old(self, access_group, volume):
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
cim_ccs_path = self._get_cim_service_path(
cim_sys.path, 'CIM_ControllerConfigurationService')

@@ -2522,11 +2375,9 @@ class Smis(IStorageAreaNetwork):
if property_list is None:
property_list = []

- if (not self.fallback_mode and
- self._profile_is_supported(SNIA.MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_6,
- strict=False,
- raise_error=False)):
+ if self._profile_is_supported(SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_6,
+ raise_error=False):
return self._c.Associators(
cim_spc_path,
AssocClass='CIM_AssociatedPrivilege',
@@ -2570,8 +2421,7 @@ class Smis(IStorageAreaNetwork):

# Workaround for EMC VNX/CX
if mask_type == Smis.MASK_TYPE_GROUP:
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
if cim_sys.path.classname == 'Clar_StorageSystem':
mask_type = Smis.MASK_TYPE_MASK

@@ -2610,8 +2460,7 @@ class Smis(IStorageAreaNetwork):

# Workaround for EMC VNX/CX
if mask_type == Smis.MASK_TYPE_GROUP:
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
if cim_sys.path.classname == 'Clar_StorageSystem':
mask_type = Smis.MASK_TYPE_MASK

@@ -2698,31 +2547,30 @@ class Smis(IStorageAreaNetwork):
def _mask_type(self, raise_error=False):
"""
Return Smis.MASK_TYPE_NO_SUPPORT, MASK_TYPE_MASK or MASK_TYPE_GROUP
- For fallback_mode, return MASK_TYPE_MASK
if 'Group Masking and Mapping' profile is supported, return
- MASK_TYPE_GROUP
+ 'MASK_TYPE_GROUP'
+ if 'Masking and Mapping' profile is supported, return
+ 'MASK_TYPE_MASK'

If raise_error == False, just return Smis.MASK_TYPE_NO_SUPPORT
or, raise NO_SUPPORT error.
"""
- if self.fallback_mode:
- return Smis.MASK_TYPE_MASK
- if self._profile_is_supported(SNIA.GROUP_MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_5,
- strict=False,
+ if self._profile_is_supported(SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5,
raise_error=False):
return Smis.MASK_TYPE_GROUP
- if self._profile_is_supported(SNIA.MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ if self._profile_is_supported(SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
raise_error=False):
return Smis.MASK_TYPE_MASK
if raise_error:
raise LsmError(ErrorNumber.NO_SUPPORT,
"Target SMI-S provider does not support "
"%s version %s or %s version %s" %
- (SNIA.MASK_PROFILE, SNIA.SMIS_SPEC_VER_1_4,
- SNIA.GROUP_MASK_PROFILE, SNIA.SMIS_SPEC_VER_1_5))
+ (SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
+ SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5))
return Smis.MASK_TYPE_NO_SUPPORT

@handle_cim_errors
@@ -2730,7 +2578,7 @@ class Smis(IStorageAreaNetwork):
rc = []
mask_type = self._mask_type(raise_error=True)

- cim_sys_pros = self._property_list_of_id('System')
+ cim_sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)

cim_spc_pros = self._cim_spc_pros()
@@ -2741,7 +2589,7 @@ class Smis(IStorageAreaNetwork):
# CIM_RegisteredProfile, but actually they don't support it.
mask_type = Smis.MASK_TYPE_MASK

- system_id = self._sys_id(cim_sys)
+ system_id = SmisSys.sys_id_of_cim_sys(cim_sys)
if mask_type == Smis.MASK_TYPE_GROUP:
cim_init_mg_pros = self._cim_init_mg_pros()
cim_init_mgs = self._cim_init_mg_of(cim_sys.path,
@@ -2800,8 +2648,7 @@ class Smis(IStorageAreaNetwork):
expect_class='CIM_StorageHardwareID')

def _ag_init_add_group(self, access_group, init_id, init_type):
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)

if cim_sys.path.classname == 'Clar_StorageSystem':
raise LsmError(ErrorNumber.NO_SUPPORT,
@@ -2858,8 +2705,7 @@ class Smis(IStorageAreaNetwork):
def _ag_init_add_old(self, access_group, init_id, init_type):
# CIM_StorageHardwareIDManagementService.CreateStorageHardwareID()
# is mandatory since 1.4rev6
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)

if cim_sys.path.classname == 'Clar_StorageSystem':
raise LsmError(ErrorNumber.NO_SUPPORT,
@@ -2905,8 +2751,7 @@ class Smis(IStorageAreaNetwork):
Call CIM_GroupMaskingMappingService.RemoveMembers() against
CIM_InitiatorMaskingGroup.
"""
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)

cim_init_mg_pros = self._cim_init_mg_pros()
cim_init_mg = self._cim_init_mg_of_id(
@@ -2960,8 +2805,7 @@ class Smis(IStorageAreaNetwork):
return self._ag_init_del_old(access_group, init_id)

def _ag_init_del_old(self, access_group, init_id):
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)

cim_spc = self._cim_spc_of_id(access_group.id, raise_error=True)

@@ -3029,11 +2873,9 @@ class Smis(IStorageAreaNetwork):
by ourselves in case URI contain 'system=xxx'.
"""
rc = []
- if not self.fallback_mode:
- self._profile_is_supported(SNIA.DISK_LITE_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=True)
+ self._profile_is_supported(SmisCommon.SNIA_DISK_LITE_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
+ raise_error=True)
cim_disk_pros = Smis._new_disk_cim_disk_pros(flags)
cim_disks = self._enumerate('CIM_DiskDrive', cim_disk_pros)
for cim_disk in cim_disks:
@@ -3251,210 +3093,62 @@ class Smis(IStorageAreaNetwork):

return element_type, unsupported

- def _profile_is_supported(self, profile_name, spec_ver, strict=False,
+ def _profile_is_supported(self, profile_name, spec_ver,
raise_error=False):
"""
Usage:
- Check whether we support certain profile at certain SNIA
- specification version.
- When strict == False(default), profile spec version later or equal
- than require spec_ver will also be consider as found.
- When strict == True, only defined spec_version is allowed.
- Require self.cim_rps containing all CIM_RegisteredProfile
- Will raise LsmError(ErrorNumber.NO_SUPPORT, 'xxx') if raise_error
- is True when nothing found.
+ Just a wrapper of SmisCommon.profile_check().
Parameter:
- profile_name # SNIA.XXXX_PROFILE
- spec_ver # SNIA.SMIS_SPEC_VER_XXX
- strict # False or True. If True, only defined
- # spec_version is consider as supported
- # If false, will return the maximum version of
- # spec.
+ profile_name # SmisCommon.SNIA_XXXX_PROFILE
+ spec_ver # SmisCommon.SMIS_SPEC_VER_XXX
raise_error # Raise LsmError if not found
Returns:
- None # Not supported.
+ True
or
- spec_int # Integer. Converted by _spec_ver_str_to_num()
+ False
"""
- req_ver = _spec_ver_str_to_num(spec_ver)
-
- max_spec_ver_str = None
- max_spec_ver = None
- for cim_rp in self.cim_rps:
- if 'RegisteredName' not in cim_rp or \
- 'RegisteredVersion' not in cim_rp:
- continue
- if cim_rp['RegisteredName'] == profile_name:
- # check spec version
- cur_ver = _spec_ver_str_to_num(cim_rp['RegisteredVersion'])
-
- if strict and cur_ver == req_ver:
- return cur_ver
- elif cur_ver >= req_ver:
- if max_spec_ver is None or \
- cur_ver > max_spec_ver:
- max_spec_ver = cur_ver
- max_spec_ver_str = cim_rp['RegisteredVersion']
- if (strict or max_spec_ver is None) and raise_error:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "SNIA SMI-S %s '%s' profile is not supported" %
- (spec_ver, profile_name))
-
- return max_spec_ver
+ return SmisCommon.profile_check(
+ self._profile_dict, profile_name, spec_ver, raise_error)

def _root_cim_syss(self, property_list=None):
"""
- For fallback mode, this just enumerate CIM_ComputerSystem.
- We require vendor to implement profile registration when using
- "Multiple System Profile".
- For normal mode, this just find out the root CIM_ComputerSystem
- via:
-
- CIM_RegisteredProfile # Root Profile('Array') in interop
- |
- | CIM_ElementConformsToProfile
- v
- CIM_ComputerSystem # vendor namespace
-
- We also assume no matter which version of root profile can lead to
- the same CIM_ComputerSystem instance.
- As CIM_ComputerSystem has no property indicate SNIA SMI-S version,
- this is assumption should work. Tested on EMC SMI-S provider which
- provide 1.4, 1.5, 1.6 root profile.
+ Provide simplfied(less argument) version of SmisSys.root_cim_sys()
+ internally.
"""
- cim_scss_path = []
- id_pros = self._property_list_of_id('System', property_list)
- if property_list is None:
- property_list = id_pros
- else:
- property_list = _merge_list(property_list, id_pros)
+ return SmisSys.root_cim_sys(
+ self._c, self._blk_root_cim_rp, self.system_list, property_list)

- cim_syss = []
- if self.fallback_mode:
- # Fallback mode:
- # Find out the root CIM_ComputerSystem using the fallback method:
- # CIM_StorageConfigurationService # Enumerate
- # |
- # | CIM_HostedService
- # v
- # CIM_ComputerSystem
- # If CIM_StorageConfigurationService is not support neither,
- # we enumerate CIM_ComputerSystem.
- try:
- cim_scss_path = self._c.EnumerateInstanceNames(
- 'CIM_StorageConfigurationService')
- except CIMError as e:
- # If array does not support CIM_StorageConfigurationService
- # we use CIM_ComputerSystem which is mandatory.
- # We might get some non-storage array listed as system.
- # but we would like to take that risk instead of
- # skipping basic support of old SMIS provider.
- if e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- cim_syss = self._c.EnumerateInstances(
- 'CIM_ComputerSystem',
- PropertyList=property_list,
- LocalOnly=False)
- else:
- raise
+ def _cim_sys_of_sys_id(self, sys_id, property_list=None):
+ """
+ Return CIMInstance of
+ """
+ cim_syss = self._root_cim_syss(property_list)
+ for cim_sys in cim_syss:
+ if SmisSys.sys_id_of_cim_sys(cim_sys) == sys_id:
+ return cim_sys
+ raise LsmError(
+ ErrorNumber.NOT_FOUND_SYSTEM,
+ "System %s not found" % sys_id)

- if not cim_syss:
- for cim_scs_path in cim_scss_path:
- cim_tmp = None
- # CIM_ComputerSystem is one-one map to
- # CIM_StorageConfigurationService
- cim_tmp = self._c.Associators(
- cim_scs_path,
- AssocClass='CIM_HostedService',
- ResultClass='CIM_ComputerSystem',
- PropertyList=property_list)
- if cim_tmp and cim_tmp[0]:
- cim_syss.extend([cim_tmp[0]])
- else:
- for cim_rp in self.cim_rps:
- if cim_rp['RegisteredName'] == SNIA.BLK_ROOT_PROFILE and\
- cim_rp['RegisteredOrganization'] == SNIA.REG_ORG_CODE:
- cim_syss = self._c.Associators(
- cim_rp.path,
- ResultClass='CIM_ComputerSystem',
- AssocClass='CIM_ElementConformsToProfile',
- PropertyList=property_list)
- # Any version of 'Array' profile can get us to root
- # CIM_ComputerSystem. startup() already has checked the
- # 1.4 version
- break
- if len(cim_syss) == 0:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Current SMI-S provider does not provide "
- "the root CIM_ComputerSystem associated "
- "to 'Array' CIM_RegisteredProfile. Try "
- "add 'force_fallback_mode=yes' into URI")
-
- # System URI Filtering
- if self.system_list:
- needed_cim_syss = []
- for cim_sys in cim_syss:
- if self._sys_id(cim_sys) in self.system_list:
- needed_cim_syss.extend([cim_sys])
- return needed_cim_syss
- else:
- return cim_syss
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)

- def _fc_tgt_is_supported(self, cim_sys_path):
+ def _fc_tgt_is_supported(self):
"""
Return True if FC Target Port 1.4+ profile is supported.
- For fallback_mode, we call self._cim_fc_tgt_of() and do try-except
"""
- if self.fallback_mode:
- try:
- self._cim_fc_tgt_of(cim_sys_path)
- except CIMError as e:
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- return False
- return True
-
- flag_fc_support = self._profile_is_supported(
- SNIA.FC_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ return self._profile_is_supported(
+ SmisCommon.SNIA_FC_TGT_PORT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
raise_error=False)
- # One more check for NetApp Typo:
- # NetApp: 'FC Target Port'
- # SMI-S: 'FC Target Ports'
- # Bug reported.
- if not flag_fc_support:
- flag_fc_support = self._profile_is_supported(
- 'FC Target Port',
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
- if flag_fc_support:
- return True
- else:
- return False

- def _iscsi_tgt_is_supported(self, cim_sys_path):
+ def _iscsi_tgt_is_supported(self):
"""
- Return True if FC Target Port 1.4+ profile is supported.
- For fallback_mode, we call self._cim_iscsi_pg_of() and do try-except
- For fallback_mode:
- Even CIM_EthernetPort is the control class of iSCSI Target
- Ports profile, but that class is optional. :(
+ Return True if FC Target Port 1.2+ profile is supported.
We use CIM_iSCSIProtocolEndpoint as it's a start point we are
using in our code of target_ports().
"""
- if self.fallback_mode:
- try:
- self._cim_iscsi_pg_of(cim_sys_path)
- except CIMError as e:
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_CLASS:
- return False
- return True
-
- if self._profile_is_supported(SNIA.ISCSI_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ if self._profile_is_supported(SmisCommon.SNIA_ISCSI_TGT_PORT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_2,
raise_error=False):
return True
return False
@@ -3462,15 +3156,11 @@ class Smis(IStorageAreaNetwork):
def _multi_sys_is_supported(self):
"""
Return True if Multiple ComputerSystem 1.4+ profile is supported.
- For fallback_mode, always return True.
Return False else.
"""
- if self.fallback_mode:
- return True
flag_multi_sys_support = self._profile_is_supported(
- SNIA.MULTI_SYS_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ SmisCommon.SNIA_MULTI_SYS_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
raise_error=False)
if flag_multi_sys_support:
return True
@@ -3763,25 +3453,21 @@ class Smis(IStorageAreaNetwork):
cim_fc_tgt_pros = ['UsageRestriction', 'ElementName', 'SystemName',
'PermanentAddress', 'PortDiscriminator',
'LinkTechnology', 'DeviceID']
+ flag_fc_support = self._fc_tgt_is_supported()
+ flag_iscsi_support = self._iscsi_tgt_is_supported()
+ if flag_fc_support is False and flag_iscsi_support is False:
+ raise LsmError(ErrorNumber.NO_SUPPORT,
+ "Target SMI-S provider does not support any of"
+ "these profiles: '%s %s', '%s %s'"
+ % (SmisCommon.SMIS_SPEC_VER_1_4,
+ SmisCommon.SNIA_FC_TGT_PORT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_2,
+ SmisCommon.SNIA_ISCSI_TGT_PORT_PROFILE))

cim_syss = self._root_cim_syss(
- property_list=self._property_list_of_id('System'))
+ property_list=SmisSys.cim_sys_id_pros())
for cim_sys in cim_syss:
- system_id = self._sys_id(cim_sys)
- flag_fc_support = self._fc_tgt_is_supported(cim_sys.path)
- flag_iscsi_support = self._iscsi_tgt_is_supported(cim_sys.path)
-
- # Assuming: if one system does not support target_ports(),
- # all systems from the same provider will not support
- # target_ports().
- if flag_fc_support is False and flag_iscsi_support is False:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Target SMI-S provider does not support any of"
- "these profiles: '%s %s', '%s %s'"
- % (SNIA.SMIS_SPEC_VER_1_4,
- SNIA.FC_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- SNIA.ISCSI_TGT_PORT_PROFILE))
+ system_id = SmisSys.sys_id_of_cim_sys(cim_sys)

if flag_fc_support:
# CIM_FCPort might be not belong to root cim_sys
@@ -3978,14 +3664,9 @@ class Smis(IStorageAreaNetwork):
"""
org_init_id = init_id
init_id = _lsm_init_id_to_snia(init_id)
- if self.fallback_mode:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "access_group_create() is not supported in "
- "fallback mode")

- self._profile_is_supported(SNIA.GROUP_MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_5,
- strict=False,
+ self._profile_is_supported(SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5,
raise_error=True)

if init_type != AccessGroup.INIT_TYPE_WWPN and \
@@ -3994,8 +3675,7 @@ class Smis(IStorageAreaNetwork):
"SMI-S plugin only support creating FC/FCoE WWPN "
"and iSCSI AccessGroup")

- cim_sys = self._get_cim_instance_by_id(
- 'System', system.id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(system.id)
if cim_sys.path.classname == 'Clar_StorageSystem':
# EMC VNX/CX does not support Group M&M, which incorrectly exposed
# in CIM_RegisteredProfile
@@ -4083,14 +3763,8 @@ class Smis(IStorageAreaNetwork):
return self._cim_init_mg_to_lsm(cim_init_mg, system.id)

def access_group_delete(self, access_group, flags=0):
- if self.fallback_mode:
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "access_group_create() is not supported in "
- "fallback mode")
-
- self._profile_is_supported(SNIA.GROUP_MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_5,
- strict=False,
+ self._profile_is_supported(SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5,
raise_error=True)

cim_init_mg = self._cim_init_mg_of_id(
--
1.8.3.1
Andy Grover
2014-09-15 19:16:14 UTC
Permalink
Post by Gris Ge
* Profile register check in plugin_register() has been moved to
SmisCommon.profile_register_load().
* New _plugin_register_vendor_workaround() with NetApp profile register typo
NetApp: 'FC Target Port'
SMI-S: 'FC Target Ports'
* Fallback mode has been removed.
* The 'force_fallback_mode' has been changed to silently ignored.
No provider are facing issue on this.
* Updated self._root_cim_syss() to use SmisSys.root_cim_sys().
* Replaced _get_cim_instance_by_id('System', sys_id) with
self._cim_sys_of_sys_id(sys_id).
* The SNIA class has been merged into SmisCommon.
* Change _iscsi_tgt_is_supported() to check 1.2 version of 'iSCSI Target
Port' profile as it has not been changed since then and many(HP and HDS)
are still using 1.2 version string.
* self._sys_id() -> SmisSys.sys_id_of_cim_sys()
* self._cim_sys_to_lsm -> SmisSys.cim_sys_to_lsm_sys()
* self._cim_sys_pros() -> SmisSys.cim_sys_pros()
* self._property_list_of_id('System') -> SmisSys.cim_sys_id_pros()
Each one of your bullet points should be a patch, please break this up
into smaller pieces.

Regards -- Andy
Post by Gris Ge
Tested on these arrays for 'lsmcli list --type systems' and
* LSI/Avago MegaRAID
* EMC VNX
* EMC CX
* EMC VMAX
* IBM XIV
* Fujitsu DX80S2
* HP 3PAR
* Dell/Compellent Storage Center
* Dot Hill AssuredSAN 4524
* HDS AMS2100
* NetApp ONTAP
* NetApp-E
Tested on this array for 'lsmcli volume-mask', 'lsmcli volume-create' and
* EMC VNX
Plugin_test PASS[1] on EMC VNX.
[1] Except plugin_test failed on IS_MASKED when deleting a volume and access
group.
---
plugin/smispy/smis.py | 586 +++++++++++---------------------------------------
1 file changed, 130 insertions(+), 456 deletions(-)
diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 870c6c1..c433ed0 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -35,6 +35,8 @@ from lsm import (IStorageAreaNetwork, error, uri_parse, LsmError, ErrorNumber,
search_property)
from dmtf import DMTF
+from smis_common import SmisCommon
+from smis_sys import SmisSys
# cim_xxx CIMInstance
"Does not support provided init_type: %d" % init_type)
- BLK_ROOT_PROFILE = 'Array'
- BLK_SRVS_PROFILE = 'Block Services'
- DISK_LITE_PROFILE = 'Disk Drive Lite'
- MULTI_SYS_PROFILE = 'Multiple Computer System'
- MASK_PROFILE = 'Masking and Mapping'
- GROUP_MASK_PROFILE = 'Group Masking and Mapping'
- FC_TGT_PORT_PROFILE = 'FC Target Ports'
- ISCSI_TGT_PORT_PROFILE = 'iSCSI Target Ports'
- SMIS_SPEC_VER_1_4 = '1.4'
- SMIS_SPEC_VER_1_5 = '1.5'
- SMIS_SPEC_VER_1_6 = '1.6'
- REG_ORG_CODE = pywbem.Uint16(11)
-
-
"""
SMI-S plug-ing which exposes a small subset of the overall provided
DMTF_STATUS_POWER_MODE = 18
# DSP 1033 Profile Registration
- DMTF_INTEROP_NAMESPACES = ['interop', 'root/interop']
SMIS_DEFAULT_NAMESPACE = 'interop'
IAAN_WBEM_HTTP_PORT = 5988
self._c = None
self.tmo = 0
self.system_list = None
- self.cim_rps = []
- self.cim_root_profile_dict = dict()
- self.fallback_mode = True # Means we cannot use profile register
self.all_vendor_namespaces = []
self.debug_path = None
+ self._profile_dict = {}
+ self._blk_root_cim_rp = None
+ self._vendor_namespace = None
def _get_cim_instance_by_id(self, class_type, requested_id,
if 'force_fallback_mode' in u['parameters'] and \
- return
+ # force_fallback_mode is not needed any more.
+ # silently ignore.
+ pass
- # Checking profile registration support status unless
- # force_fallback_mode is enabled in URI.
- namespace_check_list = Smis.DMTF_INTEROP_NAMESPACES
- if 'namespace' in u['parameters'] and \
- namespace_check_list.extend([u['parameters']['namespace']])
+ (self._profile_dict, self._blk_root_cim_rp) = \
+ SmisCommon.profile_register_load(self._c)
- self.cim_rps = self._c.EnumerateInstances(
- 'CIM_RegisteredProfile',
- namespace=interop_namespace,
- PropertyList=['RegisteredName', 'RegisteredVersion',
- 'RegisteredOrganization'],
- LocalOnly=False)
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- e[0] == pywbem.CIM_ERR_INVALID_NAMESPACE or \
- pass
- raise
- break
+ self._plugin_register_vendor_workaround()
- self.fallback_mode = False
- self.all_vendor_namespaces = []
- # Support 'Array' profile is step 0 for this whole plugin.
- # We find out all 'Array' CIM_RegisteredProfile and stored
- # them into self.cim_root_profile_dict
- if not self._profile_is_supported(
- SNIA.BLK_ROOT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Target SMI-S provider does not support "
- "SNIA SMI-S SPEC %s '%s' profile" %
- (SNIA.SMIS_SPEC_VER_1_4,
- SNIA.BLK_ROOT_PROFILE))
+ SmisCommon.profile_check(
+ self._profile_dict, SmisCommon.SNIA_BLK_ROOT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4, raise_error=True)
+
+ """
+ Vendor specfic wround for plugin_register()
+ """
+ # NetApp: 'FC Target Port'
+ # SMI-S: 'FC Target Ports'
+ self._profile_dict[SmisCommon.SNIA_FC_TGT_PORT_PROFILE] = \
+ self._profile_dict['FC Target Port']
+ return None
self.tmo = ms
volume_resize()
volume_delete()
"""
- # pools() is mandatory, we will try pools() related methods first
- self._cim_pools_of(cim_sys_path)
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Target SMI-S provider does not support "
- "CIM_StoragePool querying which is "
- "mandatory for pools() method")
- raise
-
# CIM_StorageConfigurationService is optional.
cim_scs_path = self._get_cim_service_path(
cim_sys_path, 'CIM_StorageConfigurationService')
return
- # Assuming provider support disk drive when systems under it
- # support it.
- self._enumerate('CIM_DiskDrive')
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- return
- raise
- if not self._profile_is_supported(SNIA.DISK_LITE_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- return
+ if not self._profile_is_supported(SmisCommon.SNIA_DISK_LITE_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
+ return
cap.set(Capabilities.DISKS)
return
Fallback mode means target provider does not support interop, but
they still need to follow at least SNIA SMI-S 1.4rev6
"""
- cim_ccs_path = self._get_cim_service_path(
- cim_sys_path, 'CIM_ControllerConfigurationService')
- return
-
- elif not self._profile_is_supported(SNIA.MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ if not self._profile_is_supported(SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
return
cap.set(Capabilities.ACCESS_GROUPS)
return
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_WWPN)
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_ISCSI_IQN)
return
- # LSI MegaRAID actually not support FC Target and iSCSI target,
- # They expose empty list of CIM_FCPort
- return
-
- flag_fc_support = False
- flag_iscsi_support = False
- flag_fc_support = True
- flag_iscsi_support = True
- # CIM_FCPort is the control class of FC Targets profile
- self._cim_fc_tgt_of(cim_sys_path)
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- flag_fc_support = False
-
- self._cim_iscsi_pg_of(cim_sys_path)
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- flag_iscsi_support = False
- flag_fc_support = self._profile_is_supported(
- SNIA.FC_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
- # NetApp: 'FC Target Port'
- # SMI-S: 'FC Target Ports'
- # Bug reported.
- flag_fc_support = self._profile_is_supported(
- 'FC Target Port',
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
- flag_iscsi_support = self._profile_is_supported(
- SNIA.ISCSI_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
+ flag_fc_support = self._fc_tgt_is_supported()
+ flag_iscsi_support = self._iscsi_tgt_is_supported()
cap.set(Capabilities.TARGET_PORTS)
cap.set(Capabilities.ACCESS_GROUPS_GRANTED_TO_VOLUME)
cap.set(Capabilities.VOLUMES_ACCESSIBLE_BY_ACCESS_GROUP)
cap.set(Capabilities.VOLUME_MASK)
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_WWPN)
cap.set(Capabilities.ACCESS_GROUP_CREATE_WWPN)
cap.set(Capabilities.ACCESS_GROUP_INITIATOR_ADD_ISCSI_IQN)
cap.set(Capabilities.ACCESS_GROUP_CREATE_ISCSI_IQN)
@handle_cim_errors
- cim_sys = self._get_cim_instance_by_id(
- 'System', system.id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(system.id)
cap = Capabilities()
rc = []
rc = ['SystemName', 'DeviceID']
- rc = ['Name']
rc = ['InstanceID']
"""
return self._id('SystemChild', cim_xxx)
- """
- Return CIM_ComputerSystem['Name']
- """
- return self._id('System', cim_sys)
-
"""
Return CIM_StoragePool['InstanceID']
profile.
"""
rc = []
- cim_sys_pros = self._property_list_of_id("System")
+ cim_sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)
cim_vol_pros = self._cim_vol_pros()
- sys_id = self._sys_id(cim_sys)
+ sys_id = SmisSys.sys_id_of_cim_sys(cim_sys)
pool_pros = self._property_list_of_id('Pool')
pool_id = self._pool_id(cim_pool)
rc = []
cim_pool_pros = self._new_pool_cim_pool_pros()
- cim_sys_pros = self._property_list_of_id("System")
+ cim_sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)
- system_id = self._sys_id(cim_sys)
+ system_id = SmisSys.sys_id_of_cim_sys(cim_sys)
# Skip spare storage pool.
if 'Usage' in cim_pool and \
Find out the system ID for certain CIM_StoragePool.
Will return '' if failed.
"""
- sys_pros = self._property_list_of_id('System')
+ sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._c.Associators(cim_pool.path,
ResultClass='CIM_ComputerSystem',
PropertyList=sys_pros)
- return self._sys_id(cim_syss[0])
+ return SmisSys.sys_id_of_cim_sys(cim_syss[0])
return ''
@handle_cim_errors
total_space, free_space,
status, status_info, system_id)
- # In the case of systems we are assuming that the System Name is
- # unique.
- status = System.STATUS_UNKNOWN
- status_info = ''
-
- (status, status_info) = \
- DMTF.cim_sys_status_of(cim_sys['OperationalStatus'])
-
- return System(cim_sys['Name'], cim_sys['ElementName'], status,
- status_info)
-
- """
- Return a list of properties required to create a LSM System
- """
- cim_sys_pros = self._property_list_of_id('System',
- ['ElementName',
- 'OperationalStatus'])
- return cim_sys_pros
-
@handle_cim_errors
"""
don't check support status here as startup() already checked 'Array'
profile.
"""
- cim_sys_pros = self._cim_sys_pros()
+ cim_sys_pros = SmisSys.cim_sys_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)
- return [Smis._cim_sys_to_lsm(s) for s in cim_syss]
+ return [SmisSys.cim_sys_to_lsm_sys(s) for s in cim_syss]
"""
target ports(all FC and FCoE port for access_group.init_type == WWPN,
and the same to iSCSI)
"""
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
cim_init_mg = self._cim_init_mg_of_id(access_group.id,
raise_error=True)
mask_type = self._mask_type(raise_error=True)
# Workaround for EMC VNX/CX
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
mask_type = Smis.MASK_TYPE_MASK
access_group.id +
"will not do volume_mask()")
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
cim_css_path = self._get_cim_service_path(
cim_sys.path, 'CIM_ControllerConfigurationService')
"""
cim_vol = self._get_cim_instance_by_id(
'Volume', volume.id, raise_error=True)
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
cim_gmm_cap = self._c.Associators(
cim_sys.path,
mask_type = self._mask_type(raise_error=True)
# Workaround for EMC VNX/CX
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
mask_type = Smis.MASK_TYPE_MASK
return self._volume_unmask_old(access_group, volume)
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
cim_ccs_path = self._get_cim_service_path(
cim_sys.path, 'CIM_ControllerConfigurationService')
property_list = []
- if (not self.fallback_mode and
- self._profile_is_supported(SNIA.MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_6,
- strict=False,
+ if self._profile_is_supported(SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_6,
return self._c.Associators(
cim_spc_path,
AssocClass='CIM_AssociatedPrivilege',
# Workaround for EMC VNX/CX
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
mask_type = Smis.MASK_TYPE_MASK
# Workaround for EMC VNX/CX
- cim_sys = self._get_cim_instance_by_id(
- 'System', volume.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(volume.system_id)
mask_type = Smis.MASK_TYPE_MASK
"""
Return Smis.MASK_TYPE_NO_SUPPORT, MASK_TYPE_MASK or MASK_TYPE_GROUP
- For fallback_mode, return MASK_TYPE_MASK
if 'Group Masking and Mapping' profile is supported, return
- MASK_TYPE_GROUP
+ 'MASK_TYPE_GROUP'
+ if 'Masking and Mapping' profile is supported, return
+ 'MASK_TYPE_MASK'
If raise_error == False, just return Smis.MASK_TYPE_NO_SUPPORT
or, raise NO_SUPPORT error.
"""
- return Smis.MASK_TYPE_MASK
- if self._profile_is_supported(SNIA.GROUP_MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_5,
- strict=False,
+ if self._profile_is_supported(SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5,
return Smis.MASK_TYPE_GROUP
- if self._profile_is_supported(SNIA.MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ if self._profile_is_supported(SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
return Smis.MASK_TYPE_MASK
raise LsmError(ErrorNumber.NO_SUPPORT,
"Target SMI-S provider does not support "
"%s version %s or %s version %s" %
- (SNIA.MASK_PROFILE, SNIA.SMIS_SPEC_VER_1_4,
- SNIA.GROUP_MASK_PROFILE, SNIA.SMIS_SPEC_VER_1_5))
+ (SmisCommon.SNIA_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
+ SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5))
return Smis.MASK_TYPE_NO_SUPPORT
@handle_cim_errors
rc = []
mask_type = self._mask_type(raise_error=True)
- cim_sys_pros = self._property_list_of_id('System')
+ cim_sys_pros = SmisSys.cim_sys_id_pros()
cim_syss = self._root_cim_syss(cim_sys_pros)
cim_spc_pros = self._cim_spc_pros()
# CIM_RegisteredProfile, but actually they don't support it.
mask_type = Smis.MASK_TYPE_MASK
- system_id = self._sys_id(cim_sys)
+ system_id = SmisSys.sys_id_of_cim_sys(cim_sys)
cim_init_mg_pros = self._cim_init_mg_pros()
cim_init_mgs = self._cim_init_mg_of(cim_sys.path,
expect_class='CIM_StorageHardwareID')
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
raise LsmError(ErrorNumber.NO_SUPPORT,
# CIM_StorageHardwareIDManagementService.CreateStorageHardwareID()
# is mandatory since 1.4rev6
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
raise LsmError(ErrorNumber.NO_SUPPORT,
Call CIM_GroupMaskingMappingService.RemoveMembers() against
CIM_InitiatorMaskingGroup.
"""
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
cim_init_mg_pros = self._cim_init_mg_pros()
cim_init_mg = self._cim_init_mg_of_id(
return self._ag_init_del_old(access_group, init_id)
- cim_sys = self._get_cim_instance_by_id(
- 'System', access_group.system_id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
cim_spc = self._cim_spc_of_id(access_group.id, raise_error=True)
by ourselves in case URI contain 'system=xxx'.
"""
rc = []
- self._profile_is_supported(SNIA.DISK_LITE_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=True)
+ self._profile_is_supported(SmisCommon.SNIA_DISK_LITE_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
+ raise_error=True)
cim_disk_pros = Smis._new_disk_cim_disk_pros(flags)
cim_disks = self._enumerate('CIM_DiskDrive', cim_disk_pros)
return element_type, unsupported
- def _profile_is_supported(self, profile_name, spec_ver, strict=False,
+ def _profile_is_supported(self, profile_name, spec_ver,
"""
- Check whether we support certain profile at certain SNIA
- specification version.
- When strict == False(default), profile spec version later or equal
- than require spec_ver will also be consider as found.
- When strict == True, only defined spec_version is allowed.
- Require self.cim_rps containing all CIM_RegisteredProfile
- Will raise LsmError(ErrorNumber.NO_SUPPORT, 'xxx') if raise_error
- is True when nothing found.
+ Just a wrapper of SmisCommon.profile_check().
- profile_name # SNIA.XXXX_PROFILE
- spec_ver # SNIA.SMIS_SPEC_VER_XXX
- strict # False or True. If True, only defined
- # spec_version is consider as supported
- # If false, will return the maximum version of
- # spec.
+ profile_name # SmisCommon.SNIA_XXXX_PROFILE
+ spec_ver # SmisCommon.SMIS_SPEC_VER_XXX
raise_error # Raise LsmError if not found
- None # Not supported.
+ True
or
- spec_int # Integer. Converted by _spec_ver_str_to_num()
+ False
"""
- req_ver = _spec_ver_str_to_num(spec_ver)
-
- max_spec_ver_str = None
- max_spec_ver = None
- if 'RegisteredName' not in cim_rp or \
- continue
- # check spec version
- cur_ver = _spec_ver_str_to_num(cim_rp['RegisteredVersion'])
-
- return cur_ver
- if max_spec_ver is None or \
- max_spec_ver = cur_ver
- max_spec_ver_str = cim_rp['RegisteredVersion']
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "SNIA SMI-S %s '%s' profile is not supported" %
- (spec_ver, profile_name))
-
- return max_spec_ver
+ return SmisCommon.profile_check(
+ self._profile_dict, profile_name, spec_ver, raise_error)
"""
- For fallback mode, this just enumerate CIM_ComputerSystem.
- We require vendor to implement profile registration when using
- "Multiple System Profile".
- For normal mode, this just find out the root CIM_ComputerSystem
-
- CIM_RegisteredProfile # Root Profile('Array') in interop
- |
- | CIM_ElementConformsToProfile
- v
- CIM_ComputerSystem # vendor namespace
-
- We also assume no matter which version of root profile can lead to
- the same CIM_ComputerSystem instance.
- As CIM_ComputerSystem has no property indicate SNIA SMI-S version,
- this is assumption should work. Tested on EMC SMI-S provider which
- provide 1.4, 1.5, 1.6 root profile.
+ Provide simplfied(less argument) version of SmisSys.root_cim_sys()
+ internally.
"""
- cim_scss_path = []
- id_pros = self._property_list_of_id('System', property_list)
- property_list = id_pros
- property_list = _merge_list(property_list, id_pros)
+ return SmisSys.root_cim_sys(
+ self._c, self._blk_root_cim_rp, self.system_list, property_list)
- cim_syss = []
- # CIM_StorageConfigurationService # Enumerate
- # |
- # | CIM_HostedService
- # v
- # CIM_ComputerSystem
- # If CIM_StorageConfigurationService is not support neither,
- # we enumerate CIM_ComputerSystem.
- cim_scss_path = self._c.EnumerateInstanceNames(
- 'CIM_StorageConfigurationService')
- # If array does not support CIM_StorageConfigurationService
- # we use CIM_ComputerSystem which is mandatory.
- # We might get some non-storage array listed as system.
- # but we would like to take that risk instead of
- # skipping basic support of old SMIS provider.
- cim_syss = self._c.EnumerateInstances(
- 'CIM_ComputerSystem',
- PropertyList=property_list,
- LocalOnly=False)
- raise
+ """
+ Return CIMInstance of
+ """
+ cim_syss = self._root_cim_syss(property_list)
+ return cim_sys
+ raise LsmError(
+ ErrorNumber.NOT_FOUND_SYSTEM,
+ "System %s not found" % sys_id)
- cim_tmp = None
- # CIM_ComputerSystem is one-one map to
- # CIM_StorageConfigurationService
- cim_tmp = self._c.Associators(
- cim_scs_path,
- AssocClass='CIM_HostedService',
- ResultClass='CIM_ComputerSystem',
- PropertyList=property_list)
- cim_syss.extend([cim_tmp[0]])
- if cim_rp['RegisteredName'] == SNIA.BLK_ROOT_PROFILE and\
- cim_syss = self._c.Associators(
- cim_rp.path,
- ResultClass='CIM_ComputerSystem',
- AssocClass='CIM_ElementConformsToProfile',
- PropertyList=property_list)
- # Any version of 'Array' profile can get us to root
- # CIM_ComputerSystem. startup() already has checked the
- # 1.4 version
- break
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Current SMI-S provider does not provide "
- "the root CIM_ComputerSystem associated "
- "to 'Array' CIM_RegisteredProfile. Try "
- "add 'force_fallback_mode=yes' into URI")
-
- # System URI Filtering
- needed_cim_syss = []
- needed_cim_syss.extend([cim_sys])
- return needed_cim_syss
- return cim_syss
+ cim_sys = self._cim_sys_of_sys_id(access_group.system_id)
"""
Return True if FC Target Port 1.4+ profile is supported.
- For fallback_mode, we call self._cim_fc_tgt_of() and do try-except
"""
- self._cim_fc_tgt_of(cim_sys_path)
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- return False
- return True
-
- flag_fc_support = self._profile_is_supported(
- SNIA.FC_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ return self._profile_is_supported(
+ SmisCommon.SNIA_FC_TGT_PORT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
raise_error=False)
- # NetApp: 'FC Target Port'
- # SMI-S: 'FC Target Ports'
- # Bug reported.
- flag_fc_support = self._profile_is_supported(
- 'FC Target Port',
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
- raise_error=False)
- return True
- return False
"""
- Return True if FC Target Port 1.4+ profile is supported.
- For fallback_mode, we call self._cim_iscsi_pg_of() and do try-except
- Even CIM_EthernetPort is the control class of iSCSI Target
- Ports profile, but that class is optional. :(
+ Return True if FC Target Port 1.2+ profile is supported.
We use CIM_iSCSIProtocolEndpoint as it's a start point we are
using in our code of target_ports().
"""
- self._cim_iscsi_pg_of(cim_sys_path)
- if e[0] == pywbem.CIM_ERR_NOT_SUPPORTED or \
- return False
- return True
-
- if self._profile_is_supported(SNIA.ISCSI_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ if self._profile_is_supported(SmisCommon.SNIA_ISCSI_TGT_PORT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_2,
return True
return False
"""
Return True if Multiple ComputerSystem 1.4+ profile is supported.
- For fallback_mode, always return True.
Return False else.
"""
- return True
flag_multi_sys_support = self._profile_is_supported(
- SNIA.MULTI_SYS_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- strict=False,
+ SmisCommon.SNIA_MULTI_SYS_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_4,
raise_error=False)
return True
cim_fc_tgt_pros = ['UsageRestriction', 'ElementName', 'SystemName',
'PermanentAddress', 'PortDiscriminator',
'LinkTechnology', 'DeviceID']
+ flag_fc_support = self._fc_tgt_is_supported()
+ flag_iscsi_support = self._iscsi_tgt_is_supported()
+ raise LsmError(ErrorNumber.NO_SUPPORT,
+ "Target SMI-S provider does not support any of"
+ "these profiles: '%s %s', '%s %s'"
+ % (SmisCommon.SMIS_SPEC_VER_1_4,
+ SmisCommon.SNIA_FC_TGT_PORT_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_2,
+ SmisCommon.SNIA_ISCSI_TGT_PORT_PROFILE))
cim_syss = self._root_cim_syss(
- property_list=self._property_list_of_id('System'))
+ property_list=SmisSys.cim_sys_id_pros())
- system_id = self._sys_id(cim_sys)
- flag_fc_support = self._fc_tgt_is_supported(cim_sys.path)
- flag_iscsi_support = self._iscsi_tgt_is_supported(cim_sys.path)
-
- # Assuming: if one system does not support target_ports(),
- # all systems from the same provider will not support
- # target_ports().
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "Target SMI-S provider does not support any of"
- "these profiles: '%s %s', '%s %s'"
- % (SNIA.SMIS_SPEC_VER_1_4,
- SNIA.FC_TGT_PORT_PROFILE,
- SNIA.SMIS_SPEC_VER_1_4,
- SNIA.ISCSI_TGT_PORT_PROFILE))
+ system_id = SmisSys.sys_id_of_cim_sys(cim_sys)
# CIM_FCPort might be not belong to root cim_sys
"""
org_init_id = init_id
init_id = _lsm_init_id_to_snia(init_id)
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "access_group_create() is not supported in "
- "fallback mode")
- self._profile_is_supported(SNIA.GROUP_MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_5,
- strict=False,
+ self._profile_is_supported(SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5,
raise_error=True)
if init_type != AccessGroup.INIT_TYPE_WWPN and \
"SMI-S plugin only support creating FC/FCoE WWPN "
"and iSCSI AccessGroup")
- cim_sys = self._get_cim_instance_by_id(
- 'System', system.id, raise_error=True)
+ cim_sys = self._cim_sys_of_sys_id(system.id)
# EMC VNX/CX does not support Group M&M, which incorrectly exposed
# in CIM_RegisteredProfile
return self._cim_init_mg_to_lsm(cim_init_mg, system.id)
- raise LsmError(ErrorNumber.NO_SUPPORT,
- "access_group_create() is not supported in "
- "fallback mode")
-
- self._profile_is_supported(SNIA.GROUP_MASK_PROFILE,
- SNIA.SMIS_SPEC_VER_1_5,
- strict=False,
+ self._profile_is_supported(SmisCommon.SNIA_GROUP_MASK_PROFILE,
+ SmisCommon.SMIS_SPEC_VER_1_5,
raise_error=True)
cim_init_mg = self._cim_init_mg_of_id(
Gris Ge
2014-09-16 09:51:49 UTC
Permalink
Post by Andy Grover
Each one of your bullet points should be a patch, please break this
up into smaller pieces.
Regards -- Andy
Will do.
Than you Andy for the detailed review and suggestions.

Best regards!
--
Gris Ge
Andy Grover
2014-09-15 19:13:54 UTC
Permalink
Post by Gris Ge
1. DMTF
# Store DMTF CIM constants
2. SmisCommon
# Store SNIA constants and share modules for SmisSys and etc.
3. SmisSys # Coming SmisVol, SmisPool, SmisDisk and etc.
# All lsm.System related methods.
4. Smis, MegaRAID, ESeries
# Plugin
5. SmisProxy
# Proxy for MegaRAID, ESeries and Smis plugin.
What I'd like to see in a 0/x message is WHY you are reorganizing. Also,
just like a patch should only do one thing, this patchset should really
only do one (bigger) thing, and this patchset does two things -
reorganize plugins and then improves performance. Make this two patchsets.
Post by Gris Ge
Module dependency rule: a module can only depend on lower(smaller number)
layer modules.
What does this mean and why is it important?
Post by Gris Ge
Before: 5.07 seconds
Now: 2.02 seconds
Before: 0.42 seconds
Now: 0.31 seconds
2. Improve cim_sys retrieving by skip leaf CIM_ComputerSystem.
Before: self._get_cim_instance_by_id('System', system.id)
# It enumerate all CIM_ComputerSystem,
# 1 enumerate call.
Now: self._cim_sys_of_sys_id(system.id)
# Only search in root CIM_ComputerSystem,
# 1 assocication call
No actually performance improvement found as it just save some
data CIM transfer.
Is it a performance improvement or not??? If not then the subject is
wrong. Maybe it's a "round-trip reduction" or something.

Regards -- Andy
Gris Ge
2014-09-16 08:48:09 UTC
Permalink
Post by Andy Grover
What I'd like to see in a 0/x message is WHY you are reorganizing.
Also, just like a patch should only do one thing, this patchset
should really only do one (bigger) thing, and this patchset does two
things - reorganize plugins and then improves performance. Make this
two patchsets.
Thanks for the tips. I will make then into two.
Post by Andy Grover
Post by Gris Ge
Module dependency rule: a module can only depend on lower(smaller number)
layer modules.
What does this mean and why is it important?
Just in case of loop import: A import B which import A.
Assign layer number to modules can easily solve this and also keep code
well organized.
I will mention that in future patch comments.
Post by Andy Grover
Post by Gris Ge
Before: 5.07 seconds
Now: 2.02 seconds
Before: 0.42 seconds
Now: 0.31 seconds
2. Improve cim_sys retrieving by skip leaf CIM_ComputerSystem.
Before: self._get_cim_instance_by_id('System', system.id)
# It enumerate all CIM_ComputerSystem,
# 1 enumerate call.
Now: self._cim_sys_of_sys_id(system.id)
# Only search in root CIM_ComputerSystem,
# 1 assocication call
No actually performance improvement found as it just save some
data CIM transfer.
Is it a performance improvement or not??? If not then the subject is
wrong. Maybe it's a "round-trip reduction" or something.
No performance improvement on EMC VNX as it only has two leaf
CIM_ComputerSystem, saving two instances CIM data will not save too much
network transfer time.
Post by Andy Grover
Regards -- Andy
Thank you again for the comments.
--
Gris Ge
Loading...