Eduardo has posted comments on this change. Change subject: vdsm: propagate MountError when mount fails ......................................................................
Patch Set 4: (5 inline comments) .................................................... 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()) Use fileutils.createdir() instead. Line 205: except OSError as e: Line 206: if e.errno != errno.EEXIST: Line 207: raise Line 208: Line 207: raise Line 208: Line 209: try: Line 210: self._mount.mount(self.options, self._vfsType) Line 211: except MountError as e: You need to log here the MountError. Line 212: try: Line 213: os.rmdir(self._getLocalPath()) Line 214: except OSError: Line 215: pass Line 209: try: Line 210: self._mount.mount(self.options, self._vfsType) Line 211: except MountError as e: Line 212: try: Line 213: os.rmdir(self._getLocalPath()) Please remove this try clause. There is no any reason to assume that failing to remove the mount point is OK here. Nobody should fiddle with the mount point during this thread mount process. Line 214: except OSError: Line 215: pass Line 216: raise e Line 217: 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 214: except OSError: Is a very bad practice to silently pass exceptions. Anyway, remove this try-except. Line 215: pass Line 216: raise e Line 217: Line 218: try: Line 214: except OSError: Line 215: pass Line 216: raise e Line 217: Line 218: try: The necessity for this check should be reconsidered if fileutils.createdir() was used. If it remains this block should be an else block. Line 219: fileSD.validateDirAccess(self.getMountObj().getRecord().fs_file) Line 220: except se.StorageServerAccessPermissionError: Line 221: try: Line 222: self.disconnect() -- 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
