Dan Kenigsberg has posted comments on this change.
Change subject: image: shrink the new volume When merging block cow volumes
......................................................................
Patch Set 8: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File lib/vdsm/qemuImg.py
Line 96:
Line 97: cmd.append(image)
Line 98: rc, out, err = utils.execCmd(cmd)
Line 99:
Line 100: #FIXME: filter other rc as well
Sorry, I do not understand this comment.
Line 101: if rc != 0:
Line 102: raise QImgError(rc, out, err)
Line 103: try:
Line 104: check = {
Line 103: try:
Line 104: check = {
Line 105: 'offset': int(__iregexSearch("offset", out[1]))
Line 106: }
Line 107: except:
Edu alert!
Unless this error is very frequent, please include the exception traceback.
Line 108: raise QImgError(rc, out, err, "unable to parse qemu-img check
output")
Line 109:
Line 110: return check
Line 111:
....................................................
File vdsm/storage/blockVolume.py
Line 302: """
Line 303: self.log.info("Request to reduce LV %s of image %s in VG %s
with "
Line 304: "size = %s", self.volUUID, self.imgUUID,
self.sdUUID,
Line 305: newSize)
Line 306: sizemb = (newSize + 2047) / 2048
Isn't it just SECTORS_TO_MB as in extend()?
Line 307: lvm.reduceLV(self.sdUUID, self.volUUID, sizemb)
Line 308:
Line 309: def shrinkToOptimalSize(self):
Line 310: """
Line 328: finalSize = (volActualSize + volExtendSize * volUtil *
0.01)
Line 329: finalSize += volExtendSize - (finalSize % volExtendSize)
Line 330: self.log.debug('Shrink qcow volume: %s to : %s bytes',
Line 331: self.volUUID, finalSize)
Line 332: self.reduce((finalSize + 511) / 512)
Federico, what about your anti-512 vendetta ?
Line 333:
Line 334: @classmethod
Line 335: def renameVolumeRollback(cls, taskObj, sdUUID, oldUUID, newUUID):
Line 336: try:
....................................................
File vdsm/storage/volume.py
Line 561: Extend the apparent size of logical volume (thin provisioning)
Line 562: """
Line 563: pass
Line 564:
Line 565: def reduce(self, newsize):
Edu alert!
Do we really need these placeholders? If we do, do NOT use pass, but
raise NotImplementedError
instead. I think that only the FileVolume method should be implemented as
"pass".
Line 566: """
Line 567: reduce a logical volume
Line 568: """
Line 569: pass
--
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[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
