Discussion:
[Libstoragemgmt-devel] [PATCH 7/8] _data.py: Revert change optional data value type
Tony Asleson
2014-06-16 20:26:26 UTC
Permalink
Reverts the change thus preserving the data type associated to
the 'key'.

Signed-off-by: Tony Asleson <***@redhat.com>
---
python_binding/lsm/_data.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index b0ade68..9554c46 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -791,7 +791,7 @@ class OptionalData(IData):
return self._values[key]

def set(self, key, value):
- self._values[str(key)] = str(value)
+ self._values[str(key)] = value


class Capabilities(IData):
--
1.8.2.1
Tony Asleson
2014-06-16 20:26:22 UTC
Permalink
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):
--
1.8.2.1
Tony Asleson
2014-06-16 20:26:23 UTC
Permalink
This was made manditory and thus these optional adds
are incorrect.

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

diff --git a/plugin/smispy/smis.py b/plugin/smispy/smis.py
index cd733bb..a596ca5 100644
--- a/plugin/smispy/smis.py
+++ b/plugin/smispy/smis.py
@@ -1566,6 +1566,8 @@ class Smis(IStorageAreaNetwork):
"""
if not system_id:
system_id = self._sys_id_of_cim_pool(cim_pool)
+
+ status_info = ''
pool_id = self._pool_id(cim_pool)
name = ''
total_space = Pool.TOTAL_SPACE_NOT_FOUND
@@ -1579,9 +1581,10 @@ class Smis(IStorageAreaNetwork):
free_space = cim_pool['RemainingManagedSpace']
if 'OperationalStatus' in cim_pool:
status = Smis._pool_status_of(cim_pool)[0]
+ status_info = Smis._pool_status_of(cim_pool)[1]

- return Pool(pool_id, name, total_space, free_space, status, '',
- system_id)
+ return Pool(pool_id, name, total_space, free_space, status,
+ status_info, system_id)

@staticmethod
def _cim_sys_2_lsm_sys(cim_sys):
@@ -2772,7 +2775,6 @@ class Smis(IStorageAreaNetwork):
'member_type': Pool.MEMBER_TYPE_UNKNOWN,
'member_ids': [],
'element_type': Pool.ELEMENT_TYPE_UNKNOWN,
- 'status_info': '',
}

# check whether current pool support create volume or not.
@@ -2882,9 +2884,6 @@ class Smis(IStorageAreaNetwork):
if raid_type is not None:
opt_pro_dict['raid_type'] = raid_type

- if 'OperationalStatus' in cim_pool:
- opt_pro_dict['status_info'] = Smis._pool_status_of(cim_pool)[1]
-
return opt_pro_dict

@staticmethod
--
1.8.2.1
Tony Asleson
2014-06-16 20:26:20 UTC
Permalink
Signed-off-by: Tony Asleson <***@redhat.com>
---
plugin/sim/simarray.py | 2 +-
plugin/targetd/targetd.py | 5 +----
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/plugin/sim/simarray.py b/plugin/sim/simarray.py
index 10cfc27..c7cc2cc 100644
--- a/plugin/sim/simarray.py
+++ b/plugin/sim/simarray.py
@@ -25,7 +25,7 @@ import tempfile
import os
import time

-from lsm._common import (size_human_2_size_bytes, size_bytes_2_size_human)
+from lsm import (size_human_2_size_bytes, size_bytes_2_size_human)
from lsm import (System, Volume, Disk, Pool, FileSystem, AccessGroup,
Initiator, FsSnapshot, NfsExport, OptionalData, md5,
LsmError, ErrorNumber, JobStatus)
diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index 428cb6c..a996d95 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
@@ -210,10 +210,7 @@ class TargetdStorage(IStorageAreaNetwork, INfs):

if access_group.init_type != AccessGroup.INIT_TYPE_ISCSI_IQN:
raise LsmError(ErrorNumber.NO_SUPPORT,
- "Targetd does not %s(%d) type access group"
- % (AccessGroup._init_type_to_str(
- access_group.init_type),
- access_group.init_type))
+ "Targetd only support ISCSI initiator group type")

ag_id = md5(access_group.init_ids[0])
vol_id = volume.id
--
1.8.2.1
Tony Asleson
2014-06-16 20:26:27 UTC
Permalink
Match the expected naming of a hash data structure for obtaining
a list of keys.

Signed-off-by: Tony Asleson <***@redhat.com>
---
python_binding/lsm/_data.py | 4 ++--
test/plugin_test.py | 2 +-
tools/lsmcli/data_display.py | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 9554c46..6c65faa 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -165,7 +165,7 @@ class IData(object):
else:
#Make sure the properties only contain ones we permit
allowed = set(self.OPT_PROPERTIES)
- actual = set(optional_data.list())
+ actual = set(optional_data.keys())

if actual <= allowed:
return optional_data
@@ -783,7 +783,7 @@ class OptionalData(IData):
else:
self._values = {}

- def list(self):
+ def keys(self):
rc = self._values.keys()
return rc

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 67bc34d..089449d 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -253,7 +253,7 @@ class TestPlugin(unittest.TestCase):

def _get_pool_by_usage(self, system_id, element_type):
for p in self.pool_by_sys_id[system_id]:
- if 'element_type' in p.optional_data.list():
+ if 'element_type' in p.optional_data.keys():
if int(p.optional_data.get('element_type')) == element_type:
return p
return None
diff --git a/tools/lsmcli/data_display.py b/tools/lsmcli/data_display.py
index 05eda29..685e119 100644
--- a/tools/lsmcli/data_display.py
+++ b/tools/lsmcli/data_display.py
@@ -707,7 +707,7 @@ class DisplayData(object):
data_dict[key_str] = value

if flag_dsp_all_data:
- cur_support_opt_keys = obj.optional_data.list()
+ cur_support_opt_keys = obj.optional_data.keys()
for key in optional_headers.keys():
if key not in cur_support_opt_keys:
continue
@@ -718,7 +718,7 @@ class DisplayData(object):
data_dict[key_str] = value

if extra_properties:
- cur_support_opt_keys = obj.optional_data.list()
+ cur_support_opt_keys = obj.optional_data.keys()
for key in extra_properties:
if key in data_dict.keys():
# already contained
--
1.8.2.1
Tony Asleson
2014-06-16 20:26:21 UTC
Permalink
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:
--
1.8.2.1
Tony Asleson
2014-06-16 20:26:24 UTC
Permalink
Running the plugin_test.py yielded a number of
issues which are addressed with this patch.

Signed-off-by: Tony Asleson <***@redhat.com>
---
plugin/ontap/na.py | 45 ++++++++++++++++++-------------
plugin/ontap/ontap.py | 16 ++++++-----
test/plugin_test.py | 74 +++++++++++++++++++++++++++++++++------------------
3 files changed, 84 insertions(+), 51 deletions(-)

diff --git a/plugin/ontap/na.py b/plugin/ontap/na.py
index 668ce30..0986a9c 100644
--- a/plugin/ontap/na.py
+++ b/plugin/ontap/na.py
@@ -379,20 +379,20 @@ class Filer(object):
try:
self._invoke('volume-offline', {'name': vol_name})
online = True
- except FilerError as fe:
- if fe.errno != Filer.EFSDOESNOTEXIST:
- raise fe
+ except FilerError as f_error:
+ if f_error.errno != Filer.EFSDOESNOTEXIST:
+ raise

try:
self._invoke('volume-destroy', {'name': vol_name})
- except FilerError as fe:
+ except FilerError as f_error:
#If the volume was online, we will return it to same status
if online:
try:
self._invoke('volume-online', {'name': vol_name})
except FilerError:
pass
- raise fe
+ raise f_error

def volume_names(self):
"""
@@ -435,20 +435,29 @@ class Filer(object):

while True:
progress = self._invoke('clone-list-status',
- {'clone-id': c_id})['status']['ops-info']
-
- if progress['clone-state'] == 'failed':
- self._invoke('clone-clear', {'clone-id': c_id})
- raise FilerError(progress['error'], progress['reason'])
- elif progress['clone-state'] == 'running' \
- or progress['clone-state'] == 'fail exit':
- # State needs to transition to failed before we can clear it!
- time.sleep(0.2) # Don't hog cpu
- elif progress['clone-state'] == 'completed':
- return progress['destination-file']
+ {'clone-id': c_id})
+
+ # According to the spec the output is optional, if not present
+ # then we are done and good
+ if 'status' in progress:
+ progress = progress['status']['ops-info']
+
+ if progress['clone-state'] == 'failed':
+ self._invoke('clone-clear', {'clone-id': c_id})
+ raise FilerError(progress['error'], progress['reason'])
+ elif progress['clone-state'] == 'running' \
+ or progress['clone-state'] == 'fail exit':
+ # State needs to transition to failed before we can
+ # clear it!
+ time.sleep(0.2) # Don't hog cpu
+ elif progress['clone-state'] == 'completed':
+ return
+ else:
+ raise FilerError(ErrorNumber.NOT_IMPLEMENTED,
+ 'Unexpected state=' +
+ progress['clone-state'])
else:
- raise FilerError(ErrorNumber.NOT_IMPLEMENTED,
- 'Unexpected state=' + progress['clone-state'])
+ return

def lun_online(self, lun_path):
self._invoke('lun-online', {'path': lun_path})
diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index ca3aed2..cde128f 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -65,6 +65,8 @@ def handle_ontap_errors(method):
def na_wrapper(*args, **kwargs):
try:
return method(*args, **kwargs)
+ except LsmError:
+ raise
except na.FilerError as oe:
error, error_msg = error_map(oe)
raise LsmError(error, error_msg)
@@ -178,10 +180,10 @@ class Ontap(IStorageAreaNetwork, INfs):
return self.f.validate()

def time_out_set(self, ms, flags=0):
- self.f.timeout = ms / Ontap.TMO_CONV
+ self.f.timeout = int(ms / Ontap.TMO_CONV)

def time_out_get(self, flags=0):
- return self.f.timeout * Ontap.TMO_CONV
+ return int(self.f.timeout * Ontap.TMO_CONV)

def plugin_unregister(self, flags=0):
pass
@@ -375,7 +377,6 @@ class Ontap(IStorageAreaNetwork, INfs):
opt_data.set('thinp_type', Pool.THINP_TYPE_THICK)
else:
opt_data.set('thinp_type', Pool.THINP_TYPE_UNKNOWN)
- opt_data.set('status_info', self._status_info_of_na_aggr(na_aggr))
element_type = (
Pool.ELEMENT_TYPE_POOL |
Pool.ELEMENT_TYPE_FS |
@@ -385,6 +386,7 @@ class Ontap(IStorageAreaNetwork, INfs):
opt_data.set('element_type', element_type)

return Pool(pool_id, pool_name, total_space, free_space, status,
+ self._status_info_of_na_aggr(na_aggr),
system_id, opt_data)

@staticmethod
@@ -433,10 +435,10 @@ class Ontap(IStorageAreaNetwork, INfs):
opt_data.set('thinp_type', Pool.THINP_TYPE_UNKNOWN)

opt_data.set('status_info', self._status_info_of_na_vol(na_vol))
- element_type = Pool.ELEMENT_TYPE_VOLUME
- opt_data.set('element_type', element_type)
+ opt_data.set('element_type', Pool.ELEMENT_TYPE_VOLUME)

return Pool(pool_id, pool_name, total_space, free_space, status,
+ self._status_info_of_na_vol(na_vol),
system_id, opt_data)

@handle_ontap_errors
@@ -635,7 +637,7 @@ class Ontap(IStorageAreaNetwork, INfs):
luns = self.f.luns_get_specific(aggr=volume.pool_id,
na_volume_name=vol)

- if len(luns) == 1:
+ if len(luns) == 1 and Ontap.LSM_VOL_PREFIX in vol:
self.f.volume_delete(vol)
else:
self.f.lun_delete(volume.name)
@@ -705,7 +707,7 @@ class Ontap(IStorageAreaNetwork, INfs):
#Put volume back to previous size
self._na_resize_recovery(
Ontap._vol_to_na_volume_name(volume_src), -size)
- raise e
+ raise
return None, self._get_volume(dest, volume_src.pool_id)
else:
#TODO Need to get instructions on how to provide this
diff --git a/test/plugin_test.py b/test/plugin_test.py
index 139b1d4..67bc34d 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -176,10 +176,11 @@ class TestProxy(object):

if isinstance(e, lsm.LsmError) and \
e.code != lsm.ErrorNumber.NO_SUPPORT:
- TestProxy.log_result(_proxy_method_name,
- dict(rc=False,
- stack_trace=traceback.format_exc(),
- msg=str(e)))
+ TestProxy.log_result(
+ _proxy_method_name,
+ dict(rc=False,
+ stack_trace=traceback.format_exc(),
+ msg=str(e)))
raise e

# If the job can do async, we will block looping on it.
@@ -240,11 +241,23 @@ class TestPlugin(unittest.TestCase):
self.c = TestProxy(lsm.Client(TestPlugin.URI, TestPlugin.PASSWORD))

self.systems = self.c.systems()
- self.pools = self.c.pools()
+ self.pools = self.c.pools(flags=lsm.Pool.FLAG_RETRIEVE_FULL_INFO)
+
+ self.pool_by_sys_id = {}
+
+ for s in self.systems:
+ self.pool_by_sys_id[s.id] = [p for p in self.pools if
+ p.system_id == s.id]

- self.pool_by_sys_id = dict((p.system_id, p) for p in self.pools)
# TODO Store what exists, so that we don't remove it

+ def _get_pool_by_usage(self, system_id, element_type):
+ for p in self.pool_by_sys_id[system_id]:
+ if 'element_type' in p.optional_data.list():
+ if int(p.optional_data.get('element_type')) == element_type:
+ return p
+ return None
+
def tearDown(self):
# TODO Walk the array looking for stuff we have created and remove it
# What should we do if an array supports a create operation, but not
@@ -270,10 +283,9 @@ class TestPlugin(unittest.TestCase):
pools_list = self.c.pools()
self.assertTrue(len(pools_list) > 0, "We need at least 1 pool to test")

-
@staticmethod
def _vpd_correct(vpd):
- p = re.compile('^[A-F0-9]+$')
+ p = re.compile('^[a-fA-F0-9]+$')

if vpd is not None and len(vpd) > 0 and p.match(vpd) is not None:
return True
@@ -298,25 +310,31 @@ class TestPlugin(unittest.TestCase):

def _volume_create(self, system_id):
if system_id in self.pool_by_sys_id:
- p = self.pool_by_sys_id[system_id]
+ p = self._get_pool_by_usage(system_id,
+ lsm.Pool.ELEMENT_TYPE_VOLUME)

- vol_size = min(p.free_space / 10, mb_in_bytes(512))
+ if p:
+ vol_size = min(p.free_space / 10, mb_in_bytes(512))

- vol = self.c.volume_create(p, rs('volume'), vol_size,
- lsm.Volume.PROVISION_DEFAULT)[1]
+ vol = self.c.volume_create(p, rs('volume'), vol_size,
+ lsm.Volume.PROVISION_DEFAULT)[1]

- self.assertTrue(self._volume_exists(vol.id))
- return vol, p
+ self.assertTrue(self._volume_exists(vol.id))
+ return vol, p

def _fs_create(self, system_id):
if system_id in self.pool_by_sys_id:
- p = self.pool_by_sys_id[system_id]
-
- fs_size = min(p.free_space / 10, mb_in_bytes(512))
- fs = self.c.fs_create(p, rs('fs'), fs_size)[1]
-
- self.assertTrue(self._fs_exists(fs.id))
- return fs, p
+ pools = self.pool_by_sys_id[system_id]
+
+ for p in pools:
+ if p.free_space > mb_in_bytes(250) and \
+ int(p.optional_data.get('element_type')) & \
+ lsm.Pool.ELEMENT_TYPE_FS:
+ fs_size = min(p.free_space / 10, mb_in_bytes(512))
+ fs = self.c.fs_create(p, rs('fs'), fs_size)[1]
+ self.assertTrue(self._fs_exists(fs.id))
+ return fs, p
+ return None, None

def _volume_delete(self, volume):
self.c.volume_delete(volume)
@@ -379,8 +397,8 @@ class TestPlugin(unittest.TestCase):
lsm.Capabilities.VOLUME_DELETE,
lsm.Capabilities.VOLUME_RESIZE]):
vol = self._volume_create(s.id)[0]
- vol_resize = self.c.volume_resize(vol,
- vol.size_bytes * 1.10)[1]
+ vol_resize = self.c.volume_resize(
+ vol, int(vol.size_bytes * 1.10))[1]
self.assertTrue(vol.size_bytes < vol_resize.size_bytes)
self.assertTrue(vol.id == vol_resize.id,
"Expecting re-sized volume to refer to "
@@ -443,13 +461,17 @@ class TestPlugin(unittest.TestCase):
for s in self.systems:
cap = self.c.capabilities(s)

- if supported(cap, [lsm.Capabilities.VOLUME_CREATE,
+ if supported(cap,
+ [lsm.Capabilities.VOLUME_COPY_RANGE_BLOCK_SIZE,
+ lsm.Capabilities.VOLUME_CREATE,
lsm.Capabilities.VOLUME_DELETE,
lsm.Capabilities.VOLUME_COPY_RANGE]):

+ size = self.c.volume_replicate_range_block_size(s)
+
vol, pool = self._volume_create(s.id)

- br = lsm.BlockRange(0, 100, 10)
+ br = lsm.BlockRange(0, size, size)

if supported(
cap, [lsm.Capabilities.VOLUME_COPY_RANGE_CLONE]):
@@ -461,7 +483,7 @@ class TestPlugin(unittest.TestCase):
self.c.volume_replicate_range,
lsm.Volume.REPLICATE_CLONE, vol, vol, [br])

- br = lsm.BlockRange(200, 400, 50)
+ br = lsm.BlockRange(size * 2, size, size)

if supported(
cap, [lsm.Capabilities.VOLUME_COPY_RANGE_COPY]):
--
1.8.2.1
Tony Asleson
2014-06-16 20:26:25 UTC
Permalink
The existing code assumes that optional data is a key/value where the key is
a string type. However, the python code is placing numeric values, string lists
and strings into the optional data which automatically gets serialized and
de-serialized back to the client retaining the types enclosed. I made a change
in a previous commit which changes it to make everything a string and we broke
the display code which was expecting the member id of the optional pool data to
be a list of strings.

This patch adds the following capabilities for optional data type values for
the C API:

-Get/Set long double
-Get/Set uint64_t
-Get/Set int64_t
-Get/Set lsm_string_list

I added a enum and function:

typedef enum {
LSM_OPTIONAL_DATA_INVALID = -2, /**< Invalid */
LSM_OPTIONAL_DATA_NOT_FOUND = -1, /**< Key not found */
LSM_OPTIONAL_DATA_STRING = 1, /**< Contains a string */
LSM_OPTIONAL_DATA_SIGN_INT = 2, /**< Contains a signed int */
LSM_OPTIONAL_DATA_UNSIGNED_INT = 3, /**< Contains an unsigned int */
LSM_OPTIONAL_DATA_REAL = 4, /**< Contains a real number */
LSM_OPTIONAL_DATA_STRING_LIST = 10 /**< Contains a list of strings*/
} lsm_optional_data_type;

lsm_optional_data_type LSM_DLL_EXPORT lsm_optional_data_type_get(
lsm_optional_data *op, const char *key);

Which needs to be utilized to see that type the value is. Once that is
determined then the user can call one of the newly added corresponding get
functions.

However, this is far from ideal for the following reason. The JSON RPC IPC
used between the client and plug-in only has a single number type which
encompasses all of the types listed above (int64_t, uint64_t, long double)
and then some (numbers of arbitrary size). Thus a plug-in could be returning
a value to a client and the 'type' of numeric may vary based on the numerical
value. So a client to be robust would need to handle checking for multiple
numeric types for a single optional value. The only way I see this being
consistent is if we extend the API so that we encode the 'type' and value in
the serialized JSON so that we can consistently give the same 'type' of numeric
to the client application. This patch looks at the numeric string and puts it
in the first available fitting number in this order: int64, uint64, long double.

Any other suggestions people have, please share them.

Thanks!

Regards,
Tony

Signed-off-by: Tony Asleson <***@redhat.com>
---
.../libstoragemgmt/libstoragemgmt_optionaldata.h | 125 +++++++++-
c_binding/lsm_convert.cpp | 77 +++++-
c_binding/lsm_datatypes.cpp | 266 +++++++++++++++++++--
c_binding/lsm_datatypes.hpp | 31 ++-
c_binding/lsm_ipc.cpp | 29 ++-
c_binding/lsm_ipc.hpp | 8 +-
plugin/simc/simc_lsmplugin.c | 2 +-
test/tester.c | 101 +++++++-
8 files changed, 601 insertions(+), 38 deletions(-)

diff --git a/c_binding/include/libstoragemgmt/libstoragemgmt_optionaldata.h b/c_binding/include/libstoragemgmt/libstoragemgmt_optionaldata.h
index cfd6974..5128609 100644
--- a/c_binding/include/libstoragemgmt/libstoragemgmt_optionaldata.h
+++ b/c_binding/include/libstoragemgmt/libstoragemgmt_optionaldata.h
@@ -27,6 +27,26 @@
extern "C" {
#endif

+typedef enum {
+ LSM_OPTIONAL_DATA_INVALID = -2, /**< Invalid */
+ LSM_OPTIONAL_DATA_NOT_FOUND = -1, /**< Key not found */
+ LSM_OPTIONAL_DATA_STRING = 1, /**< Contains a string */
+ LSM_OPTIONAL_DATA_SIGN_INT = 2, /**< Contains a signed int */
+ LSM_OPTIONAL_DATA_UNSIGNED_INT = 3, /**< Contains an unsigned int */
+ LSM_OPTIONAL_DATA_REAL = 4, /**< Contains a real number */
+ LSM_OPTIONAL_DATA_STRING_LIST = 10 /**< Contains a list of strings*/
+} lsm_optional_data_type;
+
+
+/**
+ * Returns the type of data stored.
+ * @param op Record
+ * @param key Key to lookup
+ * @return One of the enumerated types above.
+ */
+lsm_optional_data_type LSM_DLL_EXPORT lsm_optional_data_type_get(
+ lsm_optional_data *op, const char *key);
+
/**
* Free a optional data record
* @param op Record to free.
@@ -38,11 +58,10 @@ int LSM_DLL_EXPORT lsm_optional_data_record_free(lsm_optional_data *op);
* Get the list of 'keys' available in the optional data
* @param [in] op Valid optional data pointer
* @param [out] l String list pointer
- * @param [out] count Number of items in string list
* @return LSM_ERR_OK on success, else error reason
*/
-int LSM_DLL_EXPORT lsm_optional_data_list_get(lsm_optional_data *op,
- lsm_string_list **l, uint32_t *count);
+int LSM_DLL_EXPORT lsm_optional_data_keys(lsm_optional_data *op,
+ lsm_string_list **l);

/**
* Get the value of a key (string)
@@ -71,7 +90,105 @@ int LSM_DLL_EXPORT lsm_optional_data_string_set(lsm_optional_data *op,
* @param src lsm_optional_data to copy
* @return NULL on error/memory allocation failure, else copy
*/
-lsm_optional_data LSM_DLL_EXPORT *lsm_optional_data_record_copy(lsm_optional_data *src);
+lsm_optional_data LSM_DLL_EXPORT *lsm_optional_data_record_copy(
+ lsm_optional_data *src);
+
+/**
+ * Set the value of a key with a signed integer
+ * @param[in] op Valid optional data pointer
+ * @param[in] key Key to set value for (key is copied)
+ * @param[in] value Value to set
+ * @return LSM_ERR_OK on success, else error reason
+ */
+int LSM_DLL_EXPORT lsm_optional_data_int64_set(lsm_optional_data *op,
+ const char *key,
+ int64_t value);
+
+/**
+ * Get the value of the key which is a int64.
+ * Note: lsm_optional_data_type_get needs to return LSM_OPTIONAL_DATA_SIGN_INT
+ * before it is valid to call this function.
+ * @param op
+ * @param key
+ * @return Value, MAX on errors. To determine if value is valid call
+ * lsm_optional_data_type_get first, then you are ensured return value is
+ * correct.
+ */
+int64_t LSM_DLL_EXPORT lsm_optional_data_int64_get(lsm_optional_data *op,
+ const char *key);
+
+/**
+ * Set the value of a key with an unsigned integer
+ * @param[in] op Valid optional data pointer
+ * @param[in] key Key to set value for (key is copied)
+ * @param[in] value Value to set
+ * @return LSM_ERR_OK on success, else error reason
+ */
+int LSM_DLL_EXPORT lsm_optional_data_uint64_set(lsm_optional_data *op,
+ const char *key,
+ uint64_t value);
+
+/**
+ * Get the value of the key which is a uint64.
+ * Note: lsm_optional_data_type_get needs to return
+ * LSM_OPTIONAL_DATA_UNSIGNED_INT before it is valid to call this function.
+ * @param [in] op Valid optional data pointer
+ * @param [in] key Key that exists
+ * @return Value, MAX on errors. To determine if value is valid call
+ * lsm_optional_data_type_get first, then you are ensured return value is
+ * correct.
+ */
+uint64_t LSM_DLL_EXPORT lsm_optional_data_uint64_get(lsm_optional_data *op,
+ const char *key);
+
+/**
+ * Set the value of a key with a real number
+ * @param[in] op Valid optional data pointer
+ * @param[in] key Key to set value for (key is copied)
+ * @param[in] value Value to set
+ * @return LSM_ERR_OK on success, else error reason
+ */
+int LSM_DLL_EXPORT lsm_optional_data_real_set(lsm_optional_data *op,
+ const char *key,
+ long double value);
+
+/**
+ * Get the value of the key which is a real.
+ * Note: lsm_optional_data_type_get needs to return
+ * LSM_OPTIONAL_DATA_REAL before it is valid to call this function.
+ * @param [in] op Valid optional data pointer
+ * @param [in] key Key that exists
+ * @return Value, MAX on errors. To determine if value is valid call
+ * lsm_optional_data_type_get first, then you are ensured return value is
+ * correct.
+ */
+long double LSM_DLL_EXPORT lsm_optional_data_real_get(lsm_optional_data *op,
+ const char *key);
+
+
+/**
+ * Set the value of a key with a string list
+ * @param[in] op Valid optional data pointer
+ * @param[in] key Key to set value for (key is copied)
+ * @param[in] value Value to set
+ * @return LSM_ERR_OK on success, else error reason
+ */
+int LSM_DLL_EXPORT lsm_optional_data_string_list_set(lsm_optional_data *op,
+ const char *key,
+ lsm_string_list *sl);
+
+/**
+ * Get the value of the key which is a string list.
+ * Note: lsm_optional_data_type_get needs to return
+ * LSM_OPTIONAL_DATA_STRING_LIST before it is valid to call this function.
+ * @param [in] op Valid optional data pointer
+ * @param [in] key Key that exists
+ * @return Value, NULL on errors. To determine if value is valid call
+ * lsm_optional_data_type_get first, then you are ensured return value is
+ * correct.
+ */
+lsm_string_list LSM_DLL_EXPORT *lsm_optional_data_string_list_get(
+ lsm_optional_data *op, const char *key);

#ifdef __cplusplus
}
diff --git a/c_binding/lsm_convert.cpp b/c_binding/lsm_convert.cpp
index 86e9e8f..8bb8684 100644
--- a/c_binding/lsm_convert.cpp
+++ b/c_binding/lsm_convert.cpp
@@ -662,15 +662,59 @@ Value capabilities_to_value(lsm_storage_capabilities *cap)
lsm_optional_data *value_to_optional_data(Value &op)
{
lsm_optional_data *rc = NULL;
+ int set_result = 0;
if( is_expected_object(op, "OptionalData") ) {
rc = lsm_optional_data_record_alloc();
if ( rc ) {
std::map<std::string, Value> v = op["values"].asObject();
std::map<std::string, Value>::iterator itr;
for(itr=v.begin(); itr != v.end(); ++itr) {
- if( LSM_ERR_OK != lsm_optional_data_string_set(rc,
- itr->first.c_str(),
- itr->second.asC_str())) {
+ const char *key = itr->first.c_str();
+ Value v = itr->second;
+
+ //TODO: Check return codes of all these sets!
+ switch(v.valueType()){
+ case(Value::string_t): {
+ set_result =
+ lsm_optional_data_string_set(rc, key, v.asC_str());
+ break;
+ }
+ case(Value::numeric_t): {
+ int num_rc = 0;
+ int64_t si = 0;
+ uint64_t ui = 0;
+ long double d = 0;
+
+ num_rc = number_convert(v.asNumString(), &si, &ui, &d);
+ if( num_rc > 0 ) {
+ if( 1 == num_rc ) {
+ set_result =
+ lsm_optional_data_int64_set(rc, key, si);
+ } else if( 2 == num_rc ) {
+ set_result =
+ lsm_optional_data_uint64_set(rc, key, ui);
+ } else if( 3 == num_rc ) {
+ set_result =
+ lsm_optional_data_real_set(rc, key, ui);
+ }
+ }
+ break;
+ }
+ case(Value::array_t): {
+ lsm_string_list *sl = value_to_string_list(v);
+ if( sl ) {
+ set_result =
+ lsm_optional_data_string_list_set(rc, key, sl);
+ lsm_string_list_free(sl);
+ }
+ break;
+ }
+ default:
+ break;
+ }
+
+ /* If the set failed free results and bail */
+ if( LSM_ERR_OK != set_result ) {
lsm_optional_data_record_free(rc);
rc = NULL;
break;
@@ -693,7 +737,32 @@ Value optional_data_to_value(lsm_optional_data *op)

g_hash_table_iter_init(&iter, op->data);
while(g_hash_table_iter_next(&iter, &key, &value)) {
- embedded_values[(const char*)key] = Value((const char*)(value));
+ struct optional_data *od = (struct optional_data *)value;
+ switch( od->t ) {
+ case( LSM_OPTIONAL_DATA_STRING ): {
+ embedded_values[(const char*)key] = Value(od->v.s);
+ break;
+ }
+ case( LSM_OPTIONAL_DATA_STRING_LIST ): {
+ embedded_values[(const char*)key] =
+ string_list_to_value(od->v.sl);
+ break;
+ }
+ case( LSM_OPTIONAL_DATA_SIGN_INT ): {
+ embedded_values[(const char*)key] = Value(od->v.si);
+ break;
+ }
+ case( LSM_OPTIONAL_DATA_UNSIGNED_INT ): {
+ embedded_values[(const char*)key] = Value(od->v.ui);
+ break;
+ }
+ case( LSM_OPTIONAL_DATA_REAL ): {
+ embedded_values[(const char*)key] = Value(od->v.d);
+ break;
+ }
+ default:
+ break;
+ }
}

c["class"] = Value("OptionalData");
diff --git a/c_binding/lsm_datatypes.cpp b/c_binding/lsm_datatypes.cpp
index 3ee76d0..9b004a9 100644
--- a/c_binding/lsm_datatypes.cpp
+++ b/c_binding/lsm_datatypes.cpp
@@ -45,6 +45,9 @@
#include <unistd.h>
#include <dlfcn.h>
#include <glib.h>
+#include <errno.h>
+#include <float.h>
+#include <inttypes.h>

#ifdef __cplusplus
extern "C" {
@@ -1822,6 +1825,44 @@ char* capability_string(lsm_storage_capabilities *c)
return rc;
}

+void optional_data_free(void *d)
+{
+ struct optional_data *op = (struct optional_data *)d;
+ if (op->t == LSM_OPTIONAL_DATA_STRING_LIST ) {
+ lsm_string_list_free(op->v.sl);
+ } else if( op->t == LSM_OPTIONAL_DATA_STRING ) {
+ free(op->v.s);
+ }
+
+ free(d);
+}
+
+struct optional_data *optional_data_copy(struct optional_data *op)
+{
+ struct optional_data *copy = (struct optional_data*)
+ calloc(1, sizeof(optional_data));
+
+ if( copy ) {
+ copy->t = op->t;
+ copy->v = op->v;
+
+ if( op->t == LSM_OPTIONAL_DATA_STRING) {
+ copy->v.s = strdup(op->v.s);
+ if( !copy->v.s ) {
+ optional_data_free(copy);
+ copy = NULL;
+ }
+ } else if( op->t == LSM_OPTIONAL_DATA_STRING_LIST) {
+ copy->v.sl = lsm_string_list_copy(op->v.sl);
+ if( !copy->v.sl ) {
+ optional_data_free(copy);
+ copy = NULL;
+ }
+ }
+ }
+ return copy;
+}
+
lsm_optional_data *lsm_optional_data_record_alloc(void)
{
lsm_optional_data *rc = NULL;
@@ -1829,7 +1870,8 @@ lsm_optional_data *lsm_optional_data_record_alloc(void)
rc = (lsm_optional_data *)malloc(sizeof(lsm_optional_data));
if( rc ) {
rc->magic = LSM_OPTIONAL_DATA_MAGIC;
- rc->data = g_hash_table_new_full(g_str_hash, g_str_equal, free, free);
+ rc->data = g_hash_table_new_full(g_str_hash, g_str_equal, free,
+ optional_data_free);
if ( !rc->data ) {
lsm_optional_data_record_free(rc);
rc = NULL;
@@ -1851,11 +1893,19 @@ lsm_optional_data *lsm_optional_data_record_copy(lsm_optional_data *src)
/* Walk through each from src and duplicate it to dest*/
g_hash_table_iter_init(&iter, src->data);
while(g_hash_table_iter_next(&iter, &key, &value)) {
- if( LSM_ERR_OK != lsm_optional_data_string_set(dest,
- (const char*)key, (const char*)value))
- {
+ char *k_value = strdup((char*)key);
+ struct optional_data *d_value =
+ optional_data_copy((struct optional_data*)value);
+
+ if( k_value && d_value ) {
+ g_hash_table_insert(dest->data, (gpointer)k_value,
+ (gpointer)d_value);
+ } else {
+ free(k_value);
+ optional_data_free(d_value);
lsm_optional_data_record_free(dest);
dest = NULL;
+ break;
}
}
}
@@ -1878,22 +1928,22 @@ int lsm_optional_data_record_free(lsm_optional_data *op)
return LSM_ERR_INVALID_OPTIONAL_DATA;
}

-int lsm_optional_data_list_get(lsm_optional_data *op, lsm_string_list **l,
- uint32_t *count)
+int lsm_optional_data_keys(lsm_optional_data *op, lsm_string_list **l)
{
GHashTableIter iter;
gpointer key;
gpointer value;
+ uint32_t size = 0;

if( LSM_IS_OPTIONAL_DATA(op) ) {
- *count = g_hash_table_size(op->data);
+ size = g_hash_table_size(op->data);

- if( *count ) {
+ if( size ) {
*l = lsm_string_list_alloc(0);
g_hash_table_iter_init(&iter, op->data);
while(g_hash_table_iter_next(&iter, &key, &value)) {
if(LSM_ERR_OK != lsm_string_list_append(*l, (char *)key) ) {
- *count = 0;
+ size = 0;
lsm_string_list_free(*l);
*l = NULL;
return LSM_ERR_NO_MEMORY;
@@ -1905,13 +1955,52 @@ int lsm_optional_data_list_get(lsm_optional_data *op, lsm_string_list **l,
return LSM_ERR_INVALID_OPTIONAL_DATA;
}

-const char *lsm_optional_data_string_get(lsm_optional_data *op,
+lsm_optional_data_type lsm_optional_data_type_get(lsm_optional_data *op,
const char *key)
{
if( LSM_IS_OPTIONAL_DATA(op) ) {
- return (const char*)g_hash_table_lookup(op->data, key);
+ struct optional_data *od = (struct optional_data *)
+ g_hash_table_lookup(op->data, key);
+ if( od ) {
+ return od->t;
+ }
+ return LSM_OPTIONAL_DATA_NOT_FOUND;
}
- return NULL;
+ return LSM_OPTIONAL_DATA_INVALID;
+}
+
+
+#define OP_GETTER(name, return_type, type_value, member, error_value) \
+return_type name(lsm_optional_data *op, const char *key) \
+{ \
+ if( LSM_IS_OPTIONAL_DATA(op) ) { \
+ struct optional_data *od = (struct optional_data *) \
+ g_hash_table_lookup(op->data, key); \
+ if( od ) { \
+ if( od->t == type_value ) { \
+ return od->v.member; \
+ } \
+ } \
+ } \
+ return error_value; \
+} \
+
+OP_GETTER(lsm_optional_data_string_list_get, lsm_string_list *,
+ LSM_OPTIONAL_DATA_STRING_LIST, sl, NULL)
+OP_GETTER(lsm_optional_data_string_get, const char *, LSM_OPTIONAL_DATA_STRING,
+ s, NULL)
+OP_GETTER(lsm_optional_data_int64_get, int64_t, LSM_OPTIONAL_DATA_SIGN_INT, si,
+ LLONG_MAX)
+OP_GETTER(lsm_optional_data_uint64_get, uint64_t,
+ LSM_OPTIONAL_DATA_UNSIGNED_INT, ui, ULLONG_MAX)
+OP_GETTER(lsm_optional_data_real_get, long double, LSM_OPTIONAL_DATA_REAL, d,
+ LDBL_MAX)
+
+static void insert_value(GHashTable *ht, const char *key,
+ struct optional_data *value)
+{
+ g_hash_table_remove(ht, (gpointer)key);
+ g_hash_table_insert(ht, (gpointer)key, (gpointer)value);
}

int lsm_optional_data_string_set(lsm_optional_data *op,
@@ -1920,14 +2009,18 @@ int lsm_optional_data_string_set(lsm_optional_data *op,
{
if( LSM_IS_OPTIONAL_DATA(op) ) {
char *k_value = strdup(key);
- char *d_value = strdup(value);
-
- if( k_value && d_value ) {
- g_hash_table_remove(op->data, (gpointer)k_value);
- g_hash_table_insert(op->data, (gpointer)k_value, (gpointer)d_value);
+ char *op_value = strdup(value);
+ struct optional_data *d_value =
+ (struct optional_data *)calloc(1, sizeof(struct optional_data));
+
+ if( k_value && op_value && d_value) {
+ d_value->t = LSM_OPTIONAL_DATA_STRING;
+ d_value->v.s = op_value;
+ insert_value(op->data, k_value, d_value);
return LSM_ERR_OK;
} else {
free(k_value);
+ free(op_value);
free(d_value);
return LSM_ERR_NO_MEMORY;
}
@@ -1935,6 +2028,145 @@ int lsm_optional_data_string_set(lsm_optional_data *op,
return LSM_ERR_INVALID_OPTIONAL_DATA;
}

+int lsm_optional_data_string_list_set(lsm_optional_data *op, const char *key,
+ lsm_string_list *value)
+{
+ int rc = LSM_ERR_INVALID_OPTIONAL_DATA;
+ if( LSM_IS_OPTIONAL_DATA(op) ) {
+ char *k_value = strdup(key);
+ lsm_string_list *copy = lsm_string_list_copy(value);
+ struct optional_data *d_value =
+ (struct optional_data *)calloc(1, sizeof(struct optional_data));
+
+ if( k_value && copy && d_value ) {
+ d_value->t = LSM_OPTIONAL_DATA_STRING_LIST;
+ d_value->v.sl = copy;
+ insert_value(op->data, k_value, d_value);
+ rc = LSM_ERR_OK;
+ } else {
+ free(k_value);
+ free(d_value);
+ lsm_string_list_free(copy);
+ rc = LSM_ERR_NO_MEMORY;
+ }
+ }
+ return rc;
+}
+
+int lsm_optional_data_int64_set(lsm_optional_data *op, const char *key,
+ int64_t value)
+{
+ int rc = LSM_ERR_INVALID_OPTIONAL_DATA;
+ if( LSM_IS_OPTIONAL_DATA(op) ) {
+ char *k_value = strdup(key);
+ struct optional_data *d_value =
+ (struct optional_data *)calloc(1, sizeof(struct optional_data));
+
+ if( k_value && d_value ) {
+ d_value->t = LSM_OPTIONAL_DATA_SIGN_INT;
+ d_value->v.si = value;
+ insert_value(op->data, k_value, d_value);
+ rc = LSM_ERR_OK;
+ } else {
+ free(k_value);
+ free(d_value);
+ rc = LSM_ERR_NO_MEMORY;
+ }
+ }
+ return rc;
+}
+
+int lsm_optional_data_uint64_set(lsm_optional_data *op, const char *key,
+ uint64_t value)
+{
+ int rc = LSM_ERR_INVALID_OPTIONAL_DATA;
+ if( LSM_IS_OPTIONAL_DATA(op) ) {
+ char *k_value = strdup(key);
+ struct optional_data *d_value =
+ (struct optional_data *)calloc(1, sizeof(struct optional_data));
+
+ if( k_value && d_value ) {
+ d_value->t = LSM_OPTIONAL_DATA_UNSIGNED_INT;
+ d_value->v.ui = value;
+ insert_value(op->data, k_value, d_value);
+ rc = LSM_ERR_OK;
+ } else {
+ free(k_value);
+ free(d_value);
+ rc = LSM_ERR_NO_MEMORY;
+ }
+ }
+ return rc;
+}
+
+int lsm_optional_data_real_set(lsm_optional_data *op, const char *key,
+ long double value)
+{
+ int rc = LSM_ERR_INVALID_OPTIONAL_DATA;
+ if( LSM_IS_OPTIONAL_DATA(op) ) {
+ char *k_value = strdup(key);
+ struct optional_data *d_value =
+ (struct optional_data *)calloc(1, sizeof(struct optional_data));
+
+ if( k_value && d_value ) {
+ d_value->t = LSM_OPTIONAL_DATA_REAL;
+ d_value->v.d = value;
+ insert_value(op->data, k_value, d_value);
+ rc = LSM_ERR_OK;
+ } else {
+ free(k_value);
+ free(d_value);
+ rc = LSM_ERR_NO_MEMORY;
+ }
+ }
+ return rc;
+}
+
+int number_convert(const char *str_num, int64_t *si, uint64_t *ui,
+ long double *d)
+{
+ int rc = -1;
+ char *end = NULL;
+
+ if( str_num && str_num != '\0' && strlen(str_num) ) {
+ *si = 0;
+ *ui = 0;
+ *d = 0.0;
+
+ errno = 0;
+ *si = strtoll(str_num, &end, 10);
+ if( errno == 0 && *end == '\0' ) {
+ rc = 1; /* Signed number */
+ }
+
+ /* strtoull will convert negative, not wanted so skip if it has '-' */
+ if( -1 == rc && str_num[0] != '-') {
+ end = NULL;
+ errno = 0;
+ *ui = strtoull(str_num, &end, 10);
+
+ if( errno == 0 && *end == '\0' ) {
+ rc = 2; /* Unsigned number */
+ }
+ }
+
+ if( -1 == rc ) {
+ end = NULL;
+ errno = 0;
+
+ *d = strtold(str_num, &end);
+ if( errno == 0 && *end == '\0' ) {
+ rc = 3; /* Real number */
+ }
+ }
+
+ if( -1 == rc ) {
+ rc = 0; /* Not a number */
+ }
+ }
+ return rc;
+}
+
#ifdef __cplusplus
}
#endif
diff --git a/c_binding/lsm_datatypes.hpp b/c_binding/lsm_datatypes.hpp
index cc6f65f..c6ac9c5 100644
--- a/c_binding/lsm_datatypes.hpp
+++ b/c_binding/lsm_datatypes.hpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2011-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
@@ -303,6 +303,18 @@ struct LSM_DLL_LOCAL _lsm_optional_data {
GHashTable *data;
};

+union optional_value {
+ char *s;
+ long double d;
+ int64_t si;
+ uint64_t ui;
+ lsm_string_list *sl;
+};
+struct optional_data {
+ lsm_optional_data_type t;
+ union optional_value v;
+};
+
/**
* Returns a pointer to a newly created connection structure.
* @return NULL on memory exhaustion, else new connection.
@@ -336,6 +348,23 @@ LSM_DLL_LOCAL char* capability_string(lsm_storage_capabilities *c);

LSM_DLL_LOCAL const char *uds_path(void);

+/**
+ * Take a character string and tries to convert to a number.
+ * Note: Number is defined as what is acceptable for JSON number. The number
+ * is represented by int64_t if possible, else uint64_t and then long double.
+ * @param str_num Character string containing number
+ * @param si Signed 64 bit number
+ * @param ui Unsigned 64 bit number
+ * @param d Long double
+ * @return -1 = Invalid string pointer
+ * 0 = Not a number
+ * 1 = Number converted to signed integer, value in si
+ * 2 = Number converted to unsigned integer, value in ui
+ * 3 = Number converted to long double, value in d
+ */
+int LSM_DLL_LOCAL number_convert(const char *str_num, int64_t *si, uint64_t *ui,
+ long double *d);
+
#ifdef __cplusplus
}
#endif
diff --git a/c_binding/lsm_ipc.cpp b/c_binding/lsm_ipc.cpp
index de856e0..59684bf 100644
--- a/c_binding/lsm_ipc.cpp
+++ b/c_binding/lsm_ipc.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2011-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
@@ -224,6 +224,10 @@ Value::Value(double v) : t(numeric_t), s(to_string(v))
{
}

+Value::Value(long double v) : t(numeric_t), s(to_string(v))
+{
+}
+
Value::Value(uint32_t v) : t(numeric_t), s(to_string(v))
{
}
@@ -342,6 +346,16 @@ Value Value::getValue( const char* key )
return Value();
}

+const char * Value::asNumString()
+{
+ const char *rc = NULL;
+
+ if (t == numeric_t) {
+ rc = s.c_str();
+ }
+ return rc;
+}
+
void * Value::asVoid()
{
if (t == null_t) {
@@ -371,6 +385,19 @@ double Value::asDouble()
throw ValueException("Value not numeric");
}

+long double Value::asLongDouble()
+{
+ if (t == numeric_t) {
+ long double rc;
+
+ if (sscanf(s.c_str(), "%Lf", &rc) > 0) {
+ return rc;
+ }
+ throw ValueException("Value not a long double");
+ }
+ throw ValueException("Value not numeric");
+}
+
int32_t Value::asInt32_t()
{
if (t == numeric_t) {
diff --git a/c_binding/lsm_ipc.hpp b/c_binding/lsm_ipc.hpp
index 54f53de..c250efc 100644
--- a/c_binding/lsm_ipc.hpp
+++ b/c_binding/lsm_ipc.hpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2011-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
@@ -200,6 +200,7 @@ public:
* @param v value
*/
Value(double v);
+ Value(long double v);

/**
* Numeric unsigned 32 constructor
@@ -301,6 +302,10 @@ public:
*/
Value getValue(const char* key);

+ /**
+ * Returns a numeric as the string holding it.
+ */
+ const char* asNumString();

/**
* Returns NULL if void type, else ValueException
@@ -319,6 +324,7 @@ public:
* @return double value else ValueException on error
*/
double asDouble();
+ long double asLongDouble();

/**
* Signed 32 integer value represented by object.
diff --git a/plugin/simc/simc_lsmplugin.c b/plugin/simc/simc_lsmplugin.c
index a9321d2..045a360 100644
--- a/plugin/simc/simc_lsmplugin.c
+++ b/plugin/simc/simc_lsmplugin.c
@@ -2574,7 +2574,7 @@ int load( lsm_plugin_ptr c, const char *uri, const char *password,


snprintf(name, sizeof(name), "Sim C disk %d", i);
- snprintf(sn, sizeof(sn), "SIMDISKSN00000%04d\n", i);
+ snprintf(sn, sizeof(sn), "SIMDISKSN00000%04d", i);

lsm_optional_data_string_set(od, "sn", sn);

diff --git a/test/tester.c b/test/tester.c
index 07136bf..7b8e528 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -1039,7 +1039,6 @@ START_TEST(test_disks)
const char *name;
const char *system_id;
int i = 0;
- uint32_t key_count = 0;
lsm_string_list *keys = NULL;
const char *key = NULL;
const char *data = NULL;
@@ -1048,7 +1047,8 @@ START_TEST(test_disks)

fail_unless(c!=NULL);

- int rc = lsm_disk_list(c, NULL, NULL, &d, &count, 0);
+ int rc = lsm_disk_list(c, NULL, NULL, &d, &count,
+ LSM_DISK_FLAG_RETRIEVE_FULL_INFO);

if( LSM_ERR_OK == rc ) {
fail_unless(LSM_ERR_OK == rc, "%d", rc);
@@ -1080,16 +1080,36 @@ START_TEST(test_disks)

od = lsm_disk_optional_data_get(d[i]);
if( od ) {
+ lsm_optional_data_type t;
+
/* Iterate through the keys, grabbing the data */
- rc = lsm_optional_data_list_get(od, &keys, &key_count);
- if( LSM_ERR_OK == rc && keys != NULL && key_count > 0 ) {
- //uint32_t num_keys = lsmStringListSize(keys);
- //fail_unless( num_keys == key_count, "%d != %d", num_keys, key_count);
- for(j = 0; j < key_count; ++j ) {
+ rc = lsm_optional_data_keys(od, &keys);
+ if( LSM_ERR_OK == rc && keys != NULL &&
+ lsm_string_list_size(keys) > 0 ) {
+ for(j = 0; j < lsm_string_list_size(keys); ++j ) {
key = lsm_string_list_elem_get(keys, j);
- data = lsm_optional_data_string_get(od, key);
+
fail_unless(key != NULL && strlen(key) > 0);
- fail_unless(data != NULL && strlen(data) > 0);
+
+ t = lsm_optional_data_type_get(od, key);
+ if( LSM_OPTIONAL_DATA_STRING == t ) {
+ data = lsm_optional_data_string_get(od, key);
+ printf("Key=%s, Data=%s\n", key, data);
+ fail_unless(data != NULL && strlen(data) > 0);
+ } else if( LSM_OPTIONAL_DATA_STRING_LIST == t ) {
+ lsm_string_list *sl = lsm_optional_data_string_list_get(od, key);
+ if( sl ) {
+ int size = lsm_string_list_size(sl);
+ int j;
+
+ printf("Key=%s, Data=", key);
+
+ for( j = 0; j < size; j++ ) {
+ printf("%s ", lsm_string_list_elem_get(sl, j));
+ }
+ printf("\n");
+ }
+ }
}

if( keys ) {
@@ -3096,6 +3116,68 @@ START_TEST(test_search_fs)
}
END_TEST

+START_TEST(test_pool_listing)
+{
+ lsm_pool **pools = NULL;
+ uint32_t count = 0;
+ lsm_optional_data *op = NULL;
+ uint32_t i,j = 0;
+ lsm_optional_data_type t;
+ lsm_string_list *keys = NULL;
+
+ int rc = lsm_pool_list(c, NULL, NULL, &pools, &count,
+ LSM_POOL_FLAG_RETRIEVE_FULL_INFO);
+ if( LSM_ERR_OK == rc ) {
+ for( i = 0; i < count; ++i ) {
+ op = lsm_pool_optional_data_get(pools[i]);
+
+ if( op ) {
+ rc = lsm_optional_data_keys(op, &keys);
+ fail_unless(LSM_ERR_OK == rc, "lsm_optional_data_keys %d", rc);
+
+ if(LSM_ERR_OK == rc ) {
+
+ for(j = 0; j < lsm_string_list_size(keys); ++j ) {
+ const char *key = lsm_string_list_elem_get(keys, j);
+ t = lsm_optional_data_type_get(op, key);
+ if( LSM_OPTIONAL_DATA_STRING == t ) {
+ const char *value = lsm_optional_data_string_get(op, key);
+ fail_unless(value != NULL && strlen(value) > 0);
+ printf("%s=%s\n", key, value);
+ } else if( LSM_OPTIONAL_DATA_STRING_LIST == t ) {
+ lsm_string_list *v = lsm_optional_data_string_list_get(op, key);
+ if( v ) {
+ uint32_t k;
+
+ printf("%s=[", key);
+ for( k = 0; k < lsm_string_list_size(v); ++k ) {
+ printf("%s ",
+ lsm_string_list_elem_get(v, k));
+ }
+ printf("]\n");
+ lsm_string_list_free(v);
+ }
+ } else if( LSM_OPTIONAL_DATA_SIGN_INT == t ) {
+ int64_t value = lsm_optional_data_int64_get(op, key);
+ printf("%s:%" PRId64 "\n", key, value);
+ }
+ }
+
+ rc = lsm_string_list_free(keys);
+ fail_unless(LSM_ERR_OK == rc, "lsm_string_list_free %d", rc);
+ }
+
+ rc = lsm_optional_data_record_free(op);
+ fail_unless(LSM_ERR_OK == rc, "lsm_optional_data_record_free %d", rc);
+ }
+ }
+
+ rc = lsm_pool_record_array_free(pools, count);
+ fail_unless(LSM_ERR_OK == rc, "lsm_pool_record_array_free %d", rc);
+ }
+}
+END_TEST
+
Suite * lsm_suite(void)
{
Suite *s = suite_create("libStorageMgmt");
@@ -3103,6 +3185,7 @@ Suite * lsm_suite(void)
TCase *basic = tcase_create("Basic");
tcase_add_checked_fixture (basic, setup, teardown);

+ tcase_add_test(basic, test_pool_listing);
tcase_add_test(basic, test_search_fs);
tcase_add_test(basic, test_search_access_groups);
tcase_add_test(basic, test_search_disks);
--
1.8.2.1
Gris Ge
2014-06-17 12:06:23 UTC
Permalink
My uncommitted patch stack for review. Gris was unable to
apply my other patches cleanly so I'm reposting all of them.
These will apply against current master.
The last three deal with optional data changes, the others fix
bugs.
I got error when running "make check":

tester-tester.o: In function `test_pool_listing':
test/tester.c:3132: undefined reference to `lsm_pool_optional_data_get'

Besides that, I am OK for the changes.
I already rebased my patches against this set, in stead of making a V2,
we can fix that in a another patch.

Thanks.
--
Gris Ge
Tony Asleson
2014-06-17 17:17:32 UTC
Permalink
Post by Gris Ge
My uncommitted patch stack for review. Gris was unable to
apply my other patches cleanly so I'm reposting all of them.
These will apply against current master.
The last three deal with optional data changes, the others fix
bugs.
test/tester.c:3132: undefined reference to `lsm_pool_optional_data_get'
OK, this is different than the patch applying cleaning.

Thanks

Regards,
-Tony

Loading...