Discussion:
[Libstoragemgmt-devel] [PATCH 1/2] smis.py: remove opt status_info
Tony Asleson
2014-06-13 20:58:24 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-13 20:58:25 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-13 20:58: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-13 20:58:26 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-13 21:17:52 UTC
Permalink
Ignore, mistake and duplicate send.
Post by Tony Asleson
Running the plugin_test.py yielded a number of
issues which are addressed with this patch.
---
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
self._invoke('volume-offline', {'name': vol_name})
online = True
- raise fe
+ raise
self._invoke('volume-destroy', {'name': vol_name})
#If the volume was online, we will return it to same status
self._invoke('volume-online', {'name': vol_name})
pass
- raise fe
+ raise f_error
"""
progress = self._invoke('clone-list-status',
- {'clone-id': c_id})['status']['ops-info']
-
- self._invoke('clone-clear', {'clone-id': c_id})
- raise FilerError(progress['error'], progress['reason'])
- elif progress['clone-state'] == 'running' \
- # State needs to transition to failed before we can clear it!
- time.sleep(0.2) # Don't hog cpu
- 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
+ progress = progress['status']['ops-info']
+
+ self._invoke('clone-clear', {'clone-id': c_id})
+ raise FilerError(progress['error'], progress['reason'])
+ elif progress['clone-state'] == 'running' \
+ # State needs to transition to failed before we can
+ # clear it!
+ time.sleep(0.2) # Don't hog cpu
+ return
+ raise FilerError(ErrorNumber.NOT_IMPLEMENTED,
+ 'Unexpected state=' +
+ progress['clone-state'])
- raise FilerError(ErrorNumber.NOT_IMPLEMENTED,
- 'Unexpected state=' + progress['clone-state'])
+ return
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
return method(*args, **kwargs)
+ raise
error, error_msg = error_map(oe)
raise LsmError(error, error_msg)
return self.f.validate()
- self.f.timeout = ms / Ontap.TMO_CONV
+ self.f.timeout = int(ms / Ontap.TMO_CONV)
- return self.f.timeout * Ontap.TMO_CONV
+ return int(self.f.timeout * Ontap.TMO_CONV)
pass
opt_data.set('thinp_type', Pool.THINP_TYPE_THICK)
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 |
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
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
luns = self.f.luns_get_specific(aggr=volume.pool_id,
na_volume_name=vol)
self.f.volume_delete(vol)
self.f.lun_delete(volume.name)
#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)
#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
if isinstance(e, lsm.LsmError) and \
- 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.
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 = {}
+
+ 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
+ return p
+ return None
+
# 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
pools_list = self.c.pools()
self.assertTrue(len(pools_list) > 0, "We need at least 1 pool to test")
-
@staticmethod
- p = re.compile('^[A-F0-9]+$')
+ p = re.compile('^[a-fA-F0-9]+$')
return True
- 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))
+ 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
- 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]
+
+ if p.free_space > mb_in_bytes(250) and \
+ int(p.optional_data.get('element_type')) & \
+ 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
self.c.volume_delete(volume)
lsm.Capabilities.VOLUME_DELETE,
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 "
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,
+ 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(
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(
Tony Asleson
2014-06-13 21:17:32 UTC
Permalink
Ignore, mistake and duplicate send.
Post by Tony Asleson
This was made manditory and thus these optional adds
are incorrect.
---
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
"""
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
free_space = cim_pool['RemainingManagedSpace']
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
'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.
opt_pro_dict['raid_type'] = raid_type
- opt_pro_dict['status_info'] = Smis._pool_status_of(cim_pool)[1]
-
return opt_pro_dict
@staticmethod
Gris Ge
2014-06-16 07:26:01 UTC
Permalink
Any other suggestions people have, please share them.
Thanks!
Regards,
Tony
Tony,

The patch does not commit clean.

Some ideas about this:
1. Can we add each optional property a dedicate method, like:
int lsm_pool_member_ids_get(lsm_pool *p, char **member_ids[],
uint32 *member_count)
2. I like hard-coded in document for data type in stead of detect on
runtime.

3. We are keeping two set of API: OOO for python, Non-OOO for C.
To make things simpler, can we use non-OOO for both python and C?
libvirt are providing auto-generated python API/binding.
Maybe in LSM version 2?

Thanks.
--
Gris Ge
Tony Asleson
2014-06-16 15:12:45 UTC
Permalink
Post by Gris Ge
Any other suggestions people have, please share them.
Thanks!
Regards,
Tony
Tony,
The patch does not commit clean.
I will send out a new patch series in a little bit.
Post by Gris Ge
int lsm_pool_member_ids_get(lsm_pool *p, char **member_ids[],
uint32 *member_count)
This is exactly what we do for mandatory things, but not for optional.
For the C API we do have an abstraction for string list so this call
would actually be something like this if we provided it.

int lsm_pool_member_ids_get(lsm_pool *p, lsm_string_list **ids);
Post by Gris Ge
2. I like hard-coded in document for data type in stead of detect on
runtime.
I'm thinking we need to extend the optional data so that we encode the
type of number and it's value into a string and have methods added in
python to set the specific type and value. Kind of like what pywbem
does with numeric data types. JSON would treat is as string, but
library would re-encode it to the correct numeric form, especially for
languages like C which need this.
Post by Gris Ge
3. We are keeping two set of API: OOO for python, Non-OOO for C.
To make things simpler, can we use non-OOO for both python and C?
Actually the python API is not OOP IMHO. It's really just the C API
with the first parameter not needing to be specified and a few less
parameters where python has more feature rich data types. If it was
truly OOP the user would use a volume object reference and use a method
on it to resize, copy etc. instead of calling a method where you pass
the object.
Post by Gris Ge
libvirt are providing auto-generated python API/binding.
Maybe in LSM version 2?
I tried to utilize one of the auto generated tools for making the python
bindings for the C library and decided against it for the following reasons:

1. The entire code base would be in C, not python and debugging python
client issues would result in debugging C code. So memory leaks,
segmentation faults etc. would all be real possible issues for python
users. Full python == full debug in native language.

2. The Client API and Plug-in API would need python bindings, take #1 x2.

3. The amount of code to implement the RPC and JSON serialization in
python is trivial compared to the amount of C code to produce the C
bindings. This is both a blessing and a curse. It's so easy to add new
functionality in Python that its easy to underestimate the amount of
work to add it in C.

4. Some other languages don't have the ability to auto generate bindings
for C so this was a good way to ensure we are truly language agnostic.

Regards,
Tony
Andy Grover
2014-06-16 23:15:43 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.
I'm not clear on what the point of optional data is. So, this is stuff
that lsmcli can be asked to show with the -o option, but now you're
talking about handling types other than string?

Is this data intended just as additional information for lsmcli users,
or is it actually PART of the lsbsm API, and thus needing to be typed so
that a program can parse it?

The way that optional data is going, it's looking like an escape hatch
where any sort of extra, vendor-specific stuff can go, which for a
library that is supposed to abstract this from the user, seems like
trouble. I think the goal of libsm should be that clients just need
*ONE* code path calling it to do whatever -- it seems a lot less useful
if clients need to branch on what they see in optional data, and then do
different things. This will mean that libsm hasn't actually abstracted
or simplified anything.

I would recommend eliminating optional data. If there's something in
there that you need, then it isn't really optional :) Failing that, if
you keep it around, it should explicitly be for human and not
programmatic consumption, and one way to make that clear would be to not
permit types beyond string.

Regards -- Andy
Andy Grover
2014-06-16 23:15:50 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.
I'm not clear on what the point of optional data is. So, this is stuff
that lsmcli can be asked to show with the -o option, but now you're
talking about handling types other than string?

Is this data intended just as additional information for lsmcli users,
or is it actually PART of the lsbsm API, and thus needing to be typed so
that a program can parse it?

The way that optional data is going, it's looking like an escape hatch
where any sort of extra, vendor-specific stuff can go, which for a
library that is supposed to abstract this from the user, seems like
trouble. I think the goal of libsm should be that clients just need
*ONE* code path calling it to do whatever -- it seems a lot less useful
if clients need to branch on what they see in optional data, and then do
different things. This will mean that libsm hasn't actually abstracted
or simplified anything.

I would recommend eliminating optional data. If there's something in
there that you need, then it isn't really optional :) Failing that, if
you keep it around, it should explicitly be for human and not
programmatic consumption, and one way to make that clear would be to not
permit types beyond string.

Regards -- Andy
Gris Ge
2014-06-17 07:11:43 UTC
Permalink
Post by Andy Grover
I would recommend eliminating optional data. If there's something in
there that you need, then it isn't really optional :) Failing that, if
you keep it around, it should explicitly be for human and not
programmatic consumption, and one way to make that clear would be to not
permit types beyond string.
Regards -- Andy
Hi Andy,

Currently optional data is used to store properties used by advanced
user like pool raid members, raid level, thin provisioning type .

Eliminating optional data means plugin should store all properties when
initialising the lsm.XXX object which might be some performance issue,
for example:

SMI-S will take extra WBEM calls to get pool member ids. Some array
like EMC VNX, that will take 30 seconds or more. [1]

But this is could be fixed as by working with vendors and improving the plugin.

So generally, I am agree on eliminating optional data.

Thank you for the advices.
Best regards.

[1] Ideally, we could get enough information for the whole lsm.Pool
through one array query call. But we don't have that influence in
SMI-S yet.
--
Gris Ge
Tony Asleson
2014-06-17 15:21:17 UTC
Permalink
Post by Gris Ge
Post by Andy Grover
I would recommend eliminating optional data. If there's something in
there that you need, then it isn't really optional
So generally, I am agree on eliminating optional data.
I agree as well, lets take it out. I will work on a patch today that
does just that. Additional parameters to any class can be added to the
library at any time, just not removed. However, we need to make sure
that whatever is added can be supplied by all plug-in providers.

Regards,
Tony

Loading...