Shu Ming has posted comments on this change.

Change subject: BZ#836161 - Refactored deleteImage.
......................................................................


Patch Set 1: (2 inline comments)

....................................................
File vdsm/storage/fileSD.py
Line 314:         return imgList
Line 315: 
Line 316:     def deleteImage(self, sdUUID, imgUUID, volsImgs):
Line 317:         oPath = getImagePath(sdUUID, imgUUID)
Line 318:         head, tail = self.oop.os.path.split(oPath)
I think you needn't have getImagePath() function.  You only need: 
tail = imgUUID
head = os.path.join(getDompath(sdUUID), 'images')
Line 319:         nPath = os.tempnam(head, sd.REMOVED_IMAGE_PREFIX + tail)
Line 320:         try:
Line 321:             self.oop.os.rename(oPath, nPath)
Line 322:         except OSError as e:


....................................................
File vdsm/storage/hsm.py
Line 1265: 
Line 1266:         vars.task.getExclusiveLock(STORAGE, imgUUID)
Line 1267:         vars.task.getSharedLock(STORAGE, sdUUID)
Line 1268:         allVols = dom.getAllVolumes()
Line 1269:         imgsByVol = sd.getVolsOfImage(allVols, imgUUID)
Is that not "volsByImg" instead of "imgsByVol"?
Line 1270:         # Do not validate if forced.
Line 1271:         if not misc.parseBool(force) and not dom.isBackup() \
Line 1272:                 and not all(len(v.imgs) == 1 for v in 
imgsByVol.itervalues()):
Line 1273:                 msg = "Cannot delete shared image %s. volImgs: %s" \


--
To view, visit http://gerrit.ovirt.org/8506
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I304ff5cd70186ffc9789cd1ac9337efa6c5ff695
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to