Ayal Baron has posted comments on this change. Change subject: vm: add the live diskSizeExtend method ......................................................................
Patch Set 3: (5 inline comments) Partial review (stopped at hsm.py), will continue later. .................................................... File vdsm/storage/hsm.py Line 668: @public Line 669: def updateVolumeSize(self, spUUID, sdUUID, imgUUID, volUUID, newSize): Line 670: newSizeBytes = int(newSize) Line 671: volToExtend = \ Line 672: self.validateSdUUID(sdUUID).produceVolume(imgUUID, volUUID) split this line into 2 and you won't have to break the previous line and you will also get more clarity. i.e. dom = self.validateSdUUID(sdUUID) volToExtend = dom.produceVolume(imgUUID, volUUID) Line 673: volPath = volToExtend.getVolumePath() Line 674: volFormat = volToExtend.getFormat() Line 675: Line 676: if not volToExtend.isLeaf() or volToExtend.isShared(): Line 672: self.validateSdUUID(sdUUID).produceVolume(imgUUID, volUUID) Line 673: volPath = volToExtend.getVolumePath() Line 674: volFormat = volToExtend.getFormat() Line 675: Line 676: if not volToExtend.isLeaf() or volToExtend.isShared(): isShared is redundant since both methods are checking the same parameter (volType) and if it's not leaf (e.g. shared) then first condition will already catch it. Line 677: raise se.VolumeNonWritable(self.volUUID) Line 678: Line 679: if volFormat != volume.COW_FORMAT: Line 680: return dict(size=str(self.getVolumeSize(bs=1))) Line 675: Line 676: if not volToExtend.isLeaf() or volToExtend.isShared(): Line 677: raise se.VolumeNonWritable(self.volUUID) Line 678: Line 679: if volFormat != volume.COW_FORMAT: Need a comment explaining why you're not doing anything here (or doc string at start) Line 680: return dict(size=str(self.getVolumeSize(bs=1))) Line 681: Line 682: qemuImgFormat = volume.fmt2str(volume.COW_FORMAT) Line 683: Line 698: finally: Line 699: volToExtend.teardown(sdUUID, volUUID) Line 700: Line 701: # Rounding down as qemu/qemu-img Line 702: volToExtend.setSize(roundedSizeBytes / SECTOR_SIZE) 1. doesn't qemuImg already do that when you call qemuImg.info ? 2. Unless I'm missing something then if qemuImg.resize (or anything else under the try clause) throws an exception then roundedSizeBytes will be undefined... Line 703: return dict(size=str(roundedSizeBytes)) Line 704: Line 705: @public Line 706: def extendStorageDomain(self, sdUUID, spUUID, devlist, Line 3036: # during a transaction) the volume size. Line 3037: if capacity > 0: Line 3038: sectors = (capacity + SECTOR_SIZE - 1) / SECTOR_SIZE Line 3039: else: Line 3040: sectors = capacity user can pass negative numbers here. you can either check for it, or just set sectors to 0 in this case Line 3041: vol.setSize(sectors) Line 3042: Line 3043: @public Line 3044: def getVolumeInfo(self, sdUUID, spUUID, imgUUID, volUUID, options=None): -- To view, visit http://gerrit.ovirt.org/15614 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I326f0e71d53382a49eb3b43cdf0bc0472f71abdc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
