Change in vdsm[master]: fencing: use deathSignal to keep scripts at bay

danken at redhat.com danken at redhat.com
Fri Jan 31 15:47:53 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: fencing: use deathSignal to keep scripts at bay
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.ovirt.org/#/c/22997/2/vdsm.spec.in
File vdsm.spec.in:

Line 103: Requires: iproute
Line 104: Requires: python-netaddr
Line 105: Requires: python-inotify
Line 106: Requires: python-argparse
Line 107: Requires: python-cpopen >= 1.2.4
> deathSignal is in since the first 1.2.3 that works well .. don't worry abou
Actually, what I need from 1.3 is its raising proper OSError, with errno inside it.
Line 108: Requires: python-ethtool >= 0.6-3
Line 109: Requires: %{name}-python-zombiereaper = %{version}-%{release}
Line 110: Requires: rpm-python
Line 111: Requires: nfs-utils


http://gerrit.ovirt.org/#/c/22997/2/vdsm/API.py
File vdsm/API.py:

Line 1115:         if action == 'status':
Line 1116:             try:
Line 1117:                 rc, out, err = utils.execCmd(
Line 1118:                     [script], deathSignal=signal.SIGTERM,
Line 1119:                     data=inp)
> just call fence() ..
Done
Line 1120:             except OSError as e:
Line 1121:                 if e.errno == os.errno.ENOENT:
Line 1122:                     return errCode['fenceAgent']
Line 1123:                 raise


Line 1134:             else:
Line 1135:                 power = 'unknown'
Line 1136:                 message = out + err
Line 1137:             return {'status': {'code': 0, 'message': message},
Line 1138:                     'power': power}
> else here can help to understand it better..
I'd like to limit this patch to the race removal.
Line 1139:         threading.Thread(target=fence, args=(script, inp)).start()
Line 1140:         return {'status': doneCode}
Line 1141: 
Line 1142:     def ping(self):


Line 1136:                 message = out + err
Line 1137:             return {'status': {'code': 0, 'message': message},
Line 1138:                     'power': power}
Line 1139:         threading.Thread(target=fence, args=(script, inp)).start()
Line 1140:         return {'status': doneCode}
> if you already on it , can you set ret = {}, fill it during the way and ret
I personally do no not fancy carrying a single object around and filling it up, in order to have a single "return" from the function. In any case, I'd like to keep such refactoring out of this race-removal patch.
Line 1141: 
Line 1142:     def ping(self):
Line 1143:         "Ping the server. Useful for tests"
Line 1144:         return {'status': doneCode}


-- 
To view, visit http://gerrit.ovirt.org/22997
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I068acf5ca0ad0813aaef660770de79958bd0b763
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list