Nir Soffer has posted comments on this change. Change subject: fencing: Skip fencing if host is maintaining its lease ......................................................................
Patch Set 8: (2 comments) I'm not convinced that "on" should be skipped. http://gerrit.ovirt.org/#/c/30762/8/vdsm/API.py File vdsm/API.py: Line 1198: Line 1199: if action not in ('status', 'on', 'off', 'reboot'): Line 1200: raise ValueError('illegal action ' + action) Line 1201: Line 1202: if action != 'status' and not should_fence(policy): When you return {"operationStatus": "skipped"}, does engine abort the fencing operation - right? I don't see any reason to skip the "on" operation, other then the fact that we use it with "off", so we never send "on" without "off". But this make the API depend on the usage of the client, which does not make sense. Checking should_fence() for "on" operation does not make sense, since it makes sense to send "on" to a powered off host, and such host cannot have live lease on storage. Line 1203: self.log.debug("Skipping execution of action '%s'", action) Line 1204: return {'status': doneCode, 'operationStatus': 'skipped'} Line 1205: Line 1206: script = constants.EXT_FENCE_PREFIX + agent Line 1237: 'power': power} Line 1238: threading.Thread(target=fence, args=(script, inp)).start() Line 1239: Line 1240: return {'status': doneCode, Line 1241: 'operationStatus': 'initiated'} The indentation is still different from the return on line 1204, for no reason. Line 1242: Line 1243: def ping(self): Line 1244: "Ping the server. Useful for tests" Line 1245: updateTimestamp() -- To view, visit http://gerrit.ovirt.org/30762 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b1fd9521cebea28d0402e53d46d74b95e73f383 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
