[Libstoragemgmt-devel] [PATCH 3/3] _data.py: Make _check_opt_data class method

[Tony Asleson]

The method was already very close to being a method,
so moved it and removed the allowed parameter to
reference the class as it already was.

Signed-off-by: Tony Asleson <***@redhat.com>
---
python_binding/lsm/_data.py | 59 ++++++++++++++++++++-------------------------
1 file changed, 26 insertions(+), 33 deletions(-)

diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index b9fef89..b0ade68 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -25,7 +25,7 @@ except ImportError:

from json.decoder import WHITESPACE
from lsm import LsmError, ErrorNumber
-from _common import get_class, sh, default_property
+from _common import get_class, default_property


def txt_a(txt, append):
@@ -42,22 +42,6 @@ def get_key(dictionary, value):
return None


-def _check_opt_data(optional_data, allowed_properties):
- if optional_data is None:
- return OptionalData()
- else:
- #Make sure the properties only contain ones we permit
- allowed = set(allowed_properties)
- actual = set(optional_data.list())
-
- if actual <= allowed:
- return optional_data
- else:
- raise LsmError(ErrorNumber.INVALID_ARGUMENT,
- "Property keys are not supported: %s" %
- "".join(actual - allowed))
-
-
class DataEncoder(json.JSONEncoder):
"""
Custom json encoder for objects derived form ILsmData
@@ -175,6 +159,21 @@ class IData(object):
"""
return str(self._to_dict())

+ def _check_opt_data(self, optional_data):
+ if optional_data is None:
+ return OptionalData()
+ else:
+ #Make sure the properties only contain ones we permit
+ allowed = set(self.OPT_PROPERTIES)
+ actual = set(optional_data.list())
+
+ if actual <= allowed:
+ return optional_data
+ else:
+ raise LsmError(ErrorNumber.INVALID_ARGUMENT,
+ "Property keys are not supported: %s" %
+ "".join(actual - allowed))
+

@default_property('id', doc="Unique identifier")
@default_property('type', doc="Enumerated initiator type")
@@ -261,8 +260,7 @@ class Disk(IData):
self._num_of_blocks = _num_of_blocks
self._status = _status
self._system_id = _system_id
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)
self._plugin_data = _plugin_data

@property
@@ -328,8 +326,7 @@ class Volume(IData):
self._status = _status # Status
self._system_id = _system_id # System id this volume belongs
self._pool_id = _pool_id # Pool id this volume belongs
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)
self._plugin_data = _plugin_data

@property
@@ -436,8 +433,7 @@ The lsm.System class does not have class methods.
self._name = _name
self._status = _status
self._status_info = _status_info
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)
self._plugin_data = _plugin_data


@@ -653,8 +649,8 @@ class Pool(IData):
self._status_info = _status_info # Additional status text of pool
self._system_id = _system_id # System id this pool belongs
self._plugin_data = _plugin_data # Plugin private data
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)
+

@default_property('id', doc="Unique identifier")
@default_property('name', doc="File system name")
@@ -676,8 +672,7 @@ class FileSystem(IData):
self._free_space = _free_space
self._pool_id = _pool_id
self._system_id = _system_id
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)
self._plugin_data = _plugin_data


@@ -688,13 +683,13 @@ class FileSystem(IData):
@default_property("plugin_data", "Private plugin data")
class FsSnapshot(IData):
FLAG_RETRIEVE_FULL_INFO = 1 << 0
+
def __init__(self, _id, _name, _ts, _optional_data=None,
_plugin_data=None):
self._id = _id
self._name = _name
self._ts = int(_ts)
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)
self._plugin_data = _plugin_data


@@ -732,8 +727,7 @@ class NfsExport(IData):
self._anonuid = _anonuid # uid for anonymous user id
self._anongid = _anongid # gid for anonymous group id
self._options = _options # NFS options
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)
self._plugin_data = _plugin_data


