Ayal Baron has posted comments on this change. Change subject: vdsm: propagate MountError when mount fails ......................................................................
Patch Set 4: (3 inline comments) Only thing that needs revising here is the logging issue (need to log mount failure) .................................................... File vdsm/storage/storageServer.py Line 200: if self._mount.isMounted(): Line 201: return Line 202: Line 203: try: Line 204: os.makedirs(self._getLocalPath()) Since Edu asked to review in light of 12042, then note that the suggestion to use fileutils.createdir here is irrelevant since the permissions you'd use are not relevant once dir is mounted (hence permissions check below is mandatory) Line 205: except OSError as e: Line 206: if e.errno != errno.EEXIST: Line 207: raise Line 208: Line 205: except OSError as e: Line 206: if e.errno != errno.EEXIST: Line 207: raise Line 208: Line 209: try: What is missing here and available there is the logging that mount failed. Line 210: self._mount.mount(self.options, self._vfsType) Line 211: except MountError as e: Line 212: try: Line 213: os.rmdir(self._getLocalPath()) Line 211: except MountError as e: Line 212: try: Line 213: os.rmdir(self._getLocalPath()) Line 214: except OSError: Line 215: pass right, not catching here would be masking a first order problem (mount failure) with a second order problem (removal of mount point failed when trying to clean up after mount failure) Line 216: raise e Line 217: Line 218: try: Line 219: fileSD.validateDirAccess(self.getMountObj().getRecord().fs_file) -- To view, visit http://gerrit.ovirt.org/10966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f36b3ea18690d7cf53439e5a0342b1495f4f181 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[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]> Gerrit-Reviewer: Vered Volansky <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
