Change in vdsm[master]: storage: Add logging on filesystem altering operations

danken at redhat.com danken at redhat.com
Thu May 15 16:37:22 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: storage: Add logging on filesystem altering operations
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.ovirt.org/#/c/26046/8/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 1075:         for volUUID in volUUIDs:
Line 1076:             srcVol = os.path.join(srcImgPath, volUUID)
Line 1077:             dstVol = os.path.join(imgRunDir, volUUID)
Line 1078:             try:
Line 1079:                 self.log.debug("Creating symlink from %s to %s", srcVol,
try-except blocks should be as short as possible. There's no need to put this log line inside it. If for some reason, self.log.debug() raises an EEXIST, you are going to add a false log line in 1084.

So if you can keep as many of your changes out of their respective try-except block, it would be an improvement.
Line 1080:                                dstVol)
Line 1081:                 os.symlink(srcVol, dstVol)
Line 1082:             except OSError as e:
Line 1083:                 if e.errno == errno.EEXIST:


http://gerrit.ovirt.org/#/c/26046/8/vdsm/storage/blockVolume.py
File vdsm/storage/blockVolume.py:

Line 250:             self.log.error("cannot remove volume %s/%s", self.sdUUID,
Line 251:                            self.volUUID, exc_info=True)
Line 252: 
Line 253:         try:
Line 254:             self.log.debug("Unlinking %s", vol_path)
here we try hard to swallow exceptions, so it's fine.
Line 255:             os.unlink(vol_path)
Line 256:             return True
Line 257:         except Exception as e:
Line 258:             eFound = e


-- 
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 <xfrancis at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Xavi Francisco <xfrancis at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list