Change in vdsm[master]: vdsm: propagate MountError when mount fails

ewarszaw at redhat.com ewarszaw at redhat.com
Mon Mar 18 17:21:03 UTC 2013


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 <vvolansk at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list