Change in vdsm[master]: fileSD: fix deleteImage flow
ishaby at redhat.com
ishaby at redhat.com
Tue Dec 22 12:14:39 UTC 2015
Idan Shaby has posted comments on this change.
Change subject: fileSD: fix deleteImage flow
......................................................................
Patch Set 1:
(5 comments)
https://gerrit.ovirt.org/#/c/50804/1//COMMIT_MSG
Commit Message:
Line 14:
Line 15: Also, if one of the files couldn't be removed, we didn't try to remove
Line 16: the others.
Line 17: Now we handle each removal separately, so that a failure in the removal
Line 18: of one file doesn't affect the removal of the others.
> Please describe also fixing handling of missing files, logging a warning in
Can't see a reason to describe here each and every case of the errors we print (the code is very simple), but I will add a few words just to make it even more clear.
Line 19:
Line 20: Bug-Url: https://bugzilla.redhat.com/1292092
Line 21: Change-Id: I9eeb28a70f708a4f9a5effe4ff294da63b757369
https://gerrit.ovirt.org/#/c/50804/1/vdsm/storage/fileSD.py
File vdsm/storage/fileSD.py:
Line 223: except OSError as e:
Line 224: self.log.error("removed image dir: %s can't be removed", toDelDir)
Line 225: raise se.ImageDeleteError("%s %s" % (imgUUID, str(e)))
Line 226:
Line 227: def _deleteFile(self, fileType, filePath):
> - Rename _deleteFile to _deleteVolumeFile. This is not a generic utility fo
Done
Line 228: try:
Line 229: self.oop.os.remove(filePath)
Line 230: self.log.debug("%s file: %s was removed.", fileType, filePath)
Line 231: except OSError as ose:
Line 226:
Line 227: def _deleteFile(self, fileType, filePath):
Line 228: try:
Line 229: self.oop.os.remove(filePath)
Line 230: self.log.debug("%s file: %s was removed.", fileType, filePath)
> Put the log *before* the operation - if the operation get stuck, we want to
Done
Line 231: except OSError as ose:
Line 232: if ose.errno != errno.ENOENT:
Line 233: self.log.error("%s file: %s can't be removed.",
Line 234: fileType, filePath, exc_info=True)
Line 227: def _deleteFile(self, fileType, filePath):
Line 228: try:
Line 229: self.oop.os.remove(filePath)
Line 230: self.log.debug("%s file: %s was removed.", fileType, filePath)
Line 231: except OSError as ose:
> Don't use such names, in this context there is only *one* exception, and th
Done
Line 232: if ose.errno != errno.ENOENT:
Line 233: self.log.error("%s file: %s can't be removed.",
Line 234: fileType, filePath, exc_info=True)
Line 235:
Line 230: self.log.debug("%s file: %s was removed.", fileType, filePath)
Line 231: except OSError as ose:
Line 232: if ose.errno != errno.ENOENT:
Line 233: self.log.error("%s file: %s can't be removed.",
Line 234: fileType, filePath, exc_info=True)
> Do not use exc_info, it is non-standard and we want to kill it.
Done
Line 235:
Line 236: def getAllVolumes(self):
Line 237: """
Line 238: 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: 1
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