-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/1485/#review2185
-----------------------------------------------------------
This is great job! You seem to understand my (undocumented) Storage provider design ;-).
mof/LMI_Storage-BlockStorage.mof
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1150>
This change is wrong, 'Size' is still size of the volume group, there is
nothing 'thin' in CreateOrModifyVG
mof/LMI_Storage-BlockStorage.mof
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1151>
The description should be: the volume group from which the thin pool should be
allocated.
mof/LMI_Storage-BlockStorage.mof
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1152>
I would emphasize that it's *physical* size of the thin pool, the pool will can
store most Size bytes of data.
The *logical* size is unlimited.
mof/LMI_Storage-BlockStorage.mof
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1153>
I would emphasize that it's *logical* size, it may be much higher than size we
have in underlying storage.
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1154>
This is inconsistent with MOF file:
"If this parameter is not provided, implementation will choose on
it's own when creating the device."
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1155>
This may return None if there is no pool for the device, subsequent 'if' will
throw ugly exception.
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1156>
This may return None if there is no pool for the device, subsequent 'if' will
throw ugly exception.
in addition, naming thin lv as 'pool' is confusing
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1157>
thinpool can be None if the device was destroyed when the Job was in queue.
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1158>
there is now storage.from_blivet_size() to recalculate thinlv.size to bytes
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1159>
there is now storage.to_blivet_size to recalc size to blivet units
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1160>
I understand it as '(any) InPool and (any) ElementType must not be used
together'
What about '_This_ InPool and ElementType combination is not supported'
And it might be probably CIM_ERR_INVALID_PARAMETER (please check similar error
handling)
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1161>
pywbem.CIM_ERR_INVALID_PARAMETER?
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1163>
pywbem.CIM_ERR_INVALID_PARAMETER?
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1165>
pywbem.CIM_ERR_INVALID_PARAMETER?
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1164>
pywbem.CIM_ERR_INVALID_PARAMETER?
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1162>
pool can be None if the pool is not found
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1166>
vg can be 'None' if it disappeared when the job was in queue
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1167>
use storage.to_blivet_size
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1168>
use storage.from_blivet_size
src/lmi/storage/LMI_StorageConfigurationService.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1169>
I think that goal.ThinProvisionedPoolType is _the_ property that decides, what caller
wants to create/modify.
If it is not specified -> default is used (create plain VG).
If it is specified -> check the other options (InExtents, InPool) and throw error
if there is unsupported combination (e.g. InExtents with ThinProvisionedPoolType =
ThinlyProvisionedLimitlessStoragePool)
src/lmi/storage/LMI_VGStoragePool.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1170>
use storage.to_blivet_size and from_blivet_size
src/lmi/storage/LMI_VGStoragePool.py
<
http://reviewboard-openlmi.rhcloud.com/r/1485/#comment1171>
use storage.to_blivet_size and from_blivet_size
I miss a XYZCapabilities instance for each (plain) VG, which would say that it's
possible to allocated thin pools out of it. It can be either part of LVStorageCapabilities
or separate instance of new class.
- Jan Safranek
On Jan. 23, 2014, 2:25 p.m., Jan Synacek wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/1485/
-----------------------------------------------------------
(Updated Jan. 23, 2014, 2:25 p.m.)
Review request for OpenLMI Developers.
Repository: openlmi-storage
Description
-------
Add support for thin provisioning.
Currently, only creating / deleting thin pools and thin logical volumes
is supported.
Add some basic tests as well.
Diffs
-----
mof/LMI_Storage-BlockStorage.mof c92081c7539df2409649ad42770f8f9c5e396863
mof/LMI_Storage-MethodParameters.mof cd3cacff1ad2ff12e72d81f1406ca8ed03ccb95b
src/lmi/storage/LMI_LVAllocatedFromStoragePool.py
98d05d28eb1ed377edd85e1a83fc5be03a872a5a
src/lmi/storage/LMI_LVStorageCapabilities.py 95808296456289b3ec7cbfc0a5c81211a573acc6
src/lmi/storage/LMI_LVStorageExtent.py c4f258121396807f60fe9e269a9f1ad59fd20113
src/lmi/storage/LMI_StorageConfigurationService.py
e0729cfd0c7ca573af8a8f174dafb13b57932b77
src/lmi/storage/LMI_VGStorageCapabilities.py 966e397d1f961c7ebd6cdbf8fa5707a971b05c0a
src/lmi/storage/LMI_VGStoragePool.py 6a88fdfbca502b0b687e5ec4d891e5b2dc74d0f6
test/test_create_vg.py e650b8123fe178d5a7957d6e8ec898111d63d177
Diff:
http://reviewboard-openlmi.rhcloud.com/r/1485/diff/
Testing
-------
Thanks,
Jan Synacek