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