[PATCH 1/2] C Library: Fix potential memory leak in handle_volume_raid_create().

[Gris Ge]

* Free disks memory when value_array_to_disks() failed.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_plugin_ipc.cpp | 1 +
1 file changed, 1 insertion(+)

diff --git a/c_binding/lsm_plugin_ipc.cpp b/c_binding/lsm_plugin_ipc.cpp
index 890e1cb..575f02d 100644
--- a/c_binding/lsm_plugin_ipc.cpp
+++ b/c_binding/lsm_plugin_ipc.cpp
@@ -2314,6 +2314,7 @@ static int handle_volume_raid_create(lsm_plugin_ptr p, Value & params,
uint32_t disk_count = 0;
rc = value_array_to_disks(v_disks, &disks, &disk_count);
if (LSM_ERR_OK != rc) {
+ lsm_disk_record_array_free(disks, disk_count);
return rc;
}

[Gris Ge]

* Add dmtf.OP_STATUS_DORMANT support, treat it as Pool.STATUS_STOPPED.

* Change dmtf._op_status_to_str() to public as dmtf.op_status_to_str().

* Change dmtf.op_status_to_str() to return raw number in string if not common
know value.

* Only check the primary(first) value of CIM_StoragePool['OperationalStatus'].

* Put extra OperationalStatus into status_info.

* Tested on all hardwares I found, got no extra OperationalStatus.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/dmtf.py | 8 ++++----
plugin/smispy/smis_pool.py | 21 +++++++++++++++++++--
2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index f3fe91a..249d5f6 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -156,15 +156,15 @@
}


-def _op_status_to_str(dmtf_op_status):
+def op_status_to_str(dmtf_op_status):
"""
Just convert integer to string. NOT ALLOWING provide a list.
- Return emtpy string is not found.
+ Return a string containing the raw number if not found.
"""
try:
return _OP_STATUS_STR_CONV[dmtf_op_status]
except KeyError:
- return ''
+ return '%d' % dmtf_op_status


def op_status_list_conv(conv_dict, dmtf_op_status_list,
@@ -177,7 +177,7 @@ def op_status_list_conv(conv_dict, dmtf_op_status_list,
else:
if dmtf_op_status in _OP_STATUS_STR_CONV.keys():
status |= other_value
- status_info_list.append(_op_status_to_str(dmtf_op_status))
+ status_info_list.append(op_status_to_str(dmtf_op_status))
continue
if status == 0:
status = unknown_value
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index cb06867..52fbade 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -189,17 +189,34 @@ def _pool_element_type(smis_common, cim_pool):
dmtf.OP_STATUS_DEGRADED: Pool.STATUS_DEGRADED,
dmtf.OP_STATUS_NON_RECOVERABLE_ERROR: Pool.STATUS_ERROR,
dmtf.OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: Pool.STATUS_ERROR,
+ dmtf.OP_STATUS_DORMANT: Pool.STATUS_STOPPED,
}


def _pool_status_of_cim_pool(dmtf_op_status_list):
"""
Convert CIM_StoragePool['OperationalStatus'] to LSM
+ As SMI-S 1.6 block book section 5.2.1 'StoragePool OperationalStatus'
+ indicate, we only check primary OperationalStatus here, if additional
+ OperationalStatus found, we add it to status_info.
"""
- return dmtf.op_status_list_conv(
- _LSM_POOL_OP_STATUS_CONV, dmtf_op_status_list,
+ if len(dmtf_op_status_list) == 0:
+ raise LsmError(
+ ErrorNumber.PLUGIN_BUG,
+ "Got empty OperationalStatus")
+
+ lsm_status, status_info = dmtf.op_status_list_conv(
+ _LSM_POOL_OP_STATUS_CONV, [dmtf_op_status_list[0]],
Pool.STATUS_UNKNOWN, Pool.STATUS_OTHER)

+ if len(dmtf_op_status_list) > 1:
+ extra_status_str_list = [dmtf.op_status_to_str(x)
+ for x in dmtf_op_status_list[1:]]
+ status_info += " Extra OperationalStatus: %s" % \
+ " ".join(extra_status_str_list)
+
+ return lsm_status, status_info
+

def cim_pool_to_lsm_pool(smis_common, cim_pool, system_id):
"""

[Gris Ge]

* Free disks memory when value_array_to_disks() failed.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_plugin_ipc.cpp | 1 +
1 file changed, 1 insertion(+)

diff --git a/c_binding/lsm_plugin_ipc.cpp b/c_binding/lsm_plugin_ipc.cpp
index 890e1cb..575f02d 100644
--- a/c_binding/lsm_plugin_ipc.cpp
+++ b/c_binding/lsm_plugin_ipc.cpp
@@ -2314,6 +2314,7 @@ static int handle_volume_raid_create(lsm_plugin_ptr p, Value & params,
uint32_t disk_count = 0;
rc = value_array_to_disks(v_disks, &disks, &disk_count);
if (LSM_ERR_OK != rc) {
+ lsm_disk_record_array_free(disks, disk_count);
return rc;
}

[Gris Ge]

Changes ONTAP, MegaRAID, SMI-S plugins to follow this definition:

* When RAID degraded, the Pool.status should be:
Pool.STATUS_OK | Pool.STATUS_DEGRADED

* When RAID is rebuilding:
Pool.STATUS_RECONSTRUCTING | Pool.STATUS_DEGRADED | Pool.STATUS_OK

* When RAID is verifying data:
Pool.STATUS_OK | Pool.STATUS_VERIFYING

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/megaraid/megaraid.py | 7 +++----
plugin/ontap/ontap.py | 16 +++++++++-------
plugin/smispy/smis_pool.py | 2 +-
3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/plugin/megaraid/megaraid.py b/plugin/megaraid/megaraid.py
index 775612a..7fd2760 100644
--- a/plugin/megaraid/megaraid.py
+++ b/plugin/megaraid/megaraid.py
@@ -140,12 +140,11 @@ def _mega_size_to_lsm(mega_size):

_POOL_STATUS_MAP = {
'Onln': Pool.STATUS_OK,
- 'Dgrd': Pool.STATUS_DEGRADED,
- 'Pdgd': Pool.STATUS_DEGRADED,
+ 'Dgrd': Pool.STATUS_DEGRADED | Pool.STATUS_OK,
+ 'Pdgd': Pool.STATUS_DEGRADED | Pool.STATUS_OK,
'Offln': Pool.STATUS_ERROR,
- 'Rbld': Pool.STATUS_RECONSTRUCTING,
+ 'Rbld': Pool.STATUS_RECONSTRUCTING | Pool.STATUS_DEGRADED | Pool.STATUS_OK,
'Optl': Pool.STATUS_OK,
- # TODO(Gris Ge): The 'Optl' is undocumented, check with LSI.
}


diff --git a/plugin/ontap/ontap.py b/plugin/ontap/ontap.py
index d13490d..f6f309d 100644
--- a/plugin/ontap/ontap.py
+++ b/plugin/ontap/ontap.py
@@ -328,18 +328,20 @@ def volumes(self, search_key=None, search_value=None, flags=0):
# https://library.netapp.com/ecmdocs/ECMP1196890/html/man1/na_aggr.1.html
_AGGR_RAID_STATUS_CONV = {
'normal': Pool.STATUS_OK,
- 'verifying': Pool.STATUS_VERIFYING,
+ 'verifying': Pool.STATUS_OK | Pool.STATUS_VERIFYING,
'copying': Pool.STATUS_INITIALIZING,
- 'ironing': Pool.STATUS_VERIFYING,
- 'resyncing': Pool.STATUS_RECONSTRUCTING,
- 'mirror degraded': Pool.STATUS_DEGRADED,
+ 'ironing': Pool.STATUS_OK | Pool.STATUS_VERIFYING,
+ 'resyncing': Pool.STATUS_OK | Pool.STATUS_DEGRADED |
+ Pool.STATUS_RECONSTRUCTING,
+ 'mirror degraded': Pool.STATUS_OK | Pool.STATUS_DEGRADED,
'needs check': Pool.STATUS_ERROR,
'initializing': Pool.STATUS_INITIALIZING,
- 'growing': Pool.STATUS_GROWING,
+ 'growing': Pool.STATUS_OK | Pool.STATUS_GROWING,
'partial': Pool.STATUS_ERROR,
'noparity': Pool.STATUS_OTHER,
- 'degraded': Pool.STATUS_DEGRADED,
- 'reconstruct': Pool.STATUS_RECONSTRUCTING,
+ 'degraded': Pool.STATUS_OK | Pool.STATUS_DEGRADED,
+ 'reconstruct': Pool.STATUS_OK | Pool.STATUS_DEGRADED |
+ Pool.STATUS_RECONSTRUCTING,
'out-of-date': Pool.STATUS_OTHER,
'foreign': Pool.STATUS_OTHER,
}
diff --git a/plugin/smispy/smis_pool.py b/plugin/smispy/smis_pool.py
index cb06867..d7b599d 100644
--- a/plugin/smispy/smis_pool.py
+++ b/plugin/smispy/smis_pool.py
@@ -186,7 +186,7 @@ def _pool_element_type(smis_common, cim_pool):
_LSM_POOL_OP_STATUS_CONV = {
dmtf.OP_STATUS_OK: Pool.STATUS_OK,
dmtf.OP_STATUS_ERROR: Pool.STATUS_ERROR,
- dmtf.OP_STATUS_DEGRADED: Pool.STATUS_DEGRADED,
+ dmtf.OP_STATUS_DEGRADED: Pool.STATUS_OK | Pool.STATUS_DEGRADED,
dmtf.OP_STATUS_NON_RECOVERABLE_ERROR: Pool.STATUS_ERROR,
dmtf.OP_STATUS_SUPPORTING_ENTITY_IN_ERROR: Pool.STATUS_ERROR,
}