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 <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>