Martin Peřina has posted comments on this change. Change subject: fencing: Skip fencing if host is maintaining its lease ......................................................................
Patch Set 8: (2 comments) 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 fenci >When you return {"operationStatus": "skipped"}, does engine abort the fencing >operation - right? Yes >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. Well, with current content of fencing policy it does not make sense to check for 'on'. But 'on' is set action (unlike 'status' which is only get action), so I can imagine that we can add some new option into fencing policy for 'on' action (completely unrelated to host lease). And if we apply policy in 3.5 only for 'off' and 'reboot' and in 3.x we will need to apply this also for 'on', it may break API a lot. So IMO it makes sense to apply fencing policy to all set actions, even if current content of fencing policy doesn't have any meaning for all of them. Nevertheless if host is powered down, it should not have any active leases, so fence operation will not be skipped anyway. 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 rea OK, I didn;t notice it will fit into 80 characters. I will fix 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
