Yoav Kleinberger has posted comments on this change.

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


Patch Set 8: Code-Review+1

(3 comments)

http://gerrit.ovirt.org/#/c/26046/8//COMMIT_MSG
Commit Message:

Line 11: deleting symbolic links or removing or renaming files and folders.
Line 12: This patch tries to solve this adding debug messages before the
Line 13: operations are executed.
Line 14: 
Line 15: This patch only includes the logging in the VDSM process so no OOP
OOP -> out-of-process

hate acronyms, even when I know them.
Line 16: log management is needed.
Line 17: 
Line 18: Change-Id: I3602513af123951f71091c03f799e36ea759aa61


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

Line 433:         if not self.imagePath:
Line 434:             self.validateImagePath()
Line 435:         volPath = os.path.join(self.imagePath, self.volUUID)
Line 436:         if not os.path.lexists(volPath):
Line 437:             srcPath = lvm.lvPath(self.sdUUID, self.volUUID)
I hate shorthand. srcPath -> sourcePath, volPath -> volumePath. 
a little more typing, a lot more clarity
Line 438:             self.log.debug("Creating symlink from %s to %s", srcPath, 
volPath)
Line 439:             os.symlink(srcPath, volPath)
Line 440:         self.volumePath = volPath
Line 441: 


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:
for extention in '.meta', '.lease':
    file = '%s%s' % ( volPath, extention )
    self.log.debug( 'removing %s' % file )
    self.oop.os.remove(file)

in fact, refactor this into a small function and call it.
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

Reply via email to