Nir Soffer has posted comments on this change. Change subject: storage: Clean up image links during teardownImage ......................................................................
Patch Set 2: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/55164/2//COMMIT_MSG Commit Message: Line 10: /var/run/vdsm/images/... Unfortunately on block domains we are leaving Line 11: around a broken symlink in /rhev/data-center/... in some cases and this Line 12: will cause storage operations related to a disk of a VM that was Line 13: previously running to fail on this host. The solution is to also remove Line 14: this broken symlink during teardownImage. This fix is not enough, we can still have broken links if vdsm is killed in the right moment before removing the stale link in teardownImage. The fix should also detect a broken link and remove it. The issue seems to be here: def validateImagePath(self): ... if not os.path.isdir(imageDir): try: os.mkdir(imageDir, 0o755) except Exception: self.log.exception("Unexpected error") raise se.ImagePathError(imageDir) self._imagePath = imageDir Something like: if os.path.islink(imageDir) and not os.path.exist(imageDir): self.log.warning("Removing stale image directory link: %r", imageDir) os.unlink(imageDir) With both changes we should be safe. We must remove this crap in 4.0. there should be only one image directory and it must be created in the same location in all flows. Line 15: Line 16: Change-Id: I5fb871f3ccd785c555c72db34ab818c04d000534 Line 17: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1319987 https://gerrit.ovirt.org/#/c/55164/2/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 1083: return dst Line 1084: Line 1085: def unlinkBCImage(self, imgUUID): Line 1086: img_path = self.getLinkBCImagePath(imgUUID) Line 1087: if os.path.islink(img_path): We must log this - all changes in file system must be logged - see linkBCImage. Line 1088: os.unlink(img_path) Line 1089: Line 1090: def createImageLinks(self, srcImgPath, imgUUID, volUUIDs): Line 1091: """ https://gerrit.ovirt.org/#/c/55164/2/vdsm/storage/hsm.py File vdsm/storage/hsm.py: Line 3265: vars.task.getSharedLock(STORAGE, sdUUID) Line 3266: Line 3267: dom = sdCache.produce(sdUUID) Line 3268: dom.deactivateImage(imgUUID) Line 3269: dom.unlinkBCImage(imgUUID) Better unlinkBCImage first, only then deactivate the image. Less likely to have broken link this way. Line 3270: Line 3271: @public Line 3272: def getVolumesList(self, sdUUID, spUUID, imgUUID=volume.BLANK_UUID, Line 3273: options=None): -- To view, visit https://gerrit.ovirt.org/55164 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5fb871f3ccd785c555c72db34ab818c04d000534 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.6 Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Idan Shaby <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
