Dan Kenigsberg has posted comments on this change. Change subject: BZ#788640 - Refactor Pool.deleteImage() ......................................................................
Patch Set 11: (4 inline comments) .................................................... File vdsm/storage/sd.py Line 143: """ Filter allVols dict for volumes related to imgUUID. it is very hard to understand what this function does by this docstring. it seems that you have different names relative to getAllVolumes() of the former patch. Unit-testing this would be very simple, so please add one. Line 152: if imgUUID in vol.imgs) vol.imgs may be quite long for template volumes. it may well be worthwhile to put the leading image apart, and make vol.imgs a set. .................................................... File vdsm/storage/sp.py Line 1712: :param postZero: bool if you're fixing the docstring, do it well: :param postZero: make sure image content is not usable by future volumes :type postZero: bool Line 1728: img.delete(sdUUID=sdUUID, imgUUID=imgUUID, postZero=postZero, force=force) I haven't got into reviewing this function.. maybe tomorrow.. but please make new lines shorter. -- To view, visit http://gerrit.ovirt.org/3464 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifbc1397c69c7ffa835abe0747b6573d56bb9d74e Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Haim Ateya <[email protected]> Gerrit-Reviewer: Igor Lvovsky <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
