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 <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Nir Soffer <[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
