Change in vdsm[master]: [WIP]fencenode: split fenceNode into its own module

danken at redhat.com danken at redhat.com
Tue Jul 14 16:04:54 UTC 2015


Dan Kenigsberg has posted comments on this change.

Change subject: [WIP]fencenode: split fenceNode into its own module
......................................................................


Patch Set 1:

(5 comments)

https://gerrit.ovirt.org/#/c/43597/1/lib/vdsm/fencenode.py
File lib/vdsm/fencenode.py:

Line 28: 
Line 29: 
Line 30: @utils.traceback(on=logging.name)
Line 31: def fence(script, data):
Line 32:     # non-status actions are sent asyncronously. deathSignal is set to
I think that this comment is out of date - all actions are run in sync. However, using deathSignal=SIGTERM is a good idea anyway.
Line 33:     # make sure that no stray fencing scripts are left behind if Vdsm
Line 34:     # crashes.
Line 35:     rc, out, err = utils.execCmd(
Line 36:         [script], deathSignal=signal.SIGTERM, data=data)


Line 38:                   hidePasswd(data), out, err)
Line 39:     return rc, out, err
Line 40: 
Line 41: 
Line 42: def hidePasswd(text):
please use pep8-client names in the new module.

also, please keep functions private unless needed outside the module.
Line 43:     cleantext = ''
Line 44:     for line in text.splitlines(True):
Line 45:         if line.startswith('passwd='):
Line 46:             line = 'passwd=XXXX\n'


Line 96:                 'power': power}
Line 97:     if rc != 0:
Line 98:         message = out + err
Line 99:     return {'status': {'code': rc, 'message': message},
Line 100:             'power': 'unknown', 'operationStatus': 'initiated'}
operationStatus  should be added by the caller, depnding on whether we skipped fencing or not.


https://gerrit.ovirt.org/#/c/43597/1/vdsm/API.py
File vdsm/API.py:

Line 1185:     def __init__(self):
Line 1186:         APIBase.__init__(self)
Line 1187: 
Line 1188:     # General Host functions
Line 1189:     def ping(self):
why's this move?
Line 1190:         "Ping the server. Useful for tests"
Line 1191:         updateTimestamp()
Line 1192:         return {'status': doneCode}
Line 1193: 


Line 1213: 
Line 1214:             result = self._irs.getHostLeaseStatus(hostIdMap)
Line 1215:             rc = result['status']['code']
Line 1216:             if rc != 0:
Line 1217:                 logging.error(
I don't think self.log -> logging relates to this patch.
Line 1218:                     "Error getting host lease status, error code '%s'", rc)
Line 1219:                 return True
Line 1220: 
Line 1221:             # HOST_STATUS_LIVE means that host renewed its lease in last 80


-- 
To view, visit https://gerrit.ovirt.org/43597
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7bd5e7246cf6da21e355849014a8fc71d5edbde6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list