Shu Ming has posted comments on this change. Change subject: BZ#836161 - Rewrited deleteImage(). ......................................................................
Patch Set 2: I would prefer that you didn't submit this (2 inline comments) .................................................... File vdsm/storage/fileSD.py Line 317: return imgList Line 318: Line 319: def deleteImage(self, sdUUID, imgUUID, volsImgs): Line 320: currImgDir = getImagePath(sdUUID, imgUUID) Line 321: dirName, baseName = self.oop.os.path.split(currImgDir) The same comments as before, Does these make things simple? like: "baseName = imgUUID dirName = os.path.join(getDompath(sdUUID), 'images')" You can save one function call at least, also "os.path.join()" is faster then split(). Line 322: toDelDir = os.tempnam(dirName, sd.REMOVED_IMAGE_PREFIX + baseName) Line 323: try: Line 324: self.oop.os.rename(currImgDir, toDelDir) Line 325: 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) The same comment as the comment in patch I. I think "imgsByVol" should be "volsByImg". 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Eduardo <[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