@@ -775,8 +769,7 @@ class AccessGroup(IData):
self._init_type = _init_type
self._system_id = _system_id # System id this group belongs
self._plugin_data = _plugin_data
- self._optional_data = _check_opt_data(_optional_data,
- self.OPT_PROPERTIES)
+ self._optional_data = self._check_opt_data(_optional_data)


class OptionalData(IData):

[Tony Asleson]

A number of times this method is called with a number of identical
line of code following it which consolidate properties with those
that are used for ID. I moved this logic into the method and
added a default parameter to maintain same functionality.

Signed-off-by: Tony Asleson <***@redhat.com>
---
plugin/smispy/smis.py | 101 +++++++++++++++++++++-----------------------------
1 file changed, 43 insertions(+), 58 deletions(-)

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index 42e2749..cd733bb 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -442,11 +442,8 @@ class Smis(IStorageAreaNetwork):
If you want to save some query time, try set it as False
"""
class_name = Smis._cim_class_name_of(class_type)
- id_pros = Smis._property_list_of_id(class_type)
- if property_list:
- property_list.extend(id_pros)
- else:
- property_list = id_pros
+ property_list = Smis._property_list_of_id(class_type, property_list)
+
cim_xxxs = self._c.EnumerateInstances(class_name,
PropertyList=property_list,
LocalOnly=False)
@@ -884,9 +881,9 @@ class Smis(IStorageAreaNetwork):
"""
completed_item = None

- cim_job_pros = self._property_list_of_id('Job')
- cim_job_pros.extend(['JobState', 'PercentComplete',
- 'ErrorDescription', 'OperationalStatus'])
+ props = ['JobState', 'PercentComplete', 'ErrorDescription',
+ 'OperationalStatus']
+ cim_job_pros = self._property_list_of_id('Job', props)

cim_job = self._get_cim_instance_by_id('Job', job_id, False,
cim_job_pros)
@@ -942,30 +939,38 @@ class Smis(IStorageAreaNetwork):
"class_type %s" % class_type)

