Martin Peřina has posted comments on this change.

Change subject: fencing: Skip fencing if host is maintaining its lease
......................................................................


Patch Set 5:

(6 comments)

http://gerrit.ovirt.org/#/c/30762/5/vdsm/API.py
File vdsm/API.py:

Line 1184:             # seconds. If so, we consider the host Up and we won't 
execute
Line 1185:             # fencing, even when it's unreachable from engine
Line 1186:             for sd, status in result['domains'].iteritems():
Line 1187:                 if status == storage.clusterlock.HOST_STATUS_LIVE:
Line 1188:                     self.log.debug("Host has live lease to '%s'", sd)
> live lease *on*
Done
Line 1189:                     return False
Line 1190: 
Line 1191:             self.log.debug("Host doesn't have any live lease")
Line 1192:             return True


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):
> +1
On engine side the fencing policy is added to request only as a part of 
VdsNotResponsiveTreatment, which call stop (off) and start (on) fencing 
request. All other manual power management operation are without fencing policy 
specified, so they will be always executed
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 1236:             return {'status': {'code': 0, 'message': message},
Line 1237:                     'power': power}
Line 1238:         threading.Thread(target=fence, args=(script, inp)).start()
Line 1239: 
Line 1240:         return {
> Why different indentation here? (Compared to line 1204)
Done
Line 1241:             'status': doneCode,
Line 1242:             'operationStatus': 'initiated',
Line 1243:         }
Line 1244: 


http://gerrit.ovirt.org/#/c/30762/5/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 295: # @skipped:     Fence script was not executed due to entered fencing 
policy
Line 296: #
Line 297: # Since: 4.16.0
Line 298: ##
Line 299: {'enum': 'FenceNodeResult', 'data': ['on', 'off', 'unknown', 
'initiated', 'skipped']}
> keep new lines shorter than 80 chars, please.
Done
Line 300: 
Line 301: ##
Line 302: # @HostIdMap:
Line 303: #


Line 310: 
Line 311: ##
Line 312: # @FencingPolicy:
Line 313: #
Line 314: # Additional options for fenceNode logic.
> If I understand correctly we have:
For oVirt 3.5 the policy contains more attributes on engine side, but only 
storageDomainHostIdMap will be sent to VDSM (all other fencing policy 
attributes are used only on engine side). In future I think there might me more 
attributes needed to be sent to VDSM, so we will add only new keys in policy 
map.

I don't see any reason for multiple policies and even if they would be needed 
on engine, engine know theirs logic so it can easily compact them into one 
policy sent to VDSM. But at the moment I don't know any requests which would 
need multiple policy object sent to VDSM.

And I used map to be able to add easily more options to fenceNode in future 
without need for complex logic on engine side if the parameter is supported or 
no (as we need currently so 3.5 engine is compatible with older VDSM versions)
Line 315: #
Line 316: # @storageDomainHostIdMap:   #optional map of storage domains and 
host ids
Line 317: #
Line 318: # Since: 4.16.0


Line 342: #
Line 343: # @options:   #optional Additional agent-specific parameters in 
space-separated
Line 344: #             <var>=<val> pairs
Line 345: #
Line 346: # @policy:   #optional Additional options needed in fenceNode logic
> Add the vdsm version of when this is introduced.
Done
Line 347: #
Line 348: # Returns:
Line 349: # Result of specified fence action
Line 350: #


-- 
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: 5
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

Reply via email to