[Libstoragemgmt-devel] RFC: C API Consistency review

[Tony Asleson]

Once we get the pool create/delete functionality implemented in C I
would like to propose we start the process where only new features are
added, nothing removed or modified as expected for a stable C API.

Typically this is how things have been going, but the library does have
some accumulated variation.

This is the current public C function names and possible changes I'm
suggesting we could/should make before we cast the API in stone.

lsmAccessGroupAddInitiator -> lsmAccessGroupInitiatorAdd
lsmAccessGroupCreate
lsmAccessGroupDel
lsmAccessGroupDelInitiator -> lsmAccessGroupInitiatorDel
lsmAccessGroupGrant
lsmAccessGroupIdGet
lsmAccessGroupInitiatorIdGet
lsmAccessGroupInitiatorIdSet
lsmAccessGroupList
lsmAccessGroupNameGet
lsmAccessGroupRecordAlloc
lsmAccessGroupRecordAllocArray
lsmAccessGroupRecordCopy
lsmAccessGroupRecordFree
lsmAccessGroupRecordFreeArray
lsmAccessGroupRevoke
lsmAccessGroupsGrantedToVolume
lsmAccessGroupSystemIdGet
lsmBlockRangeBlockCountGet
lsmBlockRangeDestStartGet
lsmBlockRangeRecordAlloc
lsmBlockRangeRecordAllocArray
lsmBlockRangeRecordCopy
lsmBlockRangeRecordFree
lsmBlockRangeRecordFreeArray
lsmBlockRangeSourceStartGet
lsmCapabilities
lsmCapabilityGet
lsmCapabilityRecordAlloc
lsmCapabilityRecordFree
lsmCapabilitySet
lsmCapabilitySetN
lsmConnectClose
lsmConnectGetTimeout -> lsmConnectTimeoutGet
lsmConnectPassword
lsmConnectSetTimeout -> lsmConnectTimeoutSet
lsmDataTypeCopy
lsmDiskBlockSizeGet
lsmDiskIdGet
lsmDiskList
lsmDiskNameGet
lsmDiskNumberOfBlocksGet
lsmDiskOptionalDataGet
lsmDiskRecordAlloc
lsmDiskRecordAllocArray
lsmDiskRecordCopy
lsmDiskRecordFree
lsmDiskRecordFreeArray
lsmDiskStatusGet
lsmDiskSystemIdGet
lsmDiskTypeGet
lsmErrorCreate
lsmErrorFree
lsmErrorGetDebug -> lsmErrorDebugGet
lsmErrorGetDebugData -> lsmErrorDebugDataGet
lsmErrorGetDomain -> lsmErrorDomainGet
lsmErrorGetException -> lsmErrorExceptionGet
lsmErrorGetLast -> lsmErrorLastGet
lsmErrorGetLevel -> lsmErrorLevelGet
lsmErrorGetMessage -> lsmErrorMessageGet
lsmErrorGetNumber -> lsmErrorNumberGet
lsmFsChildDependency
lsmFsChildDependencyRm
lsmFsClone
lsmFsCreate
lsmFsDelete
lsmFsFileClone
lsmFsFreeSpaceGet
lsmFsIdGet
lsmFsList
lsmFsNameGet
lsmFsPoolIdGet
lsmFsRecordAlloc
lsmFsRecordAllocArray
lsmFsRecordCopy
lsmFsRecordFree
lsmFsRecordFreeArray
lsmFsResize
lsmFsSsCreate
lsmFsSsDelete
lsmFsSsList
lsmFsSsRevert
lsmFsSystemIdGet
lsmFsTotalSpaceGet
lsmGetAvailablePlugins -> lsmAvailablePluginsList
lsmGetPrivateData -> lsmPluginPrivateDataGet, may want Set later
lsmInitiatorGrant
lsmInitiatorIdGet
lsmInitiatorList
lsmInitiatorNameGet
lsmInitiatorRecordAlloc
lsmInitiatorRecordAllocArray
lsmInitiatorRecordCopy
lsmInitiatorRecordFree
lsmInitiatorRecordFreeArray
lsmInitiatorRevoke
lsmInitiatorsGrantedToVolume
lsmInitiatorTypeGet
lsmISCSIChapAuth
lsmJobFree
lsmJobStatusFsGet
lsmJobStatusGet
lsmJobStatusSsGet
lsmJobStatusVolumeGet
lsmLogErrorBasic
lsmNfsExportAnonGidGet
lsmNfsExportAnonGidSet
lsmNfsExportAnonUidGet
lsmNfsExportAnonUidSet
lsmNfsExportAuthTypeGet
lsmNfsExportAuthTypeSet
lsmNfsExportExportPathGet
lsmNfsExportExportPathSet
lsmNfsExportFs
lsmNfsExportFsIdGet
lsmNfsExportFsIdSet
lsmNfsExportIdGet
lsmNfsExportIdSet
lsmNfsExportOptionsGet
lsmNfsExportOptionsSet
lsmNfsExportReadOnlyGet
lsmNfsExportReadOnlySet
lsmNfsExportReadWriteGet
lsmNfsExportReadWriteSet
lsmNfsExportRecordAlloc
lsmNfsExportRecordAllocArray
lsmNfsExportRecordCopy
lsmNfsExportRecordFree
lsmNfsExportRecordFreeArray
lsmNfsExportRemove
lsmNfsExportRootGet
lsmNfsExportRootSet
lsmNfsList
lsmOptionalDataListGet
lsmOptionalDataRecordAlloc
lsmOptionalDataRecordCopy
lsmOptionalDataRecordFree
lsmOptionalDataStringGet
lsmOptionalDataStringSet
lsmPluginErrorLog
lsmPluginGetInfo -> lsmPluginInfo or lsmPluginInfoGet
lsmPluginInitV1
lsmPoolFreeSpaceGet
lsmPoolFreeSpaceSet
lsmPoolIdGet
lsmPoolList
lsmPoolNameGet
lsmPoolRecordAlloc
lsmPoolRecordAllocArray
lsmPoolRecordCopy
lsmPoolRecordFree
lsmPoolRecordFreeArray
lsmPoolStatusGet
lsmPoolSystemIdGet
lsmPoolTotalSpaceGet
lsmRegisterPluginV1
lsmSsIdGet
lsmSsNameGet
lsmSsRecordAlloc
lsmSsRecordAllocArray
lsmSsRecordCopy
lsmSsRecordFree
lsmSsRecordFreeArray
lsmSsTimeStampGet
lsmStringListAlloc
lsmStringListAppend
lsmStringListCopy
lsmStringListFree
lsmStringListGetElem -> lsmStringListElemGet
lsmStringListRemove
lsmStringListSetElem -> lsmStringListElemSet
lsmStringListSize
lsmSystemIdGet
lsmSystemList
lsmSystemNameGet
lsmSystemRecordAlloc
lsmSystemRecordAllocArray
lsmSystemRecordCopy
lsmSystemRecordFree
lsmSystemRecordFreeArray
lsmSystemStatusGet
lsmVolumeBlockSizeGet
lsmVolumeChildDependency
lsmVolumeChildDependencyRm
lsmVolumeCreate
lsmVolumeDelete
lsmVolumeIdGet
lsmVolumeList
lsmVolumeNameGet
lsmVolumeNumberOfBlocksGet
lsmVolumeOffline
lsmVolumeOnline
lsmVolumeOpStatusGet
lsmVolumePoolIdGet
lsmVolumeRecordAlloc
lsmVolumeRecordAllocArray
lsmVolumeRecordCopy
lsmVolumeRecordFree
lsmVolumeRecordFreeArray
lsmVolumeReplicate
lsmVolumeReplicateRange
lsmVolumeReplicateRangeBlockSize
lsmVolumeResize
lsmVolumesAccessibleByAccessGroup
lsmVolumesAccessibleByInitiator
lsmVolumeSystemIdGet
lsmVolumeVpd83Get

The other thing I noticed is that some free functions return a value and
some don't. Usually this is because a magic check fails that we would
return an error condition. I'm suggesting that we make them all
consistent by always returning a value.

The python API is needing a review too.

Comments/suggestions?

Regards,
Tony

[MaShimiao]

I think it's a good suggestion and a good change.
A unified style of code makes code easy to read and understand.
It's enjoyable to read unified code.
Besides, I think other function name of type arrays also can be changed.
Comments in below.

Post by Tony Asleson
Once we get the pool create/delete functionality implemented in C I
would like to propose we start the process where only new features are
added, nothing removed or modified as expected for a stable C API.
Typically this is how things have been going, but the library does have
some accumulated variation.
This is the current public C function names and possible changes I'm
suggesting we could/should make before we cast the API in stone.
lsmAccessGroupAddInitiator -> lsmAccessGroupInitiatorAdd
lsmAccessGroupCreate
lsmAccessGroupDel
lsmAccessGroupDelInitiator -> lsmAccessGroupInitiatorDel
lsmAccessGroupGrant
lsmAccessGroupIdGet
lsmAccessGroupInitiatorIdGet
lsmAccessGroupInitiatorIdSet
lsmAccessGroupList
lsmAccessGroupNameGet
lsmAccessGroupRecordAlloc
lsmAccessGroupRecordAllocArray

lsmAccessGroupRecordAllocArray -> lsmAccessGroupRecordArrayAlloc

Post by Tony Asleson
lsmAccessGroupRecordCopy
lsmAccessGroupRecordFree
lsmAccessGroupRecordFreeArray

lsmAccessGroupRecordFreeArray -> lsmAccessGroupRecordArrayFree

Post by Tony Asleson
lsmAccessGroupRevoke
lsmAccessGroupsGrantedToVolume
lsmAccessGroupSystemIdGet
lsmBlockRangeBlockCountGet
lsmBlockRangeDestStartGet
lsmBlockRangeRecordAlloc
lsmBlockRangeRecordAllocArray

lsmBlockRangeRecordAllocArray -> lsmBlockRangeRecordArrayAlloc

Post by Tony Asleson
lsmBlockRangeRecordCopy
lsmBlockRangeRecordFree
lsmBlockRangeRecordFreeArray

lsmBlockRangeRecordFreeArray -> lsmBlockRangeRecordArrayFree

Post by Tony Asleson
lsmBlockRangeSourceStartGet
lsmCapabilities

lsmCapabilities -> how about 'lsmCapabilityList'?

Post by Tony Asleson
lsmCapabilityGet
lsmCapabilityRecordAlloc
lsmCapabilityRecordFree
lsmCapabilitySet
lsmCapabilitySetN
lsmConnectClose
lsmConnectGetTimeout -> lsmConnectTimeoutGet
lsmConnectPassword
lsmConnectSetTimeout -> lsmConnectTimeoutSet
lsmDataTypeCopy
lsmDiskBlockSizeGet
lsmDiskIdGet
lsmDiskList
lsmDiskNameGet
lsmDiskNumberOfBlocksGet
lsmDiskOptionalDataGet
lsmDiskRecordAlloc
lsmDiskRecordAllocArray

lsmDiskRecordAllocArray -> lsmDiskRecordArrayAlloc

Post by Tony Asleson
lsmDiskRecordCopy
lsmDiskRecordFree
lsmDiskRecordFreeArray

lsmDiskRecordFreeArray -> lsmDiskRecordArrayFree

Post by Tony Asleson
lsmDiskStatusGet
lsmDiskSystemIdGet
lsmDiskTypeGet
lsmErrorCreate
lsmErrorFree
lsmErrorGetDebug -> lsmErrorDebugGet
lsmErrorGetDebugData -> lsmErrorDebugDataGet
lsmErrorGetDomain -> lsmErrorDomainGet
lsmErrorGetException -> lsmErrorExceptionGet
lsmErrorGetLast -> lsmErrorLastGet
lsmErrorGetLevel -> lsmErrorLevelGet
lsmErrorGetMessage -> lsmErrorMessageGet
lsmErrorGetNumber -> lsmErrorNumberGet
lsmFsChildDependency
lsmFsChildDependencyRm
lsmFsClone
lsmFsCreate
lsmFsDelete
lsmFsFileClone
lsmFsFreeSpaceGet
lsmFsIdGet
lsmFsList
lsmFsNameGet
lsmFsPoolIdGet
lsmFsRecordAlloc
lsmFsRecordAllocArray

lsmFsRecordAllocArray -> lsmFsRecordArrayAlloc

Post by Tony Asleson
lsmFsRecordCopy
lsmFsRecordFree
lsmFsRecordFreeArray

lsmFsRecordFreeArray -> lsmFsRecordArrayFree

Post by Tony Asleson
lsmFsResize
lsmFsSsCreate
lsmFsSsDelete
lsmFsSsList
lsmFsSsRevert
lsmFsSystemIdGet
lsmFsTotalSpaceGet
lsmGetAvailablePlugins -> lsmAvailablePluginsList
lsmGetPrivateData -> lsmPluginPrivateDataGet, may want Set later
lsmInitiatorGrant
lsmInitiatorIdGet
lsmInitiatorList
lsmInitiatorNameGet
lsmInitiatorRecordAlloc
lsmInitiatorRecordAllocArray

lsmInitiatorRecordAllocArray -> lsmInitiatorRecordArrayAlloc

Post by Tony Asleson
lsmInitiatorRecordCopy
lsmInitiatorRecordFree
lsmInitiatorRecordFreeArray

lsmInitiatorRecordFreeArray -> lsmInitiatorRecordArrayFree

Post by Tony Asleson
lsmInitiatorRevoke
lsmInitiatorsGrantedToVolume
lsmInitiatorTypeGet
lsmISCSIChapAuth
lsmJobFree
lsmJobStatusFsGet
lsmJobStatusGet
lsmJobStatusSsGet
lsmJobStatusVolumeGet
lsmLogErrorBasic
lsmNfsExportAnonGidGet
lsmNfsExportAnonGidSet
lsmNfsExportAnonUidGet
lsmNfsExportAnonUidSet
lsmNfsExportAuthTypeGet
lsmNfsExportAuthTypeSet
lsmNfsExportExportPathGet
lsmNfsExportExportPathSet
lsmNfsExportFs
lsmNfsExportFsIdGet
lsmNfsExportFsIdSet
lsmNfsExportIdGet
lsmNfsExportIdSet
lsmNfsExportOptionsGet
lsmNfsExportOptionsSet
lsmNfsExportReadOnlyGet
lsmNfsExportReadOnlySet
lsmNfsExportReadWriteGet
lsmNfsExportReadWriteSet
lsmNfsExportRecordAlloc
lsmNfsExportRecordAllocArray

lsmNfsExportRecordAllocArray -> lsmNfsExportRecordArrayAlloc

Post by Tony Asleson
lsmNfsExportRecordCopy
lsmNfsExportRecordFree
lsmNfsExportRecordFreeArray

lsmNfsExportRecordFreeArray -> lsmNfsExportRecordArrayFree

Post by Tony Asleson
lsmNfsExportRemove
lsmNfsExportRootGet
lsmNfsExportRootSet
lsmNfsList
lsmOptionalDataListGet
lsmOptionalDataRecordAlloc
lsmOptionalDataRecordCopy
lsmOptionalDataRecordFree
lsmOptionalDataStringGet
lsmOptionalDataStringSet
lsmPluginErrorLog
lsmPluginGetInfo -> lsmPluginInfo or lsmPluginInfoGet
lsmPluginInitV1
lsmPoolFreeSpaceGet
lsmPoolFreeSpaceSet
lsmPoolIdGet
lsmPoolList
lsmPoolNameGet
lsmPoolRecordAlloc
lsmPoolRecordAllocArray

lsmPoolRecordAllocArray -> lsmPoolRecordArrayAlloc

Post by Tony Asleson
lsmPoolRecordCopy
lsmPoolRecordFree
lsmPoolRecordFreeArray

lsmPoolRecordFreeArray -> lsmPoolRecordArrayFree

Post by Tony Asleson
lsmPoolStatusGet
lsmPoolSystemIdGet
lsmPoolTotalSpaceGet
lsmRegisterPluginV1
lsmSsIdGet
lsmSsNameGet
lsmSsRecordAlloc
lsmSsRecordAllocArray

lsmSsRecordAllocArray -> lsmSsRecordArrayAlloc

Post by Tony Asleson
lsmSsRecordCopy
lsmSsRecordFree
lsmSsRecordFreeArray

lsmSsRecordFreeArray -> lsmSsRecordArrayFree

Post by Tony Asleson
lsmSsTimeStampGet
lsmStringListAlloc
lsmStringListAppend
lsmStringListCopy
lsmStringListFree
lsmStringListGetElem -> lsmStringListElemGet
lsmStringListRemove
lsmStringListSetElem -> lsmStringListElemSet
lsmStringListSize
lsmSystemIdGet
lsmSystemList
lsmSystemNameGet
lsmSystemRecordAlloc
lsmSystemRecordAllocArray

lsmSystemRecordAllocArray -> lsmSystemRecordArrayAlloc

Post by Tony Asleson
lsmSystemRecordCopy
lsmSystemRecordFree
lsmSystemRecordFreeArray

lsmSystemRecordFreeArray -> lsmSystemRecordArrayFree

Post by Tony Asleson
lsmSystemStatusGet
lsmVolumeBlockSizeGet
lsmVolumeChildDependency
lsmVolumeChildDependencyRm
lsmVolumeCreate
lsmVolumeDelete
lsmVolumeIdGet
lsmVolumeList
lsmVolumeNameGet
lsmVolumeNumberOfBlocksGet
lsmVolumeOffline
lsmVolumeOnline
lsmVolumeOpStatusGet
lsmVolumePoolIdGet
lsmVolumeRecordAlloc
lsmVolumeRecordAllocArray

lsmVolumeRecordAllocArray -> lsmVolumeRecordArrayAlloc

Post by Tony Asleson
lsmVolumeRecordCopy
lsmVolumeRecordFree
lsmVolumeRecordFreeArray

lsmVolumeRecordFreeArray -> lsmVolumeRecordArrayFree

Post by Tony Asleson
lsmVolumeReplicate
lsmVolumeReplicateRange
lsmVolumeReplicateRangeBlockSize
lsmVolumeResize
lsmVolumesAccessibleByAccessGroup
lsmVolumesAccessibleByInitiator
lsmVolumeSystemIdGet
lsmVolumeVpd83Get
The other thing I noticed is that some free functions return a value and
some don't. Usually this is because a magic check fails that we would
return an error condition. I'm suggesting that we make them all
consistent by always returning a value.
The python API is needing a review too.
Comments/suggestions?
Regards,
Tony
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Libstoragemgmt-devel mailing list
https://lists.sourceforge.net/lists/listinfo/libstoragemgmt-devel

[Tony Asleson]

All the array ones look good, keeps with the 'verb' being at the end
[get|set|del] etc.

The capabilities one I'm still thinking should be left as is. The list
ones corresponds to zero or more of something being returned, the
capabilities is a get/set by key interface.

I'm also thinking that maybe the functions that do return an array
should be named differently? The 'list' changed to 'array'?

Moving forward we will be adding an interface where we can incrementally
retrieve results without having to keep them all in memory (cursor).
This is a fairly new feature in SMI-S.

Thanks,
Tony

Post by MaShimiao
I think it's a good suggestion and a good change.
A unified style of code makes code easy to read and understand.
It's enjoyable to read unified code.
Besides, I think other function name of type arrays also can be changed.
Comments in below.
lsmAccessGroupRecordAllocArray -> lsmAccessGroupRecordArrayAlloc
lsmAccessGroupRecordFreeArray -> lsmAccessGroupRecordArrayFree
lsmBlockRangeRecordAllocArray -> lsmBlockRangeRecordArrayAlloc
lsmBlockRangeRecordFreeArray -> lsmBlockRangeRecordArrayFree
lsmCapabilities -> how about 'lsmCapabilityList'?
lsmDiskRecordAllocArray -> lsmDiskRecordArrayAlloc
lsmDiskRecordFreeArray -> lsmDiskRecordArrayFree
lsmFsRecordAllocArray -> lsmFsRecordArrayAlloc
lsmFsRecordFreeArray -> lsmFsRecordArrayFree
lsmInitiatorRecordAllocArray -> lsmInitiatorRecordArrayAlloc
lsmInitiatorRecordFreeArray -> lsmInitiatorRecordArrayFree
lsmNfsExportRecordAllocArray -> lsmNfsExportRecordArrayAlloc
lsmNfsExportRecordFreeArray -> lsmNfsExportRecordArrayFree
lsmPoolRecordAllocArray -> lsmPoolRecordArrayAlloc
lsmPoolRecordFreeArray -> lsmPoolRecordArrayFree
lsmSsRecordAllocArray -> lsmSsRecordArrayAlloc
lsmSsRecordFreeArray -> lsmSsRecordArrayFree
lsmSystemRecordAllocArray -> lsmSystemRecordArrayAlloc
lsmSystemRecordFreeArray -> lsmSystemRecordArrayFree
lsmVolumeRecordAllocArray -> lsmVolumeRecordArrayAlloc
lsmVolumeRecordFreeArray -> lsmVolumeRecordArrayFree

[Gris Ge]

Post by Tony Asleson
This is the current public C function names and possible changes I'm
suggesting we could/should make before we cast the API in stone.
Comments/suggestions?

Hi Tony,
Just some quick thoughts:

1. We have 'Del', 'Delete' and 'Remove':
lsmAccessGroupDel
lsmNfsExportRemove
lsmVolumeDelete

2. Can we merge lsmAccessGroupGrant() and lsmInitiatorGrant()?
For those initiator mapping only plugin/array, we can make every
initiator as one group.

3. Any possible to make that list shorter? Like merge query method of
each class?

Post by Tony Asleson
Regards,
Tony

Best regards.

[Tony Asleson]

Post by Tony Asleson
lsmAccessGroupDel
lsmNfsExportRemove
lsmVolumeDelete

Good point, 'Delete' and 'Del' should be the same, will change to Del.
However, I choose 'remove' to denote that you are changing something
that doesn't destroy it like delete does, but maybe this subtle
difference isn't needed?

Post by Tony Asleson
2. Can we merge lsmAccessGroupGrant() and lsmInitiatorGrant()?
For those initiator mapping only plugin/array, we can make every
initiator as one group.

This was done for arrays that don't support access groups. For arrays
that support access groups they don't need to provide the single grant
interface by creating an access group for each initiator, we have
capability flags for that.

Post by Tony Asleson
3. Any possible to make that list shorter? Like merge query method of
each class?

For C you can't have the same function return different things without
tossing type safety out, which isn't recommended. We could build our
own variant types, but then we would catch type errors at run-time
instead of compile time, which isn't recommended.

Regards,
Tony