Discussion:
[Libstoragemgmt-devel] [PATCH 0/8] SMI-S Plugin: Split lsm.Pool related methods from smis.py to smis_pool.py
Gris Ge
2014-09-29 14:41:47 UTC
Permalink
This patchset is based on master plus these patches:

* [V2 0/3] Move capablities to separate file
* [PATCH V3 00/10] SMI-S plugin: Reorganize code layout.
* [PATCH 0/7] Move lsm.System related methods to smis_sys.py.

Highlights:
1. Improve performance of volume listing in patch 2/8.
2. Improve MegaRAID pool listing in patch 6/8.
3. Use lsm.Pool.plugin_data to store CIMInstanceName for quicker and
better way for converting lsm.Pool to CIM_StoragePool.
Check patch 7/8 for detail.

Any comments will be appreciated.

Thank you in advance.

Gris Ge (8):
SMI-S Plugin: Add smis_pool.py file.
SMI-S Plugin: Move cim_pool query into smis_pool
SMI-S Plugin: Move Pool.id generator to smis_pool
SMI-S Plugin: Add smis_pool.cim_pool_pros()
SMI-S Plugin: Remove pool_create leftover methods
SMI-S Plugin: Move cim_pool converter to smis_pool
SMI-S Plugin: New way to convert lsm_pool to cim_pool
SMI-S Plugin: Move pool_id_of_cim_vol() into smis_pool

packaging/libstoragemgmt.spec.in | 1 +
plugin/Makefile.am | 3 +-
plugin/smispy/dmtf.py | 17 ---
plugin/smispy/smis.py | 259 ++++----------------------------------
plugin/smispy/smis_constants.py | 3 +-
plugin/smispy/smis_pool.py | 261 ++++++++++++++++++++++++++++++++++++++
6 files changed, 290 insertions(+), 254 deletions(-)
create mode 100644 plugin/smispy/smis_pool.py
Gris Ge
2014-09-29 14:41:48 UTC
Permalink
* Empty smis_pool.py with license only.
* Makefile and RPM spec file updated.

Signed-off-by: Gris Ge <***@redhat.com>
---
packaging/libstoragemgmt.spec.in | 1 +
plugin/Makefile.am | 3 ++-
plugin/smispy/smis_pool.py | 16 ++++++++++++++++
3 files changed, 19 insertions(+), 1 deletions(-)
create mode 100644 plugin/smispy/smis_pool.py

diff --git a/packaging/libstoragemgmt.spec.in b/packaging/libstoragemgmt.spec.in
index 6eb1d55..5656548 100644
--- a/packaging/libstoragemgmt.spec.in
+++ b/packaging/libstoragemgmt.spec.in
@@ -319,6 +319,7 @@ fi
%{python_sitelib}/lsm/plugin/smispy/smis_common.*
%{python_sitelib}/lsm/plugin/smispy/smis_cap.*
%{python_sitelib}/lsm/plugin/smispy/smis_sys.*
+%{python_sitelib}/lsm/plugin/smispy/smis_pool.*
%{_bindir}/smispy_lsmplugin

%files netapp-plugin
diff --git a/plugin/Makefile.am b/plugin/Makefile.am
index df411f3..793fb0b 100644
--- a/plugin/Makefile.am
+++ b/plugin/Makefile.am
@@ -30,7 +30,8 @@ smispy_PYTHON = \
smispy/smis_common.py \
smispy/dmtf.py \
smispy/smis_cap.py \
- smispy/smis_sys.py
+ smispy/smis_sys.py \
+ smispy/smis_pool.py

nstordir = $(plugindir)/nstor
nstor_PYTHON = \
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
new file mode 100644
index 0000000..e1f46a5
--- /dev/null
+++ b/plugin/smispy/smis_pool.py
@@ -0,0 +1,16 @@
+## 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>
--
1.7.1
Gris Ge
2014-09-29 14:41:49 UTC
Permalink
* Replace Smis._cim_pools_of() with smis_pool.cim_pools_of_cim_sys_path()
* Merge the pool filtering(remove spare pool and other unused pool) into
smis_pool.cim_pools_of_cim_sys_path().
With this, it save some WBEM calls[1] in volumes().
Performance changes for volumes()(lsmcli list --type volumes):
EMC VNX with 95 volumes:
Old: 13.37s
New: 8.97s

[1] Old codes will try to find volumes on spare pool or other unused pool
which is waster of time.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 72 ++++++++------------------------------------
plugin/smispy/smis_pool.py | 65 +++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 59 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index fc231dd..0883bc1 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -31,6 +31,7 @@ from pywbem import CIMError
from smis_constants import *
import smis_cap
import smis_sys
+import smis_pool