@staticmethod
- def _property_list_of_id(class_type):
+ def _property_list_of_id(class_type, requested_properties=None):
"""
Return a PropertyList which the ID of current class is basing on
"""
+ rc = []
if class_type == 'Volume':
- return ['SystemName', 'DeviceID']
- if class_type == 'System':
- return ['Name']
- if class_type == 'Pool':
- return ['InstanceID']
- if class_type == 'SystemChild':
- return ['SystemName']
- if class_type == 'Disk':
- return ['SystemName', 'DeviceID']
- if class_type == 'Job':
- return ['InstanceID']
- if class_type == 'AccessGroup':
- return ['DeviceID']
- if class_type == 'Initiator':
- return ['StorageID']
- raise LsmError(ErrorNumber.INTERNAL_ERROR,
+ rc = ['SystemName', 'DeviceID']
+ elif class_type == 'System':
+ rc = ['Name']
+ elif class_type == 'Pool':
+ rc = ['InstanceID']
+ elif class_type == 'SystemChild':
+ rc = ['SystemName']
+ elif class_type == 'Disk':
+ rc = ['SystemName', 'DeviceID']
+ elif class_type == 'Job':
+ rc = ['InstanceID']
+ elif class_type == 'AccessGroup':
+ rc = ['DeviceID']
+ elif class_type == 'Initiator':
+ rc = ['StorageID']
+ else:
+ raise LsmError(ErrorNumber.INTERNAL_ERROR,
"Smis._cim_class_name_of() got unknown " +
"class_type %s" % class_type)

+ if requested_properties:
+ for p in requested_properties:
+ if p not in rc:
+ rc.extend([p])
+ return rc
+
def _sys_id_child(self, cim_xxx):
"""
Find out the system id of Pool/Volume/Disk/AccessGroup/Initiator
@@ -1100,11 +1105,10 @@ class Smis(IStorageAreaNetwork):
"""
Retrun the PropertyList required for creating new LSM Volume.
"""
- cim_vol_pros = self._property_list_of_id("Volume")
- cim_vol_pros.extend(
- ['OperationalStatus', 'ElementName', 'NameFormat',
- 'NameNamespace', 'BlockSize', 'NumberOfBlocks', 'Name',
- 'OtherIdentifyingInfo', 'IdentifyingDescriptions', 'Usage'])
+ props = ['OperationalStatus', 'ElementName', 'NameFormat',
+ 'NameNamespace', 'BlockSize', 'NumberOfBlocks', 'Name',
+ 'OtherIdentifyingInfo', 'IdentifyingDescriptions', 'Usage']
+ cim_vol_pros = self._property_list_of_id("Volume", props)
return cim_vol_pros

def _new_vol(self, cv, pool_id=None, sys_id=None):
@@ -1605,8 +1609,9 @@ class Smis(IStorageAreaNetwork):
"""
Return a list of properties required to create a LSM System
"""
- cim_sys_pros = self._property_list_of_id('System')
- cim_sys_pros.extend(['ElementName', 'OperationalStatus'])
+ cim_sys_pros = self._property_list_of_id('System',
+ ['ElementName',
+ 'OperationalStatus'])
return cim_sys_pros

@handle_cim_errors
@@ -1637,8 +1642,8 @@ class Smis(IStorageAreaNetwork):
Return a list of properties needed to created Initiator from
CIM_StorageHardwareID
"""
- cim_init_pros = self._property_list_of_id('Initiator')
- cim_init_pros.extend(['IDType', 'ElementName'])
+ cim_init_pros = self._property_list_of_id('Initiator',
+ ['IDType', 'ElementName'])
return cim_init_pros

@handle_cim_errors
@@ -2811,8 +2816,7 @@ class Smis(IStorageAreaNetwork):
opt_pro_dict['element_type'] = Pool.ELEMENT_TYPE_VOLUME
opt_pro_dict['thinp_type'] = Pool.THINP_TYPE_THICK

- pool_id_pros = self._property_list_of_id('Pool')
- pool_id_pros.extend(['Primordial'])
+ pool_id_pros = self._property_list_of_id('Pool', ['Primordial'])
# We use some blacklist here to speed up by skipping unnecessary
# parent pool checking.
# These class are known as Disk Pool, no need to waste time on
@@ -3423,13 +3427,7 @@ class Smis(IStorageAreaNetwork):
this is assumption should work. Tested on EMC SMI-S provider which
provide 1.4, 1.5, 1.6 root profile.
"""
- cim_sys_id_pros = self._property_list_of_id('System')
- if property_list is None:
- property_list = cim_sys_id_pros
- else:
- for key_name in cim_sys_id_pros:
- if key_name not in property_list:
- property_list.extend([key_name])
+ property_list = self._property_list_of_id('System', property_list)

cim_syss = []
if self.fallback_mode:
@@ -3505,14 +3503,7 @@ class Smis(IStorageAreaNetwork):
"""
Return a CIMInstance of CIM_ComputerSystem for given system id.
"""
- cim_sys_pros = self._property_list_of_id("System")
- if property_list is None:
- property_list = cim_sys_pros
- else:
- for pro in cim_sys_pros:
- if pro not in property_list:
- property_list.extend([pro])
-
+ property_list = self._property_list_of_id("System", property_list)
cim_syss = self._root_cim_syss(property_list)

for cim_sys in cim_syss:
@@ -3525,13 +3516,7 @@ class Smis(IStorageAreaNetwork):
"""
Return a CIMInstance of CIM_StoragePool for given pool id.
"""
- cim_sys_pros = self._property_list_of_id("Pool")
- if property_list is None:
- property_list = cim_sys_pros
- else:
- for pro in cim_sys_pros:
- if pro not in property_list:
- property_list.extend([pro])
+ property_list = self._property_list_of_id("Pool", property_list)
cim_pools = self._cim_pools_of(cim_sys_path, property_list)
for cim_pool in cim_pools:
if self._pool_id(cim_pool) == pool_id: