Change in vdsm[master]: vm: increase the volume extension on storage migration

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vm: increase the volume extension on storage migration
......................................................................

Patch Set 3: I would prefer that you didn't submit this

(3 inline comments)

minor comments.

....................................................
File vdsm/libvirtvm.py
Line 1038:         return iface
Line 1039: 
Line 1040: 
Line 1041: class Drive(LibvirtVmDevice):
Line 1042:     VOLWM_CHUNK_MB = config.getint('irs', 'volume_utilization_chunk_mb')
what does VOLWM stand for?
Line 1043:     VOLWM_FREE_PCT = 100 - config.getint('irs', 'volume_utilization_percent')
Line 1044:     VOLWM_CHUNK_REPLICATE_MULT = 2  # Chunk multiplier during replication
Line 1045: 
Line 1046:     def __init__(self, conf, log, **kwargs):

Line 1078:         Returns the watermark limit, when the LV usage reaches this limit an
Line 1079:         extension is in order (thin provisioning on block devices).
Line 1080:         """
Line 1081:         return (self.VOLWM_FREE_PCT * self.volExtensionChunk *
Line 1082:                 (constants.MEGAB / 100))
The original calculation, avoiding the parenthesis, is slightly more exact. "100" divides the bigger number, not MEGAB. I see no reason to change that in this patch.
Line 1083: 
Line 1084:     def getNextVolumeExtSize(self):
Line 1085:         """
Line 1086:         Returns the next volume size in megabytes. This value is based on the

Line 1080:         """
Line 1081:         return (self.VOLWM_FREE_PCT * self.volExtensionChunk *
Line 1082:                 (constants.MEGAB / 100))
Line 1083: 
Line 1084:     def getNextVolumeExtSize(self):
the name confused me to think you are calculating the size of the next extension. actually it is is the next target size for the volume. I'd drop the Ext.
Line 1085:         """
Line 1086:         Returns the next volume size in megabytes. This value is based on the
Line 1087:         volExtensionChunk property and it's the size that should be requested
Line 1088:         for the next LV extension.

--
To view, visit http://gerrit.ovirt.org/9200
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib61375613712feb7118a80c50b73e678d257f251
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server