from lsm import (IStorageAreaNetwork, uri_parse, LsmError, ErrorNumber,
JobStatus, md5, Volume, AccessGroup,
@@ -813,7 +814,9 @@ class Smis(IStorageAreaNetwork):
for cim_sys in cim_syss:
sys_id = smis_sys.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):
+ cim_pools = smis_pool.cim_pools_of_cim_sys_path(
+ self._c, cim_sys.path, pool_pros)
+ for cim_pool in cim_pools:
pool_id = self._pool_id(cim_pool)
cim_vols = self._c.Associators(
cim_pool.path,
@@ -831,19 +834,6 @@ class Smis(IStorageAreaNetwork):
rc.extend([vol])
return search_property(rc, search_key, search_value)

- def _cim_pools_of(self, cim_sys_path, property_list=None):
- if property_list is None:
- property_list = ['Primordial']
- else:
- property_list = merge_list(property_list, ['Primordial'])
-
- cim_pools = self._c.Associators(cim_sys_path,
- AssocClass='CIM_HostedStoragePool',
- ResultClass='CIM_StoragePool',
- PropertyList=property_list)
-
- return [p for p in cim_pools if not p["Primordial"]]
-
def _new_pool_cim_pool_pros(self):
"""
Return a list of properties for creating new pool.
@@ -857,17 +847,10 @@ class Smis(IStorageAreaNetwork):
@handle_cim_errors
def pools(self, search_key=None, search_value=None, flags=0):
"""
- We are basing on "Block Services Package" profile version 1.4 or
- later:
- CIM_ComputerSystem
- |
- | (CIM_HostedStoragePool)
- |
- v
- CIM_StoragePool
- As 'Block Services Package' is mandatory for 'Array' profile, we
- don't check support status here as startup() already checked 'Array'
- profile.
+ Convert CIM_StoragePool to lsm.Pool.
+ To list all CIM_StoragePool:
+ 1. List all root CIM_ComputerSystem.
+ 2. List all CIM_StoragePool associated to CIM_ComputerSystem.
"""
rc = []
cim_pool_pros = self._new_pool_cim_pool_pros()
@@ -877,40 +860,11 @@ class Smis(IStorageAreaNetwork):

for cim_sys in cim_syss:
system_id = smis_sys.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 \
- cim_pool['Usage'] == DMTF_POOL_USAGE_SPARE:
- continue
- # Skip IBM ArrayPool and ArraySitePool
- # ArrayPool is holding RAID info.
- # ArraySitePool is holding 8 disks. Predefined by array.
- # ArraySite --(1to1 map) --> Array --(1to1 map)--> Rank
-
- # By design when user get a ELEMENT_TYPE_POOL only pool,
- # user can assume he/she can allocate spaces from that pool
- # to create a new pool with ELEMENT_TYPE_VOLUME or
- # ELEMENT_TYPE_FS ability.
-
- # If we expose them out, we will have two kind of pools
- # (ArrayPool and ArraySitePool) having element_type &
- # ELEMENT_TYPE_POOL, but none of them can create a
- # ELEMENT_TYPE_VOLUME pool.
- # Only RankPool can create a ELEMENT_TYPE_VOLUME pool.
-
- # We are trying to hide the detail to provide a simple
- # abstraction.
- if cim_pool.classname == 'IBMTSDS_ArrayPool' or \
- cim_pool.classname == 'IBMTSDS_ArraySitePool':
- continue
+ cim_pools = smis_pool.cim_pools_of_cim_sys_path(
+ self._c, cim_sys.path, cim_pool_pros)
+ for cim_pool in cim_pools:
+ rc.append(self._new_pool(cim_pool, system_id))

- pool = self._new_pool(cim_pool, system_id)
- if pool:
- rc.extend([pool])
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Failed to retrieve pool information " +
- "from CIM_StoragePool: %s" % cim_pool.path)
return search_property(rc, search_key, search_value)

def _sys_id_of_cim_pool(self, cim_pool):
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index e1f46a5..9a62187 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -14,3 +14,68 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
#
# Author: Gris Ge <***@redhat.com>
+
+from utils import merge_list
+from dmtf import DMTF_POOL_USAGE_SPARE
+
+
+def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
+ """
+ Use this association to get a list of CIM_StoragePool:
+ CIM_ComputerSystem
+ |
+ | (CIM_HostedStoragePool)
+ |
+ v
+ CIM_StoragePool
+ As 'Block Services Package' is mandatory for 'Array' profile which already
+ checked by plugin_register(), we don't do any profile check here.
+ Primordial pool will be eliminated from return list.
+ These pools will be eliminated also:
+ * Spare pool with CIM_StoragePool['Usage'] == DMTF_POOL_USAGE_SPARE
+ * IBM ArrayPool(IBMTSDS_ArrayPool)
+ * IBM ArraySitePool(IBMTSDS_ArraySitePool)
+ """
+ cim_pools = []
+
+ if property_list is None:
+ property_list = ['Primordial', 'Usage']
+ else:
+ property_list = merge_list(property_list, ['Primordial', 'Usage'])
+
+ cim_pools = smis_common.Associators(
+ cim_sys_path,
+ AssocClass='CIM_HostedStoragePool',
+ ResultClass='CIM_StoragePool',
+ PropertyList=property_list)
+
+ rc = []
+ for cim_pool in cim_pools:
+ if 'Primordial' in cim_pool and cim_pool['Primordial']:
+ continue
+ if 'Usage' in cim_pool and cim_pool['Usage'] == DMTF_POOL_USAGE_SPARE:
+ continue
+ # Skip IBM ArrayPool and ArraySitePool
+ # ArrayPool is holding RAID info.
+ # ArraySitePool is holding 8 disks. Predefined by array.
+ # ArraySite --(1to1 map) --> Array --(1to1 map)--> Rank
+
+ # By design when user get a ELEMENT_TYPE_POOL only pool,
+ # user can assume he/she can allocate spaces from that pool
+ # to create a new pool with ELEMENT_TYPE_VOLUME or
+ # ELEMENT_TYPE_FS ability.
+
+ # If we expose them out, we will have two kind of pools
+ # (ArrayPool and ArraySitePool) having element_type &
+ # ELEMENT_TYPE_POOL, but none of them can create a
+ # ELEMENT_TYPE_VOLUME pool.
+ # Only RankPool can create a ELEMENT_TYPE_VOLUME pool.
+
+ # We are trying to hide the detail to provide a simple
+ # abstraction.
+ if cim_pool.classname == 'IBMTSDS_ArrayPool' or \
+ cim_pool.classname == 'IBMTSDS_ArraySitePool':
+ continue
+ rc.append(cim_pool)
+
+ return rc
--
1.7.1
Gris Ge
2014-09-29 14:41:50 UTC
Permalink
* Replace Smis._pool_id() with smis_pool.pool_id_of_cim_pool()
* Replace Smis._property_list_of_id(self, 'Pool') with
smis_pool.cim_pool_id_pros()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 0883bc1..abd323c 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -396,8 +396,6 @@ class Smis(IStorageAreaNetwork):
rc = []
if class_type == 'Volume':
rc = ['SystemName', 'DeviceID']
- elif class_type == 'Pool':
- rc = ['InstanceID']
elif class_type == 'SystemChild':
rc = ['SystemName']
elif class_type == 'Disk':
@@ -422,12 +420,6 @@ class Smis(IStorageAreaNetwork):
"""
return self._id('SystemChild', cim_xxx)

- def _pool_id(self, cim_pool):
- """
- Return CIM_StoragePool['InstanceID']
- """
- return self._id('Pool', cim_pool)
-
def _vol_id(self, cim_vol):
"""
Return the MD5 hash of CIM_StorageVolume['SystemName'] and
@@ -512,7 +504,7 @@ class Smis(IStorageAreaNetwork):
AssocClass='CIM_AllocatedFromStoragePool',
ResultClass='CIM_StoragePool',
PropertyList=property_list)[0]
- return self._pool_id(cim_pool)
+ return smis_pool.pool_id_of_cim_pool(cim_pool)

@staticmethod
def _get_vol_other_id_info(cv):
@@ -813,11 +805,11 @@ class Smis(IStorageAreaNetwork):
cim_vol_pros = self._cim_vol_pros()
for cim_sys in cim_syss:
sys_id = smis_sys.sys_id_of_cim_sys(cim_sys)
- pool_pros = self._property_list_of_id('Pool')
+ pool_pros = smis_pool.cim_pool_id_pros()
cim_pools = smis_pool.cim_pools_of_cim_sys_path(
self._c, cim_sys.path, pool_pros)
for cim_pool in cim_pools:
- pool_id = self._pool_id(cim_pool)
+ pool_id = smis_pool.pool_id_of_cim_pool(cim_pool)
cim_vols = self._c.Associators(
cim_pool.path,
AssocClass='CIM_AllocatedFromStoragePool',
@@ -838,7 +830,7 @@ class Smis(IStorageAreaNetwork):
"""
Return a list of properties for creating new pool.
"""
- pool_pros = self._property_list_of_id('Pool')
+ pool_pros = smis_pool.cim_pool_id_pros()
pool_pros.extend(['ElementName', 'TotalManagedSpace',
'RemainingManagedSpace', 'Usage',
'OperationalStatus'])
@@ -890,7 +882,7 @@ class Smis(IStorageAreaNetwork):
system_id = self._sys_id_of_cim_pool(cim_pool)

status_info = ''
- pool_id = self._pool_id(cim_pool)
+ pool_id = smis_pool.pool_id_of_cim_pool(cim_pool)
name = ''
total_space = Pool.TOTAL_SPACE_NOT_FOUND
free_space = Pool.FREE_SPACE_NOT_FOUND
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index 9a62187..b0767d8 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -17,6 +17,7 @@

from utils import merge_list
from dmtf import DMTF_POOL_USAGE_SPARE
+from lsm import LsmError, ErrorNumber


def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
@@ -79,3 +80,21 @@ def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
rc.append(cim_pool)

return rc
+
+
+def cim_pool_id_pros():
+ """
+ Return a list of CIM_StoragePool properties required to generate
+ lsm.Pool.id
+ """
+ return ['InstanceID']
+
+
+def pool_id_of_cim_pool(cim_pool):
+ if 'InstanceID' in cim_pool:
+ return cim_pool['InstanceID']
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "pool_id_of_cim_pool(): Got CIM_StoragePool with no 'InstanceID' "
+ "property: %s, %s" % (cim_pool.items(), cim_pool.path))
--
1.7.1
Gris Ge
2014-09-29 14:41:51 UTC
Permalink
* Replaced Smis._new_pool_cim_pool_pros(self) with
smis_pool.cim_pool_pros()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index abd323c..1def4ce 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -498,7 +498,7 @@ class Smis(IStorageAreaNetwork):
Takes a CIMInstance that represents a volume and returns the pool
id for that volume.
"""
- property_list = Smis._property_list_of_id('Pool')
+ property_list = smis_pool.cim_pool_id_pros()
cim_pool = self._c.Associators(
cim_vol.path,
AssocClass='CIM_AllocatedFromStoragePool',
@@ -692,7 +692,7 @@ class Smis(IStorageAreaNetwork):
For SYNC CreateOrModifyElementFromStoragePool action.
The new CIM_StoragePool is stored in out['Pool']
"""
- pool_pros = self._new_pool_cim_pool_pros()
+ pool_pros = smis_pool.cim_pool_pros()

if 'Pool' in out:
cim_new_pool = self._c.GetInstance(
@@ -771,7 +771,7 @@ class Smis(IStorageAreaNetwork):
"""
Given a CIMInstance of CIM_ConcreteJob, return a LSM Pool
"""
- pool_pros = self._new_pool_cim_pool_pros()
+ pool_pros = smis_pool.cim_pool_pros()
cim_pools = self._c.Associators(cim_job.path,
AssocClass='CIM_AffectedJobElement',
ResultClass='CIM_StoragePool',
@@ -826,16 +826,6 @@ class Smis(IStorageAreaNetwork):
rc.extend([vol])
return search_property(rc, search_key, search_value)

- def _new_pool_cim_pool_pros(self):
- """
- Return a list of properties for creating new pool.
- """
- pool_pros = smis_pool.cim_pool_id_pros()
- pool_pros.extend(['ElementName', 'TotalManagedSpace',
- 'RemainingManagedSpace', 'Usage',
- 'OperationalStatus'])
- return pool_pros
-
@handle_cim_errors
def pools(self, search_key=None, search_value=None, flags=0):
"""
@@ -845,7 +835,7 @@ class Smis(IStorageAreaNetwork):
2. List all CIM_StoragePool associated to CIM_ComputerSystem.
"""
rc = []
- cim_pool_pros = self._new_pool_cim_pool_pros()
+ cim_pool_pros = smis_pool.cim_pool_pros()

cim_sys_pros = smis_sys.cim_sys_id_pros()
cim_syss = smis_sys.root_cim_sys(self._c, cim_sys_pros)
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index b0767d8..2fec3bc 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -98,3 +98,14 @@ def pool_id_of_cim_pool(cim_pool):
ErrorNumber.PLUGIN_BUG,
"pool_id_of_cim_pool(): Got CIM_StoragePool with no 'InstanceID' "
"property: %s, %s" % (cim_pool.items(), cim_pool.path))
+
+
+def cim_pool_pros():
+ """
+ Return a list of CIM_StoragePool properties required to generate lsm.Pool.
+ """
+ pool_pros = cim_pool_id_pros()
+ pool_pros.extend(['ElementName', 'TotalManagedSpace',
+ 'RemainingManagedSpace', 'Usage',
+ 'OperationalStatus'])
+ return pool_pros
--
1.7.1
Gris Ge
2014-09-29 14:41:52 UTC
Permalink
* Remove smis_constants.JOB_RETRIEVE_POOL
* Remove Smis._new_pool_from_name() and Smis._new_pool_from_job()

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 32 --------------------------------
plugin/smispy/smis_constants.py | 3 +--
2 files changed, 1 insertions(+), 34 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 1def4ce..9ba51d2 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -182,8 +182,6 @@ class Smis(IStorageAreaNetwork):
if rc == INVOKE_OK:
if retrieve_data == JOB_RETRIEVE_VOLUME:
return None, self._new_vol_from_name(out)
- elif retrieve_data == JOB_RETRIEVE_POOL:
- return None, self._new_pool_from_name(out)
else:
return None, None

@@ -343,8 +341,6 @@ class Smis(IStorageAreaNetwork):
(ignore, retrieve_data) = self._parse_job_id(job_id)
if retrieve_data == JOB_RETRIEVE_VOLUME:
completed_item = self._new_vol_from_job(cim_job)
- elif retrieve_data == JOB_RETRIEVE_POOL:
- completed_item = self._new_pool_from_job(cim_job)
else:
status = JobStatus.ERROR

@@ -687,23 +683,6 @@ class Smis(IStorageAreaNetwork):

return self._new_vol(instance)

- def _new_pool_from_name(self, out):
- """
- For SYNC CreateOrModifyElementFromStoragePool action.
- The new CIM_StoragePool is stored in out['Pool']
- """
- pool_pros = smis_pool.cim_pool_pros()
-
- if 'Pool' in out:
- cim_new_pool = self._c.GetInstance(
- out['Pool'],
- PropertyList=pool_pros, LocalOnly=False)
- return self._new_pool(cim_new_pool)
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Got not new Pool from out of InvokeMethod" +
- "when CreateOrModifyElementFromStoragePool")
-
def _cim_spc_pros(self):
"""
Return a list of properties required to build new AccessGroup.
@@ -767,17 +746,6 @@ class Smis(IStorageAreaNetwork):
return self._new_vol(self._c.GetInstance(a.path, LocalOnly=False))
return None

- def _new_pool_from_job(self, cim_job):
- """
- Given a CIMInstance of CIM_ConcreteJob, return a LSM Pool
- """
- pool_pros = smis_pool.cim_pool_pros()
- cim_pools = self._c.Associators(cim_job.path,
- AssocClass='CIM_AffectedJobElement',
- ResultClass='CIM_StoragePool',
- PropertyList=pool_pros)
- return self._new_pool(cim_pools[0])
-
@handle_cim_errors
def volumes(self, search_key=None, search_value=None, flags=0):
"""
diff --git a/plugin/smispy/smis_constants.py b/plugin/smispy/smis_constants.py
index 0536cf5..2df9144 100644
--- a/plugin/smispy/smis_constants.py
+++ b/plugin/smispy/smis_constants.py
@@ -61,7 +61,6 @@ VOL_NAME_SPACE_SNVM = 7

JOB_RETRIEVE_NONE = 0
JOB_RETRIEVE_VOLUME = 1
-JOB_RETRIEVE_POOL = 2

IAAN_WBEM_HTTP_PORT = 5988
IAAN_WBEM_HTTPS_PORT = 5989
@@ -129,4 +128,4 @@ class Synchronized(object):
VOL_OP_STATUS_DORMANT) = (2, 3, 6, 8, 15)

# SMI-S ExposePaths device access enumerations
-(EXPOSE_PATHS_DA_READ_WRITE, EXPOSE_PATHS_DA_READ_ONLY) = (2, 3)
\ No newline at end of file
+(EXPOSE_PATHS_DA_READ_WRITE, EXPOSE_PATHS_DA_READ_ONLY) = (2, 3)
--
1.7.1
Gris Ge
2014-09-29 14:41:53 UTC
Permalink
* Move DMTF.cim_pool_status_of() to
smis_pool._pool_status_of_cim_pool().
# This method is for smis_pool internal use only now.

* Move Smis._pool_element_type() to
smis_pool._pool_element_type()
# This method is for smis_pool internal use only now.

* Removed Smis._sys_id_of_cim_pool() as no method require it.

* Add MegaRAID workaround for smis_pool._pool_element_type()
to improve performance by returning (Pool.ELEMENT_TYPE_VOLUME, 0)
directly. Tested on a remote MegaRAID card with two pools:
Old: 10.10s # With all pending patches but this one.
New: 4.61s

* Move Smis._new_pool() to smis_pool.cim_pool_to_lsm_pool()

* Including three changes in patch as _pool_status_of_cim_pool() and
_pool_element_type() is now internal method for
smis_pool.cim_pool_to_lsm_pool() now.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/dmtf.py | 17 ------
plugin/smispy/smis.py | 105 +--------------------------------------
plugin/smispy/smis_pool.py | 117 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 118 insertions(+), 121 deletions(-)

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index d020a9a..c5ea3fe 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -96,23 +96,6 @@ class DMTF(object):
return status, " ".join(status_info_list)


- _LSM_POOL_OP_STATUS_CONV = {
- OP_STATUS_OK: Pool.STATUS_OK,
- OP_STATUS_ERROR: Pool.STATUS_ERROR,
- OP_STATUS_DEGRADED: Pool.STATUS_DEGRADED,
- OP_STATUS_NON_RECOVERABLE_ERROR: Pool.STATUS_ERROR,
- OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: Pool.STATUS_ERROR,
- }
-
- @staticmethod
- def cim_pool_status_of(dmtf_op_status_list):
- """
- Convert CIM_StoragePool['OperationalStatus'] to LSM
- """
- return DMTF.dmtf_op_status_list_conv(
- DMTF._LSM_POOL_OP_STATUS_CONV, dmtf_op_status_list,
- Pool.STATUS_UNKNOWN, Pool.STATUS_OTHER)
-
EMC_DISK_STATUS_REMOVED = 32768

_LSM_DISK_OP_STATUS_CONV = {
diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 9ba51d2..46d1dbb 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -813,54 +813,12 @@ class Smis(IStorageAreaNetwork):
cim_pools = smis_pool.cim_pools_of_cim_sys_path(
self._c, cim_sys.path, cim_pool_pros)
for cim_pool in cim_pools:
- rc.append(self._new_pool(cim_pool, system_id))
+ rc.append(
+ smis_pool.cim_pool_to_lsm_pool(
+ self._c, cim_pool, system_id))

return search_property(rc, search_key, search_value)

- def _sys_id_of_cim_pool(self, cim_pool):
- """
- Find out the system ID for certain CIM_StoragePool.
- Will return '' if failed.
- """
- sys_pros = smis_sys.cim_sys_id_pros()
- cim_syss = self._c.Associators(cim_pool.path,
- ResultClass='CIM_ComputerSystem',
- PropertyList=sys_pros)
- if len(cim_syss) == 1:
- return smis_sys.sys_id_of_cim_sys(cim_syss[0])
- return ''
-
- @handle_cim_errors
- def _new_pool(self, cim_pool, system_id=''):
- """
- Return a Pool object base on information of cim_pool.
- Assuming cim_pool already holding correct properties.
- """
- if not system_id:
- system_id = self._sys_id_of_cim_pool(cim_pool)
-
- status_info = ''
- pool_id = smis_pool.pool_id_of_cim_pool(cim_pool)
- name = ''
- total_space = Pool.TOTAL_SPACE_NOT_FOUND
- free_space = Pool.FREE_SPACE_NOT_FOUND
- status = Pool.STATUS_OK
- if 'ElementName' in cim_pool:
- name = cim_pool['ElementName']
- if 'TotalManagedSpace' in cim_pool:
- total_space = cim_pool['TotalManagedSpace']
- if 'RemainingManagedSpace' in cim_pool:
- free_space = cim_pool['RemainingManagedSpace']
- if 'OperationalStatus' in cim_pool:
- (status, status_info) = DMTF.cim_pool_status_of(
- cim_pool['OperationalStatus'])
-
- element_type, unsupported = self._pool_element_type(cim_pool)
-
- return Pool(pool_id, name, element_type, unsupported,
- total_space, free_space,
- status, status_info, system_id)
-
@handle_cim_errors
def systems(self, flags=0):
"""
@@ -2359,63 +2317,6 @@ class Smis(IStorageAreaNetwork):
"requested CIM_StorageExtent %s" %
cim_pri_ext_path)

- def _pool_element_type(self, cim_pool):
-
- element_type = 0
- unsupported = 0
-
- # check whether current pool support create volume or not.
- cim_sccs = self._c.Associators(
- cim_pool.path,
- AssocClass='CIM_ElementCapabilities',
- ResultClass='CIM_StorageConfigurationCapabilities',
- PropertyList=['SupportedStorageElementFeatures',
- 'SupportedStorageElementTypes'])
- # Associate StorageConfigurationCapabilities to StoragePool
- # is experimental in SNIA 1.6rev4, Block Book PDF Page 68.
- # Section 5.1.6 StoragePool, StorageVolume and LogicalDisk
- # Manipulation, Figure 9 - Capabilities Specific to a StoragePool
- if len(cim_sccs) == 1:
- cim_scc = cim_sccs[0]
- if 'SupportedStorageElementFeatures' in cim_scc:
- supported_features = cim_scc['SupportedStorageElementFeatures']
-
- if DMTF_SUPPORT_VOL_CREATE in supported_features:
- element_type |= Pool.ELEMENT_TYPE_VOLUME
- if DMTF_SUPPORT_ELEMENT_EXPAND not in supported_features:
- unsupported |= Pool.UNSUPPORTED_VOLUME_GROW
- if DMTF_SUPPORT_ELEMENT_REDUCE not in supported_features:
- unsupported |= Pool.UNSUPPORTED_VOLUME_SHRINK
-
- else:
- # IBM DS 8000 does not support StorageConfigurationCapabilities
- # per pool yet. They has been informed. Before fix, use a quick
- # workaround.
- # TODO: Currently, we don't have a way to detect
- # Pool.ELEMENT_TYPE_POOL
- # but based on knowing definition of each vendor.
- if cim_pool.classname == 'IBMTSDS_VirtualPool' or \
- cim_pool.classname == 'IBMTSDS_ExtentPool':
- element_type = Pool.ELEMENT_TYPE_VOLUME
- elif cim_pool.classname == 'IBMTSDS_RankPool':
- element_type = Pool.ELEMENT_TYPE_POOL
- elif cim_pool.classname == 'LSIESG_StoragePool':
- element_type = Pool.ELEMENT_TYPE_VOLUME
-
- if 'Usage' in cim_pool:
- usage = cim_pool['Usage']
-
- if usage == DMTF_POOL_USAGE_UNRESTRICTED:
- element_type |= Pool.ELEMENT_TYPE_VOLUME
- if usage == DMTF_POOL_USAGE_RESERVED_FOR_SYSTEM or \
- usage > DMTF_POOL_USAGE_DELTA:
- element_type |= Pool.ELEMENT_TYPE_SYS_RESERVED
- if usage == DMTF_POOL_USAGE_DELTA:
- # We blitz all the other elements types for this designation
- element_type = Pool.ELEMENT_TYPE_DELTA
-
- return element_type, unsupported
-
@staticmethod
def _is_frontend_fc_tgt(cim_fc_tgt):
"""
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index 2fec3bc..a64c628 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -16,8 +16,8 @@
# Author: Gris Ge <***@redhat.com>

from utils import merge_list
-from dmtf import DMTF_POOL_USAGE_SPARE
-from lsm import LsmError, ErrorNumber
+from dmtf import *
+from lsm import LsmError, ErrorNumber, Pool


def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
@@ -109,3 +109,116 @@ def cim_pool_pros():
'RemainingManagedSpace', 'Usage',
'OperationalStatus'])
return pool_pros
+
+
+def _pool_element_type(smis_common, cim_pool):
+ """
+ Return a set (Pool.element_type, Pool.unsupported)
+ Using CIM_StorageConfigurationCapabilities
+ 'SupportedStorageElementFeatures' and 'SupportedStorageElementTypes'
+ property.
+ For MegaRAID, just return (Pool.ELEMENT_TYPE_VOLUME, 0)
+ """
+ if smis_common.is_megaraid():
+ return Pool.ELEMENT_TYPE_VOLUME, 0
+
+ element_type = 0
+ unsupported = 0
+
+ # check whether current pool support create volume or not.
+ cim_sccs = smis_common.Associators(
+ cim_pool.path,
+ AssocClass='CIM_ElementCapabilities',
+ ResultClass='CIM_StorageConfigurationCapabilities',
+ PropertyList=['SupportedStorageElementFeatures',
+ 'SupportedStorageElementTypes'])
+ # Associate StorageConfigurationCapabilities to StoragePool
+ # is experimental in SNIA 1.6rev4, Block Book PDF Page 68.
+ # Section 5.1.6 StoragePool, StorageVolume and LogicalDisk
+ # Manipulation, Figure 9 - Capabilities Specific to a StoragePool
+ if len(cim_sccs) == 1:
+ cim_scc = cim_sccs[0]
+ if 'SupportedStorageElementFeatures' in cim_scc:
+ supported_features = cim_scc['SupportedStorageElementFeatures']
+
+ if DMTF_SUPPORT_VOL_CREATE in supported_features:
+ element_type |= Pool.ELEMENT_TYPE_VOLUME
+ if DMTF_SUPPORT_ELEMENT_EXPAND not in supported_features:
+ unsupported |= Pool.UNSUPPORTED_VOLUME_GROW
+ if DMTF_SUPPORT_ELEMENT_REDUCE not in supported_features:
+ unsupported |= Pool.UNSUPPORTED_VOLUME_SHRINK
+
+ else:
+ # IBM DS 8000 does not support StorageConfigurationCapabilities
+ # per pool yet. They has been informed. Before fix, use a quick
+ # workaround.
+ # TODO: Currently, we don't have a way to detect
+ # Pool.ELEMENT_TYPE_POOL
+ # but based on knowing definition of each vendor.
+ if cim_pool.classname == 'IBMTSDS_VirtualPool' or \
+ cim_pool.classname == 'IBMTSDS_ExtentPool':
+ element_type = Pool.ELEMENT_TYPE_VOLUME
+ elif cim_pool.classname == 'IBMTSDS_RankPool':
+ element_type = Pool.ELEMENT_TYPE_POOL
+ elif cim_pool.classname == 'LSIESG_StoragePool':
+ element_type = Pool.ELEMENT_TYPE_VOLUME
+
+ if 'Usage' in cim_pool:
+ usage = cim_pool['Usage']
+
+ if usage == DMTF_POOL_USAGE_UNRESTRICTED:
+ element_type |= Pool.ELEMENT_TYPE_VOLUME
+ if usage == DMTF_POOL_USAGE_RESERVED_FOR_SYSTEM or \
+ usage > DMTF_POOL_USAGE_DELTA:
+ element_type |= Pool.ELEMENT_TYPE_SYS_RESERVED
+ if usage == DMTF_POOL_USAGE_DELTA:
+ # We blitz all the other elements types for this designation
+ element_type = Pool.ELEMENT_TYPE_DELTA
+
+ return element_type, unsupported
+
+
+_LSM_POOL_OP_STATUS_CONV = {
+ DMTF.OP_STATUS_OK: Pool.STATUS_OK,
+ DMTF.OP_STATUS_ERROR: Pool.STATUS_ERROR,
+ DMTF.OP_STATUS_DEGRADED: Pool.STATUS_DEGRADED,
+ DMTF.OP_STATUS_NON_RECOVERABLE_ERROR: Pool.STATUS_ERROR,
+ DMTF.OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: Pool.STATUS_ERROR,
+}
+
+
+def _pool_status_of_cim_pool(dmtf_op_status_list):
+ """
+ Convert CIM_StoragePool['OperationalStatus'] to LSM
+ """
+ return DMTF.dmtf_op_status_list_conv(
+ _LSM_POOL_OP_STATUS_CONV, dmtf_op_status_list,
+ Pool.STATUS_UNKNOWN, Pool.STATUS_OTHER)
+
+
+def cim_pool_to_lsm_pool(smis_common, cim_pool, system_id):
+ """
+ Return a Pool object base on information of cim_pool.
+ Assuming cim_pool already holding correct properties.
+ """
+ status_info = ''
+ pool_id = pool_id_of_cim_pool(cim_pool)
+ name = ''
+ total_space = Pool.TOTAL_SPACE_NOT_FOUND
+ free_space = Pool.FREE_SPACE_NOT_FOUND
+ status = Pool.STATUS_OK
+ if 'ElementName' in cim_pool:
+ name = cim_pool['ElementName']
+ if 'TotalManagedSpace' in cim_pool:
+ total_space = cim_pool['TotalManagedSpace']
+ if 'RemainingManagedSpace' in cim_pool:
+ free_space = cim_pool['RemainingManagedSpace']
+ if 'OperationalStatus' in cim_pool:
+ (status, status_info) = _pool_status_of_cim_pool(
+ cim_pool['OperationalStatus'])
+
+ element_type, unsupported = _pool_element_type(smis_common, cim_pool)
+
+ return Pool(pool_id, name, element_type, unsupported,
+ total_space, free_space,
+ status, status_info, system_id)
--
1.7.1
Gris Ge
2014-09-29 14:41:54 UTC
Permalink
* Using cPickle to save the CIMInstanceName of CIM_StoragePool into
lsm.Pool.plugin_data and convert it back when needed:
* Save:
Pool.plugin_data = cPickle.dumps(cim_pool.path)

* Load:
cPickle.loads(lsm_pool.plugin_data)

* Replace Smis._get_cim_instance_by_id(self, 'Pool', pool.id) with
smis_pool.lsm_pool_to_cim_pool_path(smis_common, pool)

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 46d1dbb..a437f52 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -878,11 +878,12 @@ class Smis(IStorageAreaNetwork):
# Get the Configuration service for the system we are interested in.
scs = self._c.get_class_instance('CIM_StorageConfigurationService',
'SystemName', pool.system_id)
- sp = self._get_cim_instance_by_id('Pool', pool.id)
+ cim_pool_path = smis_pool.lsm_pool_to_cim_pool_path(
+ self._c, pool)

in_params = {'ElementName': volume_name,
'ElementType': pywbem.Uint16(2),
- 'InPool': sp.path,
+ 'InPool': cim_pool_path,
'Size': pywbem.Uint64(size_bytes)}

try:
@@ -1155,11 +1156,9 @@ class Smis(IStorageAreaNetwork):
rs = self._c.get_class_instance("CIM_ReplicationService", 'SystemName',
volume_src.system_id,
raise_error=False)
-
+ cim_pool_path = None
if pool is not None:
- cim_pool = self._get_cim_instance_by_id('Pool', pool.id)
- else:
- cim_pool = None
+ cim_pool_path = smis_pool.lsm_pool_to_cim_pool_path(self._c, pool)

lun = self._get_cim_instance_by_id('Volume', volume_src.id)

@@ -1201,8 +1200,8 @@ class Smis(IStorageAreaNetwork):
'SourceElement': lun.path}
if rs:

- if cim_pool is not None:
- in_params['TargetPool'] = cim_pool.path
+ if cim_pool_path is not None:
+ in_params['TargetPool'] = cim_pool_path

try:

diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index a64c628..46f0445 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -18,6 +18,7 @@
from utils import merge_list
from dmtf import *
from lsm import LsmError, ErrorNumber, Pool
+import cPickle


def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
@@ -219,6 +220,25 @@ def cim_pool_to_lsm_pool(smis_common, cim_pool, system_id):

element_type, unsupported = _pool_element_type(smis_common, cim_pool)

+ plugin_data = cPickle.dumps(cim_pool.path)
+
return Pool(pool_id, name, element_type, unsupported,
total_space, free_space,
- status, status_info, system_id)
+ status, status_info, system_id, plugin_data)
+
+def lsm_pool_to_cim_pool_path(smis_common, lsm_pool):
+ """
+ Convert lsm.Pool to CIMInstanceName of CIM_StoragePool using
+ lsm.Pool.plugin_data
+ """
+ if not lsm_pool.plugin_data:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "Got lsm.Pool instance with empty plugin_data")
+ if smis_common.system_list and \
+ lsm_pool.system_id not in smis_common.system_list:
+ raise LsmError(
+ ErrorNumber.NOT_FOUND_SYSTEM,
+ "System filtered in URI")
+
+ return cPickle.loads(lsm_pool.plugin_data)
--
1.7.1
Gris Ge
2014-09-29 14:41:55 UTC
Permalink
* Replace Smis._get_pool_from_vol() with smis_pool.pool_id_of_cim_vol()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index a437f52..56b2912 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -489,19 +489,6 @@ class Smis(IStorageAreaNetwork):
retrieve_data = int(tmp_list[1])
return (md5_str, retrieve_data)

- def _get_pool_from_vol(self, cim_vol):
- """
- Takes a CIMInstance that represents a volume and returns the pool
- id for that volume.
- """
- property_list = smis_pool.cim_pool_id_pros()
- cim_pool = self._c.Associators(
- cim_vol.path,
- AssocClass='CIM_AllocatedFromStoragePool',
- ResultClass='CIM_StoragePool',
- PropertyList=property_list)[0]
- return smis_pool.pool_id_of_cim_pool(cim_pool)
-
@staticmethod
def _get_vol_other_id_info(cv):
other_id = None
@@ -559,7 +546,7 @@ class Smis(IStorageAreaNetwork):
#to not call this very often.
if pool_id is None:
#Go an retrieve the pool id
- pool_id = self._get_pool_from_vol(cv)
+ pool_id = smis_pool.pool_id_of_cim_vol(self._c, cv.path)

if sys_id is None:
sys_id = cv['SystemName']
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index 46f0445..fe77a66 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -242,3 +242,20 @@ def lsm_pool_to_cim_pool_path(smis_common, lsm_pool):
"System filtered in URI")

return cPickle.loads(lsm_pool.plugin_data)
+
+def pool_id_of_cim_vol(smis_common, cim_vol_path):
+ """
+ Find out the lsm.Pool.id of CIM_StorageVolume
+ """
+ property_list = cim_pool_id_pros()
+ cim_pools = smis_common.Associators(
+ cim_vol_path,
+ AssocClass='CIM_AllocatedFromStoragePool',
+ ResultClass='CIM_StoragePool',
+ PropertyList=property_list)
+ if len(cim_pools) != 1:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "pool_id_of_cim_vol(): Got unexpected count(%d) of cim_pool "
+ "assocated to cim_vol: %s, %s" % (cim_vol_path, cim_pools))
+ return pool_id_of_cim_pool(cim_pools[0])
--
1.7.1
Tony Asleson
2014-09-29 23:08:52 UTC
Permalink
Post by Gris Ge
* [V2 0/3] Move capablities to separate file
* [PATCH V3 00/10] SMI-S plugin: Reorganize code layout.
* [PATCH 0/7] Move lsm.System related methods to smis_sys.py.
1. Improve performance of volume listing in patch 2/8.
2. Improve MegaRAID pool listing in patch 6/8.
3. Use lsm.Pool.plugin_data to store CIMInstanceName for quicker and
better way for converting lsm.Pool to CIM_StoragePool.
Check patch 7/8 for detail.
Any comments will be appreciated.
smis_pool.py, method pool_id_of_cim_vol, the error message has 3 field
formatters specified, but only 2 values provided.

How much does caching the cim pool patch actually help, performance wise?

Pickle can execute arbitrary code. If some malicious user wanted to
trick the plug-in to execute arbitrary code they could do so by stuffing
code into plugin_data. At the moment I can't think of anything as the
plug-in executes without privilege, but I'm wondering if there would be
a better way to encode this data so we didn't have to have that attack
vector even available.

Thanks!

Regards,
Tony
Gris Ge
2014-09-30 04:01:37 UTC
Permalink
Post by Tony Asleson
Post by Gris Ge
Any comments will be appreciated.
smis_pool.py, method pool_id_of_cim_vol, the error message has 3 field
formatters specified, but only 2 values provided.
I will fix that.
Post by Tony Asleson
How much does caching the cim pool patch actually help, performance wise?
Pickle can execute arbitrary code. If some malicious user wanted to
trick the plug-in to execute arbitrary code they could do so by stuffing
code into plugin_data. At the moment I can't think of anything as the
plug-in executes without privilege, but I'm wondering if there would be
a better way to encode this data so we didn't have to have that attack
vector even available.
Thank you for pointing out the possible misuse of pickle.
I found another way to store CIMInstanceName as string.
V2 patch is coming.

Thank you again, Tony.
Best regards.
Post by Tony Asleson
Thanks!
Regards,
Tony
--
Gris Ge
Gris Ge
2014-09-30 07:13:47 UTC
Permalink
This patchset is based on master plus these patches:

* [V2 0/3] Move capablities to separate file
* [PATCH V3 00/10] SMI-S plugin: Reorganize code layout.
* [PATCH 0/7] Move lsm.System related methods to smis_sys.py.

Highlights:
1. Improve performance of volume listing in patch 2/8.
2. Improve MegaRAID pool listing in patch 6/8.
3. Use lsm.Pool.plugin_data to store CIMInstanceName for quicker and
better way for converting lsm.Pool to CIM_StoragePool.
In volume_create() and other methods, converting lsm.Pool into
cim_pool is require to invoke method via WBEM.
Previously, we are using WBEM EnumerateInstances() call to get all
cim_pools and check their pool id.
With patch 7/8, we use lsm.Pool.plugin_data with no WBEM call.
Check patch 7/8 for detail.

Changes in V2:
* Patch 7/8 is using json instead cPickle for plugin_data as cPickle as
potential security concern.
* Patch 8/8 fixed the error message formatter.

Any comments will be appreciated.

Thank you in advance.

Gris Ge (8):
SMI-S Plugin: Add smis_pool.py file.
SMI-S Plugin: Move cim_pool query into smis_pool
SMI-S Plugin: Move Pool.id generator to smis_pool
SMI-S Plugin: Add smis_pool.cim_pool_pros()
SMI-S Plugin: Remove pool_create leftover methods
SMI-S Plugin: Move cim_pool converter to smis_pool
SMI-S Plugin: New way to convert lsm_pool to cim_pool
SMI-S Plugin: Move pool_id_of_cim_vol() into smis_pool

packaging/libstoragemgmt.spec.in | 1 +
plugin/Makefile.am | 3 +-
plugin/smispy/dmtf.py | 17 ---
plugin/smispy/smis.py | 259 ++++-------------------------------
plugin/smispy/smis_constants.py | 3 +-
plugin/smispy/smis_pool.py | 285 ++++++++++++++++++++++++++++++++++++++
6 files changed, 314 insertions(+), 254 deletions(-)
create mode 100644 plugin/smispy/smis_pool.py
Gris Ge
2014-09-30 07:13:48 UTC
Permalink
* Empty smis_pool.py with license only.
* Makefile and RPM spec file updated.

Signed-off-by: Gris Ge <***@redhat.com>
---
packaging/libstoragemgmt.spec.in | 1 +
plugin/Makefile.am | 3 ++-
plugin/smispy/smis_pool.py | 16 ++++++++++++++++
3 files changed, 19 insertions(+), 1 deletions(-)
create mode 100644 plugin/smispy/smis_pool.py

diff --git a/packaging/libstoragemgmt.spec.in b/packaging/libstoragemgmt.spec.in
index 6eb1d55..5656548 100644
--- a/packaging/libstoragemgmt.spec.in
+++ b/packaging/libstoragemgmt.spec.in
@@ -319,6 +319,7 @@ fi
%{python_sitelib}/lsm/plugin/smispy/smis_common.*
%{python_sitelib}/lsm/plugin/smispy/smis_cap.*
%{python_sitelib}/lsm/plugin/smispy/smis_sys.*
+%{python_sitelib}/lsm/plugin/smispy/smis_pool.*
%{_bindir}/smispy_lsmplugin

%files netapp-plugin
diff --git a/plugin/Makefile.am b/plugin/Makefile.am
index df411f3..793fb0b 100644
--- a/plugin/Makefile.am
+++ b/plugin/Makefile.am
@@ -30,7 +30,8 @@ smispy_PYTHON = \
smispy/smis_common.py \
smispy/dmtf.py \
smispy/smis_cap.py \
- smispy/smis_sys.py
+ smispy/smis_sys.py \
+ smispy/smis_pool.py

nstordir = $(plugindir)/nstor
nstor_PYTHON = \
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
new file mode 100644
index 0000000..e1f46a5
--- /dev/null
+++ b/plugin/smispy/smis_pool.py
@@ -0,0 +1,16 @@
+## 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>
--
1.7.1
Gris Ge
2014-09-30 07:13:49 UTC
Permalink
* Replace Smis._cim_pools_of() with smis_pool.cim_pools_of_cim_sys_path()
* Merge the pool filtering(remove spare pool and other unused pool) into
smis_pool.cim_pools_of_cim_sys_path().
With this, it save some WBEM calls[1] in volumes().
Performance changes for volumes()(lsmcli list --type volumes):
EMC VNX with 95 volumes:
Old: 13.37s
New: 8.97s

[1] Old codes will try to find volumes on spare pool or other unused pool
which is waster of time.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 72 ++++++++------------------------------------
plugin/smispy/smis_pool.py | 65 +++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 59 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index fc231dd..0883bc1 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -31,6 +31,7 @@ from pywbem import CIMError
from smis_constants import *
import smis_cap
import smis_sys
+import smis_pool

from lsm import (IStorageAreaNetwork, uri_parse, LsmError, ErrorNumber,
JobStatus, md5, Volume, AccessGroup,
@@ -813,7 +814,9 @@ class Smis(IStorageAreaNetwork):
for cim_sys in cim_syss:
sys_id = smis_sys.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):
+ cim_pools = smis_pool.cim_pools_of_cim_sys_path(
+ self._c, cim_sys.path, pool_pros)
+ for cim_pool in cim_pools:
pool_id = self._pool_id(cim_pool)
cim_vols = self._c.Associators(
cim_pool.path,
@@ -831,19 +834,6 @@ class Smis(IStorageAreaNetwork):
rc.extend([vol])
return search_property(rc, search_key, search_value)

- def _cim_pools_of(self, cim_sys_path, property_list=None):
- if property_list is None:
- property_list = ['Primordial']
- else:
- property_list = merge_list(property_list, ['Primordial'])
-
- cim_pools = self._c.Associators(cim_sys_path,
- AssocClass='CIM_HostedStoragePool',
- ResultClass='CIM_StoragePool',
- PropertyList=property_list)
-
- return [p for p in cim_pools if not p["Primordial"]]
-
def _new_pool_cim_pool_pros(self):
"""
Return a list of properties for creating new pool.
@@ -857,17 +847,10 @@ class Smis(IStorageAreaNetwork):
@handle_cim_errors
def pools(self, search_key=None, search_value=None, flags=0):
"""
- We are basing on "Block Services Package" profile version 1.4 or
- later:
- CIM_ComputerSystem
- |
- | (CIM_HostedStoragePool)
- |
- v
- CIM_StoragePool
- As 'Block Services Package' is mandatory for 'Array' profile, we
- don't check support status here as startup() already checked 'Array'
- profile.
+ Convert CIM_StoragePool to lsm.Pool.
+ To list all CIM_StoragePool:
+ 1. List all root CIM_ComputerSystem.
+ 2. List all CIM_StoragePool associated to CIM_ComputerSystem.
"""
rc = []
cim_pool_pros = self._new_pool_cim_pool_pros()
@@ -877,40 +860,11 @@ class Smis(IStorageAreaNetwork):

for cim_sys in cim_syss:
system_id = smis_sys.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 \
- cim_pool['Usage'] == DMTF_POOL_USAGE_SPARE:
- continue
- # Skip IBM ArrayPool and ArraySitePool
- # ArrayPool is holding RAID info.
- # ArraySitePool is holding 8 disks. Predefined by array.
- # ArraySite --(1to1 map) --> Array --(1to1 map)--> Rank
-
- # By design when user get a ELEMENT_TYPE_POOL only pool,
- # user can assume he/she can allocate spaces from that pool
- # to create a new pool with ELEMENT_TYPE_VOLUME or
- # ELEMENT_TYPE_FS ability.
-
- # If we expose them out, we will have two kind of pools
- # (ArrayPool and ArraySitePool) having element_type &
- # ELEMENT_TYPE_POOL, but none of them can create a
- # ELEMENT_TYPE_VOLUME pool.
- # Only RankPool can create a ELEMENT_TYPE_VOLUME pool.
-
- # We are trying to hide the detail to provide a simple
- # abstraction.
- if cim_pool.classname == 'IBMTSDS_ArrayPool' or \
- cim_pool.classname == 'IBMTSDS_ArraySitePool':
- continue
+ cim_pools = smis_pool.cim_pools_of_cim_sys_path(
+ self._c, cim_sys.path, cim_pool_pros)
+ for cim_pool in cim_pools:
+ rc.append(self._new_pool(cim_pool, system_id))

- pool = self._new_pool(cim_pool, system_id)
- if pool:
- rc.extend([pool])
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Failed to retrieve pool information " +
- "from CIM_StoragePool: %s" % cim_pool.path)
return search_property(rc, search_key, search_value)

