Change in vdsm[master]: fileSD: fix deleteImage flow

ishaby at redhat.com ishaby at redhat.com
Wed Dec 23 08:16:55 UTC 2015


Idan Shaby has posted comments on this change.

Change subject: fileSD: fix deleteImage flow
......................................................................


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/50804/2/vdsm/storage/fileSD.py
File vdsm/storage/fileSD.py:

Line 225:             raise se.ImageDeleteError("%s %s" % (imgUUID, str(e)))
Line 226: 
Line 227:     def _deleteVolumeFile(self, path):
Line 228:         try:
Line 229:             self.log.debug("Removing file: %s", path)
> Put the log above the try, logging should never raise OSError, and if it do
Done
Line 230:             self.oop.os.remove(path)
Line 231:         except OSError as e:
Line 232:             if e.errno == errno.ENOENT:
Line 233:                 self.log.warning("File: %s doesn't exist.", path)


Line 229:             self.log.debug("Removing file: %s", path)
Line 230:             self.oop.os.remove(path)
Line 231:         except OSError as e:
Line 232:             if e.errno == errno.ENOENT:
Line 233:                 self.log.warning("File: %s doesn't exist.", path)
> Always show the error in the log!
Done
Line 234:             else:
Line 235:                 self.log.exception("File: %s can't be removed.", path)
Line 236: 
Line 237:     def getAllVolumes(self):


Line 231:         except OSError as e:
Line 232:             if e.errno == errno.ENOENT:
Line 233:                 self.log.warning("File: %s doesn't exist.", path)
Line 234:             else:
Line 235:                 self.log.exception("File: %s can't be removed.", path)
> Use %r for logging string value:
Done
Line 236: 
Line 237:     def getAllVolumes(self):
Line 238:         """
Line 239:         Return dict {volUUID: ((imgUUIDs,), parentUUID)} of the domain.


-- 
To view, visit https://gerrit.ovirt.org/50804
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9eeb28a70f708a4f9a5effe4ff294da63b757369
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list