[Libstoragemgmt-devel] [RFC] Python API review: lsm.System, lsm.Disk

[Gris Ge]

Hi Guys,

I have finished the draft on python API document for lsm.System and
lsm.Disk:
https://sourceforge.net/p/libstoragemgmt/wiki/Python_API_Usage/

Any feedback will be appreciated, especially about API design and document
layout.

I also noted down changes between document and current code:

## System
* If we cannot explain all possible combination of bit array map for
System.status, I would suggest we move to constant value.
Checked disk, pool and volume classes, no need to maintain a bit map.
Vendor implementation on system status:
* SMI-S plugin:
SNIA SMI-S OperationalStatus just use bit array to
identify the severe level of error and treat stress and
degraded as OK.
(1.6rev4 Common book, Section 25, PDF Page 25)
* ONTAP plugin:
ONTAP API (8.2) does not support status in 7-mode.
system-node-get() of cluster-mode has temprature, fan, battery
status. We can use them to determine the system status in the future.
* targetd plugin:
No support.
When system having multiple issue need different state string, we set to
the worst state. Example, system is having these errors:
* 2 Disks offline in on RAID 5 pool. -- STATUS_ERROR
* A battery is end of life. -- STATUS_DEGRADED
* A battery is near end of life. -- STATUS_PREDICTIVE_FAILURE
In this case, we set status to the worst state: STATUS_ERROR. Check wiki
page for server level of these status.

* Changed method:
lsm.Client.systems(system_id=None)
# Supporting querying system with given ID. Return a array/list with
# given system object only.

* Do we really need 'flags' as reserved parameter for future use?
I found no python project using reserving a parameter for future use.

* Added constants:
lsm.System.STATUS_UNKNOWN
lsm.System.STATUS_STRESSED
lsm.System.STATUS_STARTING
lsm.System.STATUS_STOPPING
lsm.System.STATUS_STOPPED
lsm.System.STATUS_OTHER
* Removed constants:
lsm.System.STATUS_VENDOR_SPECIFIC
# Replaced by lsm.System.STATUS_UNKNOWN or lsm.System.STATUS_OTHER
* Added property:
lsm.System.status_info
# Free form string. For these uses:
# 1. Store detail error message when
# lsm.System.STATUS_ERROR or lsm.System.STATUS_DEGRADED or
# lsm.System.STATUS_PREDICTIVE_FAILURE or
# lsm.System.STATUS_STRESSED
# 2. Store vendor specific status information when lsm.System.STATUS_OTHER

## Disk
* Removed constants:
lsm.Disk.DISK_TYPE_NOT_APPLICABLE
# No real use
lsm.Disk.STATUS_RECONSTRUCTING
# Reconstruction should be status of pool
lsm.Disk.RETRIEVE_FULL_INFO
# lsm.Client.disks(disk_id=None, opt_data=False) does not this constant.
* Added constants:
lsm.Disk.DISK_TYPE_LUN
# Indicate array is treating remote LUN as its disks to assemble Pool.
* 'num_of_blocks' and 'size_bytes', does C or json hold the integer as large
as python do -- sys.maxint(9223372036854775807)?
* Change 'size_bytes' to defined not calculate on runtime. The definition of
'size_bytes' is usable size. Some array(like EMC CX/VNX) support DIF disk
which sector size is 520, but only 512 is usable. Using 'block size'
multiple count of block is incorrect.
* Move 'status_info' from optional property to mandatory property.
* Removed optional property:
owner_ctrler_id
# No controller support yet.

[Tony Asleson]

Post by Gris Ge
Hi Guys,
I have finished the draft on python API document for lsm.System and
https://sourceforge.net/p/libstoragemgmt/wiki/Python_API_Usage/
Any feedback will be appreciated, especially about API design and document
layout.

Document looks good to me. I would still rather have it auto generated
by doxygen from the source so that we don't have maintain two different
things in different spots. I will see what I can do to improve the output.

Post by Gris Ge
## System
* If we cannot explain all possible combination of bit array map for
System.status, I would suggest we move to constant value.

IMHO the benefit of the bit field is that is can express just one status
if that is all that is needed or express a number of different states at
the same time. Sure some combination's don't make sense, but I believe
it is better to have flexibility in what we can convey.

What are others opinions on this?

Post by Gris Ge
Checked disk, pool and volume classes, no need to maintain a bit map.
SNIA SMI-S OperationalStatus just use bit array to
identify the severe level of error and treat stress and
degraded as OK.
(1.6rev4 Common book, Section 25, PDF Page 25)
ONTAP API (8.2) does not support status in 7-mode.
system-node-get() of cluster-mode has temprature, fan, battery
status. We can use them to determine the system status in the future.
No support.
When system having multiple issue need different state string, we set to
* 2 Disks offline in on RAID 5 pool. -- STATUS_ERROR
* A battery is end of life. -- STATUS_DEGRADED
* A battery is near end of life. -- STATUS_PREDICTIVE_FAILURE
In this case, we set status to the worst state: STATUS_ERROR. Check wiki
page for server level of these status.
lsm.Client.systems(system_id=None)
# Supporting querying system with given ID. Return a array/list with
# given system object only.
* Do we really need 'flags' as reserved parameter for future use?
I found no python project using reserving a parameter for future use.

The flags parameter was suggested by libvirt. It allows for small
changes that we haven't accounted for in the API, future proof so to
speak. Just because we don't use today, doesn't mean we won't have a
need in the future. Without the flag we will be forced to create a new
method for each small change.

Post by Gris Ge
lsm.System.STATUS_UNKNOWN
lsm.System.STATUS_STRESSED
lsm.System.STATUS_STARTING
lsm.System.STATUS_STOPPING
lsm.System.STATUS_STOPPED
lsm.System.STATUS_OTHER
lsm.System.STATUS_VENDOR_SPECIFIC
# Replaced by lsm.System.STATUS_UNKNOWN or lsm.System.STATUS_OTHER
lsm.System.status_info
# 1. Store detail error message when
# lsm.System.STATUS_ERROR or lsm.System.STATUS_DEGRADED or
# lsm.System.STATUS_PREDICTIVE_FAILURE or
# lsm.System.STATUS_STRESSED
# 2. Store vendor specific status information when lsm.System.STATUS_OTHER

As you create patches to add/remove constants, please make the changes
to the C header files too. Historically, we haven't done a good job of
keeping them the same.

Post by Gris Ge
## Disk
lsm.Disk.DISK_TYPE_NOT_APPLICABLE
# No real use
lsm.Disk.STATUS_RECONSTRUCTING
# Reconstruction should be status of pool
lsm.Disk.RETRIEVE_FULL_INFO
# lsm.Client.disks(disk_id=None, opt_data=False) does not this constant.
lsm.Disk.DISK_TYPE_LUN
# Indicate array is treating remote LUN as its disks to assemble Pool.
* 'num_of_blocks' and 'size_bytes', does C or json hold the integer as large
as python do -- sys.maxint(9223372036854775807)?

Technically python can handle integers of any size, but it would use the
'long' type not 'int' for Python vers. < 3.

For python >= 3 "PEP 0237 Essentially, long renamed to int. That is,
there is only one built-in integral type, named int; but it behaves
mostly like the old long type."

Biggest value for C would be 18446744073709551615 (2**64-1). Looks like
the biggest 'int' for python is 2**63-1 so we need to be using 'long'
for python < 3.

Post by Gris Ge
* Change 'size_bytes' to defined not calculate on runtime. The definition of
'size_bytes' is usable size. Some array(like EMC CX/VNX) support DIF disk
which sector size is 520, but only 512 is usable. Using 'block size'
multiple count of block is incorrect.

This all depends on the definition of what we are trying to convey.
Perhaps we should be more specific and report physical block size and/or
logical block size? We can remove the computed size in bytes. Can we
retrieve this value (size_bytes) from the array?

http://people.redhat.com/msnitzer/docs/linux-advanced-storage-6.0.pdf

Post by Gris Ge
* Move 'status_info' from optional property to mandatory property.

Can we get this information for all arrays?

Post by Gris Ge
owner_ctrler_id
# No controller support yet.

Regards,
Tony

[Tony Asleson]

Post by Tony Asleson

Post by Gris Ge
# Indicate array is treating remote LUN as its disks to assemble Pool.
* 'num_of_blocks' and 'size_bytes', does C or json hold the integer as large
as python do -- sys.maxint(9223372036854775807)?

Technically python can handle integers of any size, but it would use the
'long' type not 'int' for Python vers. < 3.
For python >= 3 "PEP 0237 Essentially, long renamed to int. That is,
there is only one built-in integral type, named int; but it behaves
mostly like the old long type."
Biggest value for C would be 18446744073709551615 (2**64-1). Looks like
the biggest 'int' for python is 2**63-1 so we need to be using 'long'
for python < 3.

Just to clarify, the C API max is an unsigned 64bit value, python is
using signed 64 bit without the ability to specify unsigned thus the
reason to use 'long' instead of 'int' for python ver < 3.

Regards,
Tony

[Gris Ge]

Post by Tony Asleson
Document looks good to me. I would still rather have it auto generated
by doxygen from the source so that we don't have maintain two different
things in different spots. I will see what I can do to improve the output.

When choosing between duplicate work and better documenting, I choose
better documenting even that slow thing down.
It could be much better if you could find a good way to make doxygen or
whatever generating good documents.

Post by Tony Asleson
The flags parameter was suggested by libvirt. It allows for small
changes that we haven't accounted for in the API, future proof so to
speak. Just because we don't use today, doesn't mean we won't have a
need in the future. Without the flag we will be forced to create a new
method for each small change.

I just don't understand how the 'small change' might look like.

Example:
API 1.0 definition: systems(flags=0)
User call:
Method A: systems(flags=0)
Method B: systems()
Method C: systems(0)
Small changes:
X: When flags == SOME_NUMBER, do something extra.
Y: Change method to systems(filter=0)
Z: Change method to systems(filter, flags=0)

When using change X, method A, B, and C are all pass.
When using change Y, method A, B fail.
When using change Z, method A, B fail. C will act in unexpected way.

If you are referring 'small change' as change X, reserve a parameter
only take integer with name 'flags' seems provides pretty limit change
zone for us.

Meanwhile, libvirt only reserve 'flags' for 157 methods[1] out of 297
methods[2].
libvirt does not reserve 'flags' for these type of methods:
* Connection termination methods.
* Resource deletion methods.
* Single property query methods.
# Libvirt python API is cloning their C API for property query
# calls also.

I have no vote right on this issue giving my zero experience on python
API designing. But I do hope we know what and why we are following.

[1] grep 'flags=0):' /usr/lib64/python2.7/site-packages/libvirt.py | wc -l
[2] cat /usr/lib64/python2.7/site-packages/libvirt.py | \
perl -ne 'print "$1\n" if /(def [a-z][^ ]+)\(/' | sort -u | wc -l

Post by Tony Asleson
As you create patches to add/remove constants, please make the changes
to the C header files too. Historically, we haven't done a good job of
keeping them the same.

I will try my best to understand and maintain those C and C++ code.

Post by Tony Asleson

Post by Gris Ge
* 'num_of_blocks' and 'size_bytes', does C or json hold the integer as large
as python do -- sys.maxint(9223372036854775807)?

Biggest value for C would be 18446744073709551615 (2**64-1). Looks like
the biggest 'int' for python is 2**63-1 so we need to be using 'long'
for python < 3.

Will do.

Post by Tony Asleson

Post by Gris Ge
* Change 'size_bytes' to defined not calculate on runtime. The definition of
'size_bytes' is usable size. Some array(like EMC CX/VNX) support DIF disk
which sector size is 520, but only 512 is usable. Using 'block size'
multiple count of block is incorrect.

This all depends on the definition of what we are trying to convey.
Perhaps we should be more specific and report physical block size and/or
logical block size? We can remove the computed size in bytes. Can we
retrieve this value (size_bytes) from the array?
http://people.redhat.com/msnitzer/docs/linux-advanced-storage-6.0.pdf

These logical block size and physical block size is for exposing disk to
OS.
In LSM, disk is only used for creating pool. Sector size seems irrelevant.
For OS, they are using LSM volume, where sector size matters.

About implementation:
SMI-S (including NetApp-E):
1. RAW capacity -- CIM_StorageExtent:
NumberOfBlocks * BlockSize
2. Usable capacity -- CIM_StorageExtent:
ConsumableBlocks and BlockSize
ONTAP:
1. RAW capacity only -- disk-detail-info:
bytes-per-sector * raw-disk-sectors
Targetd:
No support.

Nstore:
I have no document yet.
Current code does not support disks.

It seems 'raw size in bytes' is the choice. The 'block size' and 'block
count' can purged or moved into optional property.

Post by Tony Asleson

Post by Gris Ge
* Move 'status_info' from optional property to mandatory property.

Can we get this information for all arrays?

SMI-S:
* No, current SMI-S does not expose SMART information to file
status_info for detail error information. Since SSD is getting
popular, I believe they will introduce SMART E9 property then.
We keep it as empty string now.
* LSI has vendor specific properties indicate health of disk:
MediaErrorCount and PredictiveFailureCount
This can indicate health detail in 'status_info'
ONTAP:
* disk-detail-info['broken-details'] and
storage-ssd-info['percent-rated-life-used'] can provide information for
'status_info'

Post by Tony Asleson
Regards,
Tony