Ayal Baron has posted comments on this change.
Change subject: image: shrink the new volume When merging block cow volumes
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(7 inline comments)
....................................................
File vdsm/storage/blockVolume.py
Line 291: """
Line 292: self.log.info("Request to reduce LV %s of image %s in VG %s
with "
Line 293: "size = %s", self.volUUID, self.imgUUID,
self.sdUUID,
Line 294: newSize)
Line 295: sizemb = (newSize + 2047) / 2048
how do you know that block size is always 512?
this should be a property of the VG.
Line 296: lvm.reduceLV(self.sdUUID, self.volUUID, sizemb)
Line 297:
Line 298: def shrinkToOptimalSize(self):
Line 299: """
Line 303: """
Line 304: volParams = self.getVolumeParams()
Line 305: if volParams['volFormat'] == volume.COW_FORMAT:
Line 306: lvm.activateLVs(self.sdUUID, self.volUUID)
Line 307: check = qemuImg.check(self.getVolumePath(),
qemuImg.FORMAT.QCOW2)
activate and deactivate need to be in try..finally or in a 'with' statement in
case check throws an exception
Line 308: volActualSize = check['offset']
Line 309: lvm.deactivateLVs(self.sdUUID, self.volUUID)
Line 310: volBaseSize = int(config.get("irs",
"volume_utilization_chunk_mb"))
Line 311: volBaseSize = volBaseSize * constants.MEGAB
Line 306: lvm.activateLVs(self.sdUUID, self.volUUID)
Line 307: check = qemuImg.check(self.getVolumePath(),
qemuImg.FORMAT.QCOW2)
Line 308: volActualSize = check['offset']
Line 309: lvm.deactivateLVs(self.sdUUID, self.volUUID)
Line 310: volBaseSize = int(config.get("irs",
"volume_utilization_chunk_mb"))
s/volBaseSize/volExtendSizeMB/
Line 311: volBaseSize = volBaseSize * constants.MEGAB
Line 312: volUtil = int(config.get("irs",
"volume_utilization_percent"))
Line 313: finalSize = (volActualSize + volBaseSize * volUtil * 0.01)
Line 314: filler = volBaseSize - (finalSize % volBaseSize)
Line 307: check = qemuImg.check(self.getVolumePath(),
qemuImg.FORMAT.QCOW2)
Line 308: volActualSize = check['offset']
Line 309: lvm.deactivateLVs(self.sdUUID, self.volUUID)
Line 310: volBaseSize = int(config.get("irs",
"volume_utilization_chunk_mb"))
Line 311: volBaseSize = volBaseSize * constants.MEGAB
s/volBaseSize/volExtendSize/
Line 312: volUtil = int(config.get("irs",
"volume_utilization_percent"))
Line 313: finalSize = (volActualSize + volBaseSize * volUtil * 0.01)
Line 314: filler = volBaseSize - (finalSize % volBaseSize)
Line 315: if filler != volBaseSize:
Line 310: volBaseSize = int(config.get("irs",
"volume_utilization_chunk_mb"))
Line 311: volBaseSize = volBaseSize * constants.MEGAB
Line 312: volUtil = int(config.get("irs",
"volume_utilization_percent"))
Line 313: finalSize = (volActualSize + volBaseSize * volUtil * 0.01)
Line 314: filler = volBaseSize - (finalSize % volBaseSize)
the following condition seems redundant, just s/filler = /finalSize +=/
Line 315: if filler != volBaseSize:
Line 316: finalSize += filler
Line 317: self.reduce((finalSize + 511) / 512)
Line 318: self.log.debug('Shrink qcow volume: %s to : %s bytes',
Line 313: finalSize = (volActualSize + volBaseSize * volUtil * 0.01)
Line 314: filler = volBaseSize - (finalSize % volBaseSize)
Line 315: if filler != volBaseSize:
Line 316: finalSize += filler
Line 317: self.reduce((finalSize + 511) / 512)
again alignment to block size doesn't look like it belongs here.
Line 318: self.log.debug('Shrink qcow volume: %s to : %s bytes',
Line 319: self.volUUID,
finalSize)
Line 320:
Line 321: @classmethod
....................................................
File vdsm/storage/image.py
Line 1192: except Exception:
Line 1193: self.log.error("Failure to remove subchain %s -> %s in
image %s", ancestor,
Line 1194: successor, imgUUID, exc_info=True)
Line 1195:
Line 1196: srcVol.shrinkToOptimalSize()
if we got here and shrink fail I don't think we should throw an error. Merge
succeeded, shrink is just an optimization.
Line 1197:
--
To view, visit http://gerrit.ovirt.org/9725
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8ace4c48d278cb84ce871bc402643131265c3198
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
