Adam Litke has posted comments on this change. Change subject: storage: Initial size for thin provisioning disk ......................................................................
Patch Set 4: (5 comments) https://gerrit.ovirt.org/#/c/46417/4/tests/blockVolumeTests.py File tests/blockVolumeTests.py: Line 35: (volume.PREALLOCATED_VOL, 0, None, "0"), Line 36: (volume.PREALLOCATED_VOL, 2048, None, "1"), Line 37: (volume.PREALLOCATED_VOL, 2049, None, "2"), Line 38: (volume.PREALLOCATED_VOL, 2097152, None, "1024"), Line 39: (volume.PREALLOCATED_VOL, 2049, 2048, "2"), Should we raise se.VolumeCreationError if you specify an initialSize value other then None when selecting PREALLOCATED_VOL? This is a request which we cannot and do not honor. Line 40: (volume.SPARSE_VOL, 9999, None, Line 41: config.get("irs", "volume_utilization_chunk_mb")), Line 42: (volume.SPARSE_VOL, 8388608, 2097152, "%s" % int(1024*1.1)), Line 43: (volume.SPARSE_VOL, 8388608, 2097153, "%s" % int(1025*1.1)), https://gerrit.ovirt.org/#/c/46417/4/vdsm/rpc/vdsmapi-schema.json File vdsm/rpc/vdsmapi-schema.json: Line 8286: # Line 8287: # @initialSize: #optional if specified, initial size of volume Line 8288: # for thin provisioning on block storage. Line 8289: # When using preallocated volumes or for file storage Line 8290: # the initial size is ignored. Units please. It's sectors right? Line 8291: # (new in version 4.17.8) Line 8292: # Line 8293: # Returns: Line 8294: # A task UUID https://gerrit.ovirt.org/#/c/46417/4/vdsm/storage/blockVolume.py File vdsm/storage/blockVolume.py: Line 162: Line 163: return (dom.sdUUID, slot) Line 164: Line 165: @classmethod Line 166: def _calculateVolumeSize(cls, preallocate, size, initialSize): Can we name this _calculateVolumeAllocSize ? Size is an often overloaded term and using a more specific term makes it easier to understand. Also consider renaming the size parameter to capacity. Line 167: if preallocate == volume.SPARSE_VOL: Line 168: if initialSize: Line 169: if initialSize > size: Line 170: log.error("The volume size %s is smaller " https://gerrit.ovirt.org/#/c/46417/4/vdsm/storage/fileVolume.py File vdsm/storage/fileVolume.py: Line 105: Class specific implementation of volumeCreate. All the exceptions are Line 106: properly handled and logged in volume.create() Line 107: """ Line 108: if initialSize: Line 109: cls.log.warn("initialSize is not supported for file-based volumes" warn seems a bit stern for this situation. I'd drop it to debug (or info at the highest). Line 110: ", ignoring it") Line 111: Line 112: sizeBytes = size * BLOCK_SIZE Line 113: truncSize = sizeBytes if volFormat == volume.RAW_FORMAT else 0 Line 106: properly handled and logged in volume.create() Line 107: """ Line 108: if initialSize: Line 109: cls.log.warn("initialSize is not supported for file-based volumes" Line 110: ", ignoring it") A bit of a nit, but I'd prefer the second line not start with a comma so move the word "volumes" down to the second line. Line 111: Line 112: sizeBytes = size * BLOCK_SIZE Line 113: truncSize = sizeBytes if volFormat == volume.RAW_FORMAT else 0 Line 114: -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Fred Rolland <[email protected]> Gerrit-Reviewer: Greg Padgett <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
