Change in vdsm[master]: Add logging to fs operations in supervdsmServer

nsoffer at redhat.com nsoffer at redhat.com
Thu Mar 27 06:56:16 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: Add logging to fs operations in supervdsmServer
......................................................................


Patch Set 1:

(4 comments)

Lets split this to two patches - the first is about improving logging for creating and removing udev rules, the second is about improving loging when creating and removing the unix socket. The second patch is less interesting (storage-wise) and I don't want these changes to delay the first patch.

http://gerrit.ovirt.org/#/c/26121/1/vdsm/supervdsmServer
File vdsm/supervdsmServer:

Line 265:     def appropriateDevice(self, guid, thiefId):
Line 266:         ruleFile = _UDEV_RULE_FILE_NAME % (guid, thiefId)
Line 267:         rule = 'SYMLINK=="mapper/%s", OWNER="%s", GROUP="%s"\n' % (guid,
Line 268:                DISKIMAGE_USER, DISKIMAGE_GROUP)
Line 269:         with open(ruleFile, "w") as rf:
This needs logging - "Creating rule %s"
Line 270:             rf.write(rule)
Line 271: 
Line 272:     @logDecorator
Line 273:     def rmAppropriateRules(self, thiefId):


Line 279:         fails = []
Line 280:         for r in rules:
Line 281:             try:
Line 282:                 os.remove(r)
Line 283:                 self.log.debug("Rule %s removed", r)
Log debug before: "Removing rule %s", and after error on failures. But note that if remove failed because the file does not exist we should not log an error.
Line 284:             except OSError:
Line 285:                 fails.append(r)
Line 286:         return fails
Line 287: 


Line 396: 
Line 397:         log.debug("Cleaning old socket %s", address)
Line 398:         if os.path.exists(address):
Line 399:             os.unlink(address)
Line 400:             log.debug("Old socket %s cleaned", address)
We don't need this log - we just logged this before the operation. I think the best  change is to move the existing log into the if, since today we get a bogus log about cleaning old socket even when no such cleanup happens.

But this change is not related to storage, so lets split it to different patch.
Line 401: 
Line 402:         log.debug("Setting up keep alive thread")
Line 403: 
Line 404:         try:


Line 426:             log.debug("Terminated normally")
Line 427:         finally:
Line 428:             if os.path.exists(address):
Line 429:                 utils.rmFile(address)
Line 430:                 log.debug("Removed socket %s", address)
Log before the operation - but split it to the patch with the previous change.
Line 431: 
Line 432:     except Exception:
Line 433:         log.error("Could not start Super Vdsm", exc_info=True)
Line 434:         sys.exit(1)


-- 
To view, visit http://gerrit.ovirt.org/26121
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1d8ede0cbe0790bdfcd60ed833b3d9c54f31eba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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