def _sys_id_of_cim_pool(self, cim_pool):
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index e1f46a5..9a62187 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -14,3 +14,68 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
#
# Author: Gris Ge <***@redhat.com>
+
+from utils import merge_list
+from dmtf import DMTF_POOL_USAGE_SPARE
+
+
+def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
+ """
+ Use this association to get a list of CIM_StoragePool:
+ CIM_ComputerSystem
+ |
+ | (CIM_HostedStoragePool)
+ |
+ v
+ CIM_StoragePool
+ As 'Block Services Package' is mandatory for 'Array' profile which already
+ checked by plugin_register(), we don't do any profile check here.
+ Primordial pool will be eliminated from return list.
+ These pools will be eliminated also:
+ * Spare pool with CIM_StoragePool['Usage'] == DMTF_POOL_USAGE_SPARE
+ * IBM ArrayPool(IBMTSDS_ArrayPool)
+ * IBM ArraySitePool(IBMTSDS_ArraySitePool)
+ """
+ cim_pools = []
+
+ if property_list is None:
+ property_list = ['Primordial', 'Usage']
+ else:
+ property_list = merge_list(property_list, ['Primordial', 'Usage'])
+
+ cim_pools = smis_common.Associators(
+ cim_sys_path,
+ AssocClass='CIM_HostedStoragePool',
+ ResultClass='CIM_StoragePool',
+ PropertyList=property_list)
+
+ rc = []
+ for cim_pool in cim_pools:
+ if 'Primordial' in cim_pool and cim_pool['Primordial']:
+ continue
+ if 'Usage' in cim_pool and cim_pool['Usage'] == DMTF_POOL_USAGE_SPARE:
+ continue
+ # Skip IBM ArrayPool and ArraySitePool
+ # ArrayPool is holding RAID info.
+ # ArraySitePool is holding 8 disks. Predefined by array.
+ # ArraySite --(1to1 map) --> Array --(1to1 map)--> Rank
+
+ # By design when user get a ELEMENT_TYPE_POOL only pool,
+ # user can assume he/she can allocate spaces from that pool
+ # to create a new pool with ELEMENT_TYPE_VOLUME or
+ # ELEMENT_TYPE_FS ability.
+
+ # If we expose them out, we will have two kind of pools
+ # (ArrayPool and ArraySitePool) having element_type &
+ # ELEMENT_TYPE_POOL, but none of them can create a
+ # ELEMENT_TYPE_VOLUME pool.
+ # Only RankPool can create a ELEMENT_TYPE_VOLUME pool.
+
+ # We are trying to hide the detail to provide a simple
+ # abstraction.
+ if cim_pool.classname == 'IBMTSDS_ArrayPool' or \
+ cim_pool.classname == 'IBMTSDS_ArraySitePool':
+ continue
+ rc.append(cim_pool)
+
+ return rc
--
1.7.1
Gris Ge
2014-09-30 07:13:50 UTC
Permalink
* Replace Smis._pool_id() with smis_pool.pool_id_of_cim_pool()
* Replace Smis._property_list_of_id(self, 'Pool') with
smis_pool.cim_pool_id_pros()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 0883bc1..abd323c 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -396,8 +396,6 @@ class Smis(IStorageAreaNetwork):
rc = []
if class_type == 'Volume':
rc = ['SystemName', 'DeviceID']
- elif class_type == 'Pool':
- rc = ['InstanceID']
elif class_type == 'SystemChild':
rc = ['SystemName']
elif class_type == 'Disk':
@@ -422,12 +420,6 @@ class Smis(IStorageAreaNetwork):
"""
return self._id('SystemChild', cim_xxx)

- def _pool_id(self, cim_pool):
- """
- Return CIM_StoragePool['InstanceID']
- """
- return self._id('Pool', cim_pool)
-
def _vol_id(self, cim_vol):
"""
Return the MD5 hash of CIM_StorageVolume['SystemName'] and
@@ -512,7 +504,7 @@ class Smis(IStorageAreaNetwork):
AssocClass='CIM_AllocatedFromStoragePool',
ResultClass='CIM_StoragePool',
PropertyList=property_list)[0]
- return self._pool_id(cim_pool)
+ return smis_pool.pool_id_of_cim_pool(cim_pool)

@staticmethod
def _get_vol_other_id_info(cv):
@@ -813,11 +805,11 @@ class Smis(IStorageAreaNetwork):
cim_vol_pros = self._cim_vol_pros()
for cim_sys in cim_syss:
sys_id = smis_sys.sys_id_of_cim_sys(cim_sys)
- pool_pros = self._property_list_of_id('Pool')
+ pool_pros = smis_pool.cim_pool_id_pros()
cim_pools = smis_pool.cim_pools_of_cim_sys_path(
self._c, cim_sys.path, pool_pros)
for cim_pool in cim_pools:
- pool_id = self._pool_id(cim_pool)
+ pool_id = smis_pool.pool_id_of_cim_pool(cim_pool)
cim_vols = self._c.Associators(
cim_pool.path,
AssocClass='CIM_AllocatedFromStoragePool',
@@ -838,7 +830,7 @@ class Smis(IStorageAreaNetwork):
"""
Return a list of properties for creating new pool.
"""
- pool_pros = self._property_list_of_id('Pool')
+ pool_pros = smis_pool.cim_pool_id_pros()
pool_pros.extend(['ElementName', 'TotalManagedSpace',
'RemainingManagedSpace', 'Usage',
'OperationalStatus'])
@@ -890,7 +882,7 @@ class Smis(IStorageAreaNetwork):
system_id = self._sys_id_of_cim_pool(cim_pool)

status_info = ''
- pool_id = self._pool_id(cim_pool)
+ pool_id = smis_pool.pool_id_of_cim_pool(cim_pool)
name = ''
total_space = Pool.TOTAL_SPACE_NOT_FOUND
free_space = Pool.FREE_SPACE_NOT_FOUND
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index 9a62187..b0767d8 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -17,6 +17,7 @@

from utils import merge_list
from dmtf import DMTF_POOL_USAGE_SPARE
+from lsm import LsmError, ErrorNumber


def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
@@ -79,3 +80,21 @@ def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
rc.append(cim_pool)

return rc
+
+
+def cim_pool_id_pros():
+ """
+ Return a list of CIM_StoragePool properties required to generate
+ lsm.Pool.id
+ """
+ return ['InstanceID']
+
+
+def pool_id_of_cim_pool(cim_pool):
+ if 'InstanceID' in cim_pool:
+ return cim_pool['InstanceID']
+ else:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "pool_id_of_cim_pool(): Got CIM_StoragePool with no 'InstanceID' "
+ "property: %s, %s" % (cim_pool.items(), cim_pool.path))
--
1.7.1
Gris Ge
2014-09-30 07:13:51 UTC
Permalink
* Replaced Smis._new_pool_cim_pool_pros(self) with
smis_pool.cim_pool_pros()

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index abd323c..1def4ce 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -498,7 +498,7 @@ class Smis(IStorageAreaNetwork):
Takes a CIMInstance that represents a volume and returns the pool
id for that volume.
"""
- property_list = Smis._property_list_of_id('Pool')
+ property_list = smis_pool.cim_pool_id_pros()
cim_pool = self._c.Associators(
cim_vol.path,
AssocClass='CIM_AllocatedFromStoragePool',
@@ -692,7 +692,7 @@ class Smis(IStorageAreaNetwork):
For SYNC CreateOrModifyElementFromStoragePool action.
The new CIM_StoragePool is stored in out['Pool']
"""
- pool_pros = self._new_pool_cim_pool_pros()
+ pool_pros = smis_pool.cim_pool_pros()

if 'Pool' in out:
cim_new_pool = self._c.GetInstance(
@@ -771,7 +771,7 @@ class Smis(IStorageAreaNetwork):
"""
Given a CIMInstance of CIM_ConcreteJob, return a LSM Pool
"""
- pool_pros = self._new_pool_cim_pool_pros()
+ pool_pros = smis_pool.cim_pool_pros()
cim_pools = self._c.Associators(cim_job.path,
AssocClass='CIM_AffectedJobElement',
ResultClass='CIM_StoragePool',
@@ -826,16 +826,6 @@ class Smis(IStorageAreaNetwork):
rc.extend([vol])
return search_property(rc, search_key, search_value)

- def _new_pool_cim_pool_pros(self):
- """
- Return a list of properties for creating new pool.
- """
- pool_pros = smis_pool.cim_pool_id_pros()
- pool_pros.extend(['ElementName', 'TotalManagedSpace',
- 'RemainingManagedSpace', 'Usage',
- 'OperationalStatus'])
- return pool_pros
-
@handle_cim_errors
def pools(self, search_key=None, search_value=None, flags=0):
"""
@@ -845,7 +835,7 @@ class Smis(IStorageAreaNetwork):
2. List all CIM_StoragePool associated to CIM_ComputerSystem.
"""
rc = []
- cim_pool_pros = self._new_pool_cim_pool_pros()
+ cim_pool_pros = smis_pool.cim_pool_pros()

cim_sys_pros = smis_sys.cim_sys_id_pros()
cim_syss = smis_sys.root_cim_sys(self._c, cim_sys_pros)
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index b0767d8..2fec3bc 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -98,3 +98,14 @@ def pool_id_of_cim_pool(cim_pool):
ErrorNumber.PLUGIN_BUG,
"pool_id_of_cim_pool(): Got CIM_StoragePool with no 'InstanceID' "
"property: %s, %s" % (cim_pool.items(), cim_pool.path))
+
+
+def cim_pool_pros():
+ """
+ Return a list of CIM_StoragePool properties required to generate lsm.Pool.
+ """
+ pool_pros = cim_pool_id_pros()
+ pool_pros.extend(['ElementName', 'TotalManagedSpace',
+ 'RemainingManagedSpace', 'Usage',
+ 'OperationalStatus'])
+ return pool_pros
--
1.7.1
Gris Ge
2014-09-30 07:13:52 UTC
Permalink
* Remove smis_constants.JOB_RETRIEVE_POOL
* Remove Smis._new_pool_from_name() and Smis._new_pool_from_job()

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/smis.py | 32 --------------------------------
plugin/smispy/smis_constants.py | 3 +--
2 files changed, 1 insertions(+), 34 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 1def4ce..9ba51d2 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -182,8 +182,6 @@ class Smis(IStorageAreaNetwork):
if rc == INVOKE_OK:
if retrieve_data == JOB_RETRIEVE_VOLUME:
return None, self._new_vol_from_name(out)
- elif retrieve_data == JOB_RETRIEVE_POOL:
- return None, self._new_pool_from_name(out)
else:
return None, None

@@ -343,8 +341,6 @@ class Smis(IStorageAreaNetwork):
(ignore, retrieve_data) = self._parse_job_id(job_id)
if retrieve_data == JOB_RETRIEVE_VOLUME:
completed_item = self._new_vol_from_job(cim_job)
- elif retrieve_data == JOB_RETRIEVE_POOL:
- completed_item = self._new_pool_from_job(cim_job)
else:
status = JobStatus.ERROR

@@ -687,23 +683,6 @@ class Smis(IStorageAreaNetwork):

return self._new_vol(instance)

- def _new_pool_from_name(self, out):
- """
- For SYNC CreateOrModifyElementFromStoragePool action.
- The new CIM_StoragePool is stored in out['Pool']
- """
- pool_pros = smis_pool.cim_pool_pros()
-
- if 'Pool' in out:
- cim_new_pool = self._c.GetInstance(
- out['Pool'],
- PropertyList=pool_pros, LocalOnly=False)
- return self._new_pool(cim_new_pool)
- else:
- raise LsmError(ErrorNumber.PLUGIN_BUG,
- "Got not new Pool from out of InvokeMethod" +
- "when CreateOrModifyElementFromStoragePool")
-
def _cim_spc_pros(self):
"""
Return a list of properties required to build new AccessGroup.
@@ -767,17 +746,6 @@ class Smis(IStorageAreaNetwork):
return self._new_vol(self._c.GetInstance(a.path, LocalOnly=False))
return None

- def _new_pool_from_job(self, cim_job):
- """
- Given a CIMInstance of CIM_ConcreteJob, return a LSM Pool
- """
- pool_pros = smis_pool.cim_pool_pros()
- cim_pools = self._c.Associators(cim_job.path,
- AssocClass='CIM_AffectedJobElement',
- ResultClass='CIM_StoragePool',
- PropertyList=pool_pros)
- return self._new_pool(cim_pools[0])
-
@handle_cim_errors
def volumes(self, search_key=None, search_value=None, flags=0):
"""
diff --git a/plugin/smispy/smis_constants.py b/plugin/smispy/smis_constants.py
index 0536cf5..2df9144 100644
--- a/plugin/smispy/smis_constants.py
+++ b/plugin/smispy/smis_constants.py
@@ -61,7 +61,6 @@ VOL_NAME_SPACE_SNVM = 7

JOB_RETRIEVE_NONE = 0
JOB_RETRIEVE_VOLUME = 1
-JOB_RETRIEVE_POOL = 2

IAAN_WBEM_HTTP_PORT = 5988
IAAN_WBEM_HTTPS_PORT = 5989
@@ -129,4 +128,4 @@ class Synchronized(object):
VOL_OP_STATUS_DORMANT) = (2, 3, 6, 8, 15)

# SMI-S ExposePaths device access enumerations
-(EXPOSE_PATHS_DA_READ_WRITE, EXPOSE_PATHS_DA_READ_ONLY) = (2, 3)
\ No newline at end of file
+(EXPOSE_PATHS_DA_READ_WRITE, EXPOSE_PATHS_DA_READ_ONLY) = (2, 3)
--
1.7.1
Gris Ge
2014-09-30 07:13:53 UTC
Permalink
* Move DMTF.cim_pool_status_of() to
smis_pool._pool_status_of_cim_pool().
# This method is for smis_pool internal use only now.

* Move Smis._pool_element_type() to
smis_pool._pool_element_type()
# This method is for smis_pool internal use only now.

* Removed Smis._sys_id_of_cim_pool() as no method require it.

* Add MegaRAID workaround for smis_pool._pool_element_type()
to improve performance by returning (Pool.ELEMENT_TYPE_VOLUME, 0)
directly. Tested on a remote MegaRAID card with two pools:
Old: 10.10s # With all pending patches but this one.
New: 4.61s

* Move Smis._new_pool() to smis_pool.cim_pool_to_lsm_pool()

* Including three changes in patch as _pool_status_of_cim_pool() and
_pool_element_type() are internal methods for
smis_pool.cim_pool_to_lsm_pool() now.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/dmtf.py | 17 ------
plugin/smispy/smis.py | 105 +--------------------------------------
plugin/smispy/smis_pool.py | 117 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 118 insertions(+), 121 deletions(-)

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index d020a9a..c5ea3fe 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -96,23 +96,6 @@ class DMTF(object):
return status, " ".join(status_info_list)


- _LSM_POOL_OP_STATUS_CONV = {
- OP_STATUS_OK: Pool.STATUS_OK,
- OP_STATUS_ERROR: Pool.STATUS_ERROR,
- OP_STATUS_DEGRADED: Pool.STATUS_DEGRADED,
- OP_STATUS_NON_RECOVERABLE_ERROR: Pool.STATUS_ERROR,
- OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: Pool.STATUS_ERROR,
- }
-
- @staticmethod
- def cim_pool_status_of(dmtf_op_status_list):
- """
- Convert CIM_StoragePool['OperationalStatus'] to LSM
- """
- return DMTF.dmtf_op_status_list_conv(
- DMTF._LSM_POOL_OP_STATUS_CONV, dmtf_op_status_list,
- Pool.STATUS_UNKNOWN, Pool.STATUS_OTHER)
-
EMC_DISK_STATUS_REMOVED = 32768

_LSM_DISK_OP_STATUS_CONV = {
diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 9ba51d2..46d1dbb 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -813,54 +813,12 @@ class Smis(IStorageAreaNetwork):
cim_pools = smis_pool.cim_pools_of_cim_sys_path(
self._c, cim_sys.path, cim_pool_pros)
for cim_pool in cim_pools:
- rc.append(self._new_pool(cim_pool, system_id))
+ rc.append(
+ smis_pool.cim_pool_to_lsm_pool(
+ self._c, cim_pool, system_id))

return search_property(rc, search_key, search_value)

- def _sys_id_of_cim_pool(self, cim_pool):
- """
- Find out the system ID for certain CIM_StoragePool.
- Will return '' if failed.
- """
- sys_pros = smis_sys.cim_sys_id_pros()
- cim_syss = self._c.Associators(cim_pool.path,
- ResultClass='CIM_ComputerSystem',
- PropertyList=sys_pros)
- if len(cim_syss) == 1:
- return smis_sys.sys_id_of_cim_sys(cim_syss[0])
- return ''
-
- @handle_cim_errors
- def _new_pool(self, cim_pool, system_id=''):
- """
- Return a Pool object base on information of cim_pool.
- Assuming cim_pool already holding correct properties.
- """
- if not system_id:
- system_id = self._sys_id_of_cim_pool(cim_pool)
-
- status_info = ''
- pool_id = smis_pool.pool_id_of_cim_pool(cim_pool)
- name = ''
- total_space = Pool.TOTAL_SPACE_NOT_FOUND
- free_space = Pool.FREE_SPACE_NOT_FOUND
- status = Pool.STATUS_OK
- if 'ElementName' in cim_pool:
- name = cim_pool['ElementName']
- if 'TotalManagedSpace' in cim_pool:
- total_space = cim_pool['TotalManagedSpace']
- if 'RemainingManagedSpace' in cim_pool:
- free_space = cim_pool['RemainingManagedSpace']
- if 'OperationalStatus' in cim_pool:
- (status, status_info) = DMTF.cim_pool_status_of(
- cim_pool['OperationalStatus'])
-
- element_type, unsupported = self._pool_element_type(cim_pool)
-
- return Pool(pool_id, name, element_type, unsupported,
- total_space, free_space,
- status, status_info, system_id)
-
@handle_cim_errors
def systems(self, flags=0):
"""
@@ -2359,63 +2317,6 @@ class Smis(IStorageAreaNetwork):
"requested CIM_StorageExtent %s" %
cim_pri_ext_path)

- def _pool_element_type(self, cim_pool):
-
- element_type = 0
- unsupported = 0
-
- # check whether current pool support create volume or not.
- cim_sccs = self._c.Associators(
- cim_pool.path,
- AssocClass='CIM_ElementCapabilities',
- ResultClass='CIM_StorageConfigurationCapabilities',
- PropertyList=['SupportedStorageElementFeatures',
- 'SupportedStorageElementTypes'])
- # Associate StorageConfigurationCapabilities to StoragePool
- # is experimental in SNIA 1.6rev4, Block Book PDF Page 68.
- # Section 5.1.6 StoragePool, StorageVolume and LogicalDisk
- # Manipulation, Figure 9 - Capabilities Specific to a StoragePool
- if len(cim_sccs) == 1:
- cim_scc = cim_sccs[0]
- if 'SupportedStorageElementFeatures' in cim_scc:
- supported_features = cim_scc['SupportedStorageElementFeatures']
-
- if DMTF_SUPPORT_VOL_CREATE in supported_features:
- element_type |= Pool.ELEMENT_TYPE_VOLUME
- if DMTF_SUPPORT_ELEMENT_EXPAND not in supported_features:
- unsupported |= Pool.UNSUPPORTED_VOLUME_GROW
- if DMTF_SUPPORT_ELEMENT_REDUCE not in supported_features:
- unsupported |= Pool.UNSUPPORTED_VOLUME_SHRINK
-
- else:
- # IBM DS 8000 does not support StorageConfigurationCapabilities
- # per pool yet. They has been informed. Before fix, use a quick
- # workaround.
- # TODO: Currently, we don't have a way to detect
- # Pool.ELEMENT_TYPE_POOL
- # but based on knowing definition of each vendor.
- if cim_pool.classname == 'IBMTSDS_VirtualPool' or \
- cim_pool.classname == 'IBMTSDS_ExtentPool':
- element_type = Pool.ELEMENT_TYPE_VOLUME
- elif cim_pool.classname == 'IBMTSDS_RankPool':
- element_type = Pool.ELEMENT_TYPE_POOL
- elif cim_pool.classname == 'LSIESG_StoragePool':
- element_type = Pool.ELEMENT_TYPE_VOLUME
-
- if 'Usage' in cim_pool:
- usage = cim_pool['Usage']
-
- if usage == DMTF_POOL_USAGE_UNRESTRICTED:
- element_type |= Pool.ELEMENT_TYPE_VOLUME
- if usage == DMTF_POOL_USAGE_RESERVED_FOR_SYSTEM or \
- usage > DMTF_POOL_USAGE_DELTA:
- element_type |= Pool.ELEMENT_TYPE_SYS_RESERVED
- if usage == DMTF_POOL_USAGE_DELTA:
- # We blitz all the other elements types for this designation
- element_type = Pool.ELEMENT_TYPE_DELTA
-
- return element_type, unsupported
-
@staticmethod
def _is_frontend_fc_tgt(cim_fc_tgt):
"""
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index 2fec3bc..a64c628 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -16,8 +16,8 @@
# Author: Gris Ge <***@redhat.com>

from utils import merge_list
-from dmtf import DMTF_POOL_USAGE_SPARE
-from lsm import LsmError, ErrorNumber
+from dmtf import *
+from lsm import LsmError, ErrorNumber, Pool


def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
@@ -109,3 +109,116 @@ def cim_pool_pros():
'RemainingManagedSpace', 'Usage',
'OperationalStatus'])
return pool_pros
+
+
+def _pool_element_type(smis_common, cim_pool):
+ """
+ Return a set (Pool.element_type, Pool.unsupported)
+ Using CIM_StorageConfigurationCapabilities
+ 'SupportedStorageElementFeatures' and 'SupportedStorageElementTypes'
+ property.
+ For MegaRAID, just return (Pool.ELEMENT_TYPE_VOLUME, 0)
+ """
+ if smis_common.is_megaraid():
+ return Pool.ELEMENT_TYPE_VOLUME, 0
+
+ element_type = 0
+ unsupported = 0
+
+ # check whether current pool support create volume or not.
+ cim_sccs = smis_common.Associators(
+ cim_pool.path,
+ AssocClass='CIM_ElementCapabilities',
+ ResultClass='CIM_StorageConfigurationCapabilities',
+ PropertyList=['SupportedStorageElementFeatures',
+ 'SupportedStorageElementTypes'])
+ # Associate StorageConfigurationCapabilities to StoragePool
+ # is experimental in SNIA 1.6rev4, Block Book PDF Page 68.
+ # Section 5.1.6 StoragePool, StorageVolume and LogicalDisk
+ # Manipulation, Figure 9 - Capabilities Specific to a StoragePool
+ if len(cim_sccs) == 1:
+ cim_scc = cim_sccs[0]
+ if 'SupportedStorageElementFeatures' in cim_scc:
+ supported_features = cim_scc['SupportedStorageElementFeatures']
+
+ if DMTF_SUPPORT_VOL_CREATE in supported_features:
+ element_type |= Pool.ELEMENT_TYPE_VOLUME
+ if DMTF_SUPPORT_ELEMENT_EXPAND not in supported_features:
+ unsupported |= Pool.UNSUPPORTED_VOLUME_GROW
+ if DMTF_SUPPORT_ELEMENT_REDUCE not in supported_features:
+ unsupported |= Pool.UNSUPPORTED_VOLUME_SHRINK
+
+ else:
+ # IBM DS 8000 does not support StorageConfigurationCapabilities
+ # per pool yet. They has been informed. Before fix, use a quick
+ # workaround.
+ # TODO: Currently, we don't have a way to detect
+ # Pool.ELEMENT_TYPE_POOL
+ # but based on knowing definition of each vendor.
+ if cim_pool.classname == 'IBMTSDS_VirtualPool' or \
+ cim_pool.classname == 'IBMTSDS_ExtentPool':
+ element_type = Pool.ELEMENT_TYPE_VOLUME
+ elif cim_pool.classname == 'IBMTSDS_RankPool':
+ element_type = Pool.ELEMENT_TYPE_POOL
+ elif cim_pool.classname == 'LSIESG_StoragePool':
+ element_type = Pool.ELEMENT_TYPE_VOLUME
+
+ if 'Usage' in cim_pool:
+ usage = cim_pool['Usage']
+
+ if usage == DMTF_POOL_USAGE_UNRESTRICTED:
+ element_type |= Pool.ELEMENT_TYPE_VOLUME
+ if usage == DMTF_POOL_USAGE_RESERVED_FOR_SYSTEM or \
+ usage > DMTF_POOL_USAGE_DELTA:
+ element_type |= Pool.ELEMENT_TYPE_SYS_RESERVED
+ if usage == DMTF_POOL_USAGE_DELTA:
+ # We blitz all the other elements types for this designation
+ element_type = Pool.ELEMENT_TYPE_DELTA
+
+ return element_type, unsupported
+
+
+_LSM_POOL_OP_STATUS_CONV = {
+ DMTF.OP_STATUS_OK: Pool.STATUS_OK,
+ DMTF.OP_STATUS_ERROR: Pool.STATUS_ERROR,
+ DMTF.OP_STATUS_DEGRADED: Pool.STATUS_DEGRADED,
+ DMTF.OP_STATUS_NON_RECOVERABLE_ERROR: Pool.STATUS_ERROR,
+ DMTF.OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: Pool.STATUS_ERROR,
+}
+
+
+def _pool_status_of_cim_pool(dmtf_op_status_list):
+ """
+ Convert CIM_StoragePool['OperationalStatus'] to LSM
+ """
+ return DMTF.dmtf_op_status_list_conv(
+ _LSM_POOL_OP_STATUS_CONV, dmtf_op_status_list,
+ Pool.STATUS_UNKNOWN, Pool.STATUS_OTHER)
+
+
+def cim_pool_to_lsm_pool(smis_common, cim_pool, system_id):
+ """
+ Return a Pool object base on information of cim_pool.
+ Assuming cim_pool already holding correct properties.
+ """
+ status_info = ''
+ pool_id = pool_id_of_cim_pool(cim_pool)
+ name = ''
+ total_space = Pool.TOTAL_SPACE_NOT_FOUND
+ free_space = Pool.FREE_SPACE_NOT_FOUND
+ status = Pool.STATUS_OK
+ if 'ElementName' in cim_pool:
+ name = cim_pool['ElementName']
+ if 'TotalManagedSpace' in cim_pool:
+ total_space = cim_pool['TotalManagedSpace']
+ if 'RemainingManagedSpace' in cim_pool:
+ free_space = cim_pool['RemainingManagedSpace']
+ if 'OperationalStatus' in cim_pool:
+ (status, status_info) = _pool_status_of_cim_pool(
+ cim_pool['OperationalStatus'])
+
+ element_type, unsupported = _pool_element_type(smis_common, cim_pool)
+
+ return Pool(pool_id, name, element_type, unsupported,
+ total_space, free_space,
+ status, status_info, system_id)
--
1.7.1
Gris Ge
2014-09-30 07:13:54 UTC
Permalink
* Using json to save the CIMInstanceName of CIM_StoragePool into
lsm.Pool.plugin_data and convert it back when needed:
* Save:
Pool.plugin_data = json.dumps({
'classname': cim_path.classname,
'keybindings': dict(cim_path.keybindings),
'host': cim_path.host,
'namespace': cim_path.namespace,
})

* Load:
CIMInstanceName(**json.loads(Pool.plugin_data))

* Replace Smis._get_cim_instance_by_id(self, 'Pool', pool.id) with
smis_pool.lsm_pool_to_cim_pool_path(smis_common, pool)

Changes in V2:
* Use json instead of cPickle for security concern[1].

[1] __reduce__() will be called by cPickle.loads() which might be
use to excute any command remoted.

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 46d1dbb..a437f52 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -878,11 +878,12 @@ class Smis(IStorageAreaNetwork):
# Get the Configuration service for the system we are interested in.
scs = self._c.get_class_instance('CIM_StorageConfigurationService',
'SystemName', pool.system_id)
- sp = self._get_cim_instance_by_id('Pool', pool.id)
+ cim_pool_path = smis_pool.lsm_pool_to_cim_pool_path(
+ self._c, pool)

in_params = {'ElementName': volume_name,
'ElementType': pywbem.Uint16(2),
- 'InPool': sp.path,
+ 'InPool': cim_pool_path,
'Size': pywbem.Uint64(size_bytes)}

try:
@@ -1155,11 +1156,9 @@ class Smis(IStorageAreaNetwork):
rs = self._c.get_class_instance("CIM_ReplicationService", 'SystemName',
volume_src.system_id,
raise_error=False)
-
+ cim_pool_path = None
if pool is not None:
- cim_pool = self._get_cim_instance_by_id('Pool', pool.id)
- else:
- cim_pool = None
+ cim_pool_path = smis_pool.lsm_pool_to_cim_pool_path(self._c, pool)

lun = self._get_cim_instance_by_id('Volume', volume_src.id)

@@ -1201,8 +1200,8 @@ class Smis(IStorageAreaNetwork):
'SourceElement': lun.path}
if rs:

- if cim_pool is not None:
- in_params['TargetPool'] = cim_pool.path
+ if cim_pool_path is not None:
+ in_params['TargetPool'] = cim_pool_path

try:

diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index a64c628..bf3235b 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -18,6 +18,8 @@
from utils import merge_list
from dmtf import *
from lsm import LsmError, ErrorNumber, Pool
+import json
+from pywbem import CIMInstanceName


def cim_pools_of_cim_sys_path(smis_common, cim_sys_path, property_list=None):
@@ -219,6 +221,46 @@ def cim_pool_to_lsm_pool(smis_common, cim_pool, system_id):

element_type, unsupported = _pool_element_type(smis_common, cim_pool)

+ plugin_data = _cim_path_to_path_str(cim_pool.path)
+
return Pool(pool_id, name, element_type, unsupported,
total_space, free_space,
- status, status_info, system_id)
+ status, status_info, system_id, plugin_data)
+
+
+def lsm_pool_to_cim_pool_path(smis_common, lsm_pool):
+ """
+ Convert lsm.Pool to CIMInstanceName of CIM_StoragePool using
+ lsm.Pool.plugin_data
+ """
+ if not lsm_pool.plugin_data:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "Got lsm.Pool instance with empty plugin_data")
+ if smis_common.system_list and \
+ lsm_pool.system_id not in smis_common.system_list:
+ raise LsmError(
+ ErrorNumber.NOT_FOUND_SYSTEM,
+ "System filtered in URI")
+
+ return _path_str_to_cim_path(lsm_pool.plugin_data)
+
+
+def _cim_path_to_path_str(cim_path):
+ """
+ Convert CIMInstanceName to a string which could save in plugin_data
+ """
+ return json.dumps({
+ 'classname': cim_path.classname,
+ 'keybindings': dict(cim_path.keybindings),
+ 'host': cim_path.host,
+ 'namespace': cim_path.namespace,
+ })
+
+
+def _path_str_to_cim_path(path_str):
+ """
+ Convert a string into CIMInstanceName.
+ """
+ path_dict = json.loads(path_str)
+ return CIMInstanceName(**path_dict)
--
1.7.1
Gris Ge
2014-09-30 07:13:55 UTC
Permalink
* Replace Smis._get_pool_from_vol() with smis_pool.pool_id_of_cim_vol()

Changes in V2:
* Fix the error message formatter.

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index a437f52..56b2912 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -489,19 +489,6 @@ class Smis(IStorageAreaNetwork):
retrieve_data = int(tmp_list[1])
return (md5_str, retrieve_data)

- def _get_pool_from_vol(self, cim_vol):
- """
- Takes a CIMInstance that represents a volume and returns the pool
- id for that volume.
- """
- property_list = smis_pool.cim_pool_id_pros()
- cim_pool = self._c.Associators(
- cim_vol.path,
- AssocClass='CIM_AllocatedFromStoragePool',
- ResultClass='CIM_StoragePool',
- PropertyList=property_list)[0]
- return smis_pool.pool_id_of_cim_pool(cim_pool)
-
@staticmethod
def _get_vol_other_id_info(cv):
other_id = None
@@ -559,7 +546,7 @@ class Smis(IStorageAreaNetwork):
#to not call this very often.
if pool_id is None:
#Go an retrieve the pool id
- pool_id = self._get_pool_from_vol(cv)
+ pool_id = smis_pool.pool_id_of_cim_vol(self._c, cv.path)

if sys_id is None:
sys_id = cv['SystemName']
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index bf3235b..e97b631 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -264,3 +264,22 @@ def _path_str_to_cim_path(path_str):
"""
path_dict = json.loads(path_str)
return CIMInstanceName(**path_dict)
+
+
+def pool_id_of_cim_vol(smis_common, cim_vol_path):
+ """
+ Find out the lsm.Pool.id of CIM_StorageVolume
+ """
+ property_list = cim_pool_id_pros()
+ cim_pools = smis_common.Associators(
+ cim_vol_path,
+ AssocClass='CIM_AllocatedFromStoragePool',
+ ResultClass='CIM_StoragePool',
+ PropertyList=property_list)
+ if len(cim_pools) != 1:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "pool_id_of_cim_vol(): Got unexpected count(%d) of cim_pool " %
+ len(cim_pools) +
+ "assocated to cim_vol: %s, %s" % (cim_vol_path, cim_pools))
+ return pool_id_of_cim_pool(cim_pools[0])
--
1.7.1
Loading...