Nir Soffer has posted comments on this change. Change subject: storage: Add logging on filesystem altering operations ......................................................................
Patch Set 8: (1 comment) http://gerrit.ovirt.org/#/c/26046/8/vdsm/storage/fileSD.py File vdsm/storage/fileSD.py: Line 395: self.log.debug("Removing file: %s", metaFile) Line 396: self.oop.os.remove(metaFile) Line 397: leaseFile = volPath + '.lease' Line 398: self.log.debug("Removing file: %s", leaseFile) Line 399: self.oop.os.remove(leaseFile) > suggest: This remove some repetitive code, but is little less clear. But this is a bad idea: self.log.debug( 'removing %s' % file ) There is no reason to pay for the formatting if you do not run with debug log level. This is why logger use log("format", *args). Refactoring function to some deleteVolume() mehod would be nice, but not in this patch. Line 400: except OSError: Line 401: self.log.error("vol: %s can't be removed.", Line 402: volPath, exc_info=True) Line 403: try: -- To view, visit http://gerrit.ovirt.org/26046 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3602513af123951f71091c03f799e36ea759aa61 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Xavi Francisco <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Xavi Francisco <[email protected]> Gerrit-Reviewer: Yoav Kleinberger <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
