Change in vdsm[master]: storage: Initial size for thin provisioning disk
frolland at redhat.com
frolland at redhat.com
Sun Oct 11 11:28:17 UTC 2015
Freddy Rolland has posted comments on this change.
Change subject: storage: Initial size for thin provisioning disk
......................................................................
Patch Set 6:
(15 comments)
https://gerrit.ovirt.org/#/c/46417/6//COMMIT_MSG
Commit Message:
Line 14: is a block disk with thin provisioning, the new volume needs
Line 15: a starting size as the original disk size.
Line 16:
Line 17: If the initial size is not specified, the default initial
Line 18: volume size for thin provisioning is 1G.
> is the configured volume_utilization_chunk_mb (1GiB by default).
Done
Line 19:
Line 20: Change-Id: Iaf5f142541bf0b311a77a0544a1c4ffb689a9fde
Line 21: Bug-Url: https://bugzilla.redhat.com/1221603
https://gerrit.ovirt.org/#/c/46417/6/tests/blockVolumeTests.py
File tests/blockVolumeTests.py:
Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20:
Line 21: import storage.volume as volume
> Use:
Done
Line 22: import storage.storage_exception as se
Line 23:
Line 24: from storage.blockVolume import BlockVolume
Line 25: from testlib import permutations, expandPermutations
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20:
Line 21: import storage.volume as volume
Line 22: import storage.storage_exception as se
> from storage import storage_exception as se
Done
Line 23:
Line 24: from storage.blockVolume import BlockVolume
Line 25: from testlib import permutations, expandPermutations
Line 26: from testlib import VdsmTestCase as TestCaseBase
Line 20:
Line 21: import storage.volume as volume
Line 22: import storage.storage_exception as se
Line 23:
Line 24: from storage.blockVolume import BlockVolume
> resort after changing the import above this.
Done
Line 25: from testlib import permutations, expandPermutations
Line 26: from testlib import VdsmTestCase as TestCaseBase
Line 27: from vdsm.config import config
Line 28:
Line 23:
Line 24: from storage.blockVolume import BlockVolume
Line 25: from testlib import permutations, expandPermutations
Line 26: from testlib import VdsmTestCase as TestCaseBase
Line 27: from vdsm.config import config
> Should be before the storage imports. The order is: stdlib, vdsm lib, modul
Done
Line 28:
Line 29:
Line 30: @expandPermutations
Line 31: class BlockVolumeSizeTests(TestCaseBase):
Line 45: initial_size, result):
Line 46: size = BlockVolume._calculate_volume_alloc_size(preallocate,
Line 47: capacity,
Line 48: initial_size)
Line 49: self.assertEqual(size, result)
> Did you see my comment about using *args in previous version?
Done
Line 50:
Line 51: @permutations([
Line 52: # preallocate, capacity, initial_size,
Line 53: (volume.PREALLOCATED_VOL, 2048, 5000), # initial bigger than size
Line 56: def test_fail_invalide_block_volume_size(self, preallocate,
Line 57: capacity, initial_size):
Line 58: with self.assertRaises(se.InvalidParameterException):
Line 59: BlockVolume._calculate_volume_alloc_size(preallocate, capacity,
Line 60: initial_size)
> Since you change only hte volume preallocated type, you don't need to add t
Done
https://gerrit.ovirt.org/#/c/46417/6/tests/miscTests.py
File tests/miscTests.py:
Line 583: # size, name, result
Line 584: (1000, "size", 1000),
Line 585: ("512", "size", 1),
Line 586: ("513", "size", 2),
Line 587: (None, "size", None),
> "size" is repeated for no reason. mention it once in the body of the test
Done
Line 588: ])
Line 589: def test_validate_size(self, size, name, result):
Line 590: self.assertEqual(misc.validateSize(size, name), result)
Line 591:
Line 585: ("512", "size", 1),
Line 586: ("513", "size", 2),
Line 587: (None, "size", None),
Line 588: ])
Line 589: def test_validate_size(self, size, name, result):
> I would call this test_valid_size
Done
Line 590: self.assertEqual(misc.validateSize(size, name), result)
Line 591:
Line 592: @permutations([
Line 593: # size, name
Line 593: # size, name
Line 594: (1000.14, "size"),
Line 595: ((1, 2), "size"),
Line 596: ])
Line 597: def test_fail_validate_size(self, size, name):
> I would call this test_invalid_size
Done
Line 598: self.assertRaises(misc.se.InvalidParameterException,
Line 599: misc.validateSize, size, name)
Line 600:
Line 601:
https://gerrit.ovirt.org/#/c/46417/6/vdsm/storage/blockVolume.py
File vdsm/storage/blockVolume.py:
Line 125: Class specific implementation of volumeCreate. All the exceptions are
Line 126: properly handled and logged in volume.create()
Line 127: """
Line 128:
Line 129: volSize = cls._calculate_volume_alloc_size(preallocate,
> Lets call this lvSize
Done
Line 130: size, initialSize)
Line 131:
Line 132: lvm.createLV(dom.sdUUID, volUUID, "%s" % volSize, activate=True,
Line 133: initialTag=TAG_VOL_UNINIT)
Line 165:
Line 166: @classmethod
Line 167: def _calculate_volume_alloc_size(cls, preallocate, capacity, initial_size):
Line 168: """ Calculate the allocation size in mb of the volume
Line 169: 'preallocate' - Spare or Preallocated
> Sparse
Done
Line 170: 'capacity' - the volume size in sectors
Line 171: 'initial_size' - optional, if provided the initial allocated
Line 172: size in sectors for sparse volumes
Line 173: """
Line 180:
Line 181: if preallocate == volume.SPARSE_VOL:
Line 182: if initial_size:
Line 183: initial_size = int(initial_size * QCOW_OVERHEAD_FACTOR)
Line 184: vol_size_mb = ((initial_size + SECTORS_TO_MB - 1)
> Lets call this alloc_size to make this more clear.
Done
Line 185: / SECTORS_TO_MB)
Line 186: else:
Line 187: vol_size_mb = config.getint("irs",
Line 188: "volume_utilization_chunk_mb")
https://gerrit.ovirt.org/#/c/46417/6/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 1453: # TODO: For backwards compatibility, we need to support accepting
Line 1454: # number of sectors as int type Updated interface is accepting string
Line 1455: # type in bytes (ugly, get rid of this when possible)
Line 1456: size = misc.validateSize(size, "size")
Line 1457: initialSize = misc.validateSize(initialSize, "initialSize")
> This allows now None size, which will fail in the lower levels. I think we
Done
Line 1458:
Line 1459: if srcImgUUID:
Line 1460: misc.validateUUID(srcImgUUID, 'srcImgUUID')
Line 1461: if srcVolUUID:
https://gerrit.ovirt.org/#/c/46417/6/vdsm/storage/misc.py
File vdsm/storage/misc.py:
Line 456: number of sectors as int type and bytes as string type.
Line 457: In case of bytes, we need to convert to sectors.
Line 458: """
Line 459: if size is None:
Line 460: return None
> Don't support None, see my comment in hsm.py.
Done
Line 461: elif isinstance(size, basestring):
Line 462: size = validateN(size, name)
Line 463: return (size + SECTOR_SIZE - 1) / SECTOR_SIZE
Line 464: elif isinstance(size, int):
--
To view, visit https://gerrit.ovirt.org/46417
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf5f142541bf0b311a77a0544a1c4ffb689a9fde
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list