This is great job! You seem to understand my (undocumented) Storage provider design ;-).
mof/LMI_Storage-BlockStorage.mof
(Diff revision 1)
|
555 |
"Size of the volume group." ), |
558 |
"Size of the thin pool. On input, only used when creating a ThinPool." ), |
This change is wrong, 'Size' is still size of the volume group, there is nothing 'thin' in CreateOrModifyVG
The description should be: the volume group from which the thin pool should be allocated.
mof/LMI_Storage-BlockStorage.mof
(Diff revision 1)
|
|
|
590 |
[IN, OUT, Description( |
|
|
591 |
"Size of the volume group. On input, only used when creating a ThinPool." ), |
|
|
592 |
Units ( "Bytes" ), |
|
|
593 |
PUnit ( "byte" )] |
|
|
594 |
uint64 Size, |
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
(Diff revision 1)
|
|
|
615 |
[IN, OUT, Description("Requested thin LV size. It will be rounded to multiples of VG's ExtentSize." |
|
|
616 |
"\n Modification is not supported."), |
|
|
617 |
Units("Bytes")] |
|
|
618 |
uint64 Size, |
I would emphasize that it's *logical* size, it may be much higher than size we have in underlying storage.
This is inconsistent with MOF file:
"If this parameter is not provided, implementation will choose on
it's own when creating the device."
This may return None if there is no pool for the device, subsequent 'if' will throw ugly exception.
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
thinpool can be None if the device was destroyed when the Job was in queue.
there is now storage.from_blivet_size() to recalculate thinlv.size to bytes
there is now storage.to_blivet_size to recalc size to blivet units
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)
pywbem.CIM_ERR_INVALID_PARAMETER?
pywbem.CIM_ERR_INVALID_PARAMETER?
pywbem.CIM_ERR_INVALID_PARAMETER?
pywbem.CIM_ERR_INVALID_PARAMETER?
pool can be None if the pool is not found
vg can be 'None' if it disappeared when the job was in queue
use storage.to_blivet_size
use storage.from_blivet_size
src/lmi/storage/LMI_StorageConfigurationService.py
(Diff revision 1)
|
def cim_method_createormodifystoragepool(self, env, object_name, |
|
|
1107 |
if pool.type == 'lvmvg': |
|
|
1108 |
if param_inextents: |
|
|
1109 |
# modify vg |
|
|
1110 |
self.cim_method_createormodifyvg(env, object_name, |
|
|
1111 |
param_elementname, param_goal, |
|
|
1112 |
param_inextents, param_pool, |
|
|
1113 |
input_arguments, method_name) |
|
|
1114 |
else: |
|
|
1115 |
# create thinpool |
|
|
1116 |
self.cim_method_createormodifythinpool(env, object_name, |
|
|
1117 |
param_elementname, param_goal, |
|
|
1118 |
param_thinpool, None, |
|
|
1119 |
param_size) |
|
|
1120 |
if pool.type == 'lvmthinpool': |
|
|
1121 |
if param_inextents: |
|
|
1122 |
raise pywbem.CIMError(pywbem.CIM_ERR_NOT_SUPPORTED, |
|
|
1123 |
"InExtents specified together with Pool .") |
|
|
1124 |
else: |
|
|
1125 |
# modify thinpool |
|
|
1126 |
self.cim_method_createormodifythinpool(env, object_name, |
|
|
1127 |
param_elementname, param_goal, |
|
|
1128 |
None, param_thinpool, |
|
|
1129 |
param_size) |
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
(Diff revision 1)
|
def get_instance(self, env, model, device=None): |
|
|
135 |
# it's important to check for LVMThinPoolDevice first, because it is a subclass of LVMVolumeGroupDevice |
|
|
136 |
if isinstance(device, blivet.devices.LVMThinPoolDevice): |
|
|
137 |
model['TotalManagedSpace'] = pywbem.Uint64(device.size * units.MEGABYTE) |
|
|
138 |
# workaround for #1034721 |
|
|
139 |
if device.freeSpace < 0: |
|
|
140 |
model['RemainingManagedSpace'] = pywbem.Uint64(0) |
|
|
141 |
else: |
|
|
142 |
model['RemainingManagedSpace'] = pywbem.Uint64(device.freeSpace * units.MEGABYTE) |
|
|
143 |
model['SpaceLimit'] = pywbem.Uint64(device.size * units.MEGABYTE) |
|
|
144 |
model['SpaceLimitDetermination'] = self.Values.SpaceLimitDetermination.Limitless |
|
|
145 |
model['ThinProvisionMetaDataSpace'] = pywbem.Uint64(device.metaDataSize * units.MEGABYTE) |
|
|
146 |
else: |
|
|
147 |
model['TotalManagedSpace'] = pywbem.Uint64(device.extents * device.peSize * units.MEGABYTE) |
|
|
148 |
model['RemainingManagedSpace'] = pywbem.Uint64(device.peFree * device.peSize * units.MEGABYTE) |
|
|
149 |
model['ExtentSize'] = pywbem.Uint64(device.peSize * units.MEGABYTE) |
|
|
150 |
model['TotalExtents'] = pywbem.Uint64(device.extents) |
|
|
151 |
model['RemainingExtents'] = pywbem.Uint64(device.peFree) |
use storage.to_blivet_size and from_blivet_size
src/lmi/storage/LMI_VGStoragePool.py
(Diff revision 1)
|
def _get_setting_for_device(self, device, setting_provider): |
281 |
setting['ExtentSize'] = device.peSize * units.MEGABYTE |
292 |
|
|
|
293 |
if isinstance(device, blivet.devices.LVMThinPoolDevice): |
|
|
294 |
setting['SpaceLimit'] = device.size * units.MEGABYTE |
|
|
295 |
setting['ThinProvisionedPoolType'] = self.Values.ThinProvisionedPoolType.ThinlyProvisionedLimitlessStoragePool |
|
|
296 |
else: |
|
|
297 |
setting['ExtentSize'] = device.peSize * units.MEGABYTE |
282 |
setting['ElementName'] = device.path |
298 |
setting['ElementName'] = device.path |
|
|
299 |
|
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 January 23rd, 2014, 2:25 p.m. CET, Jan Synacek wrote:
Review request for OpenLMI Developers.
By Jan Synacek.
Updated Jan. 23, 2014, 2:25 p.m.
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)
View Diff
|