Saggi Mizrahi has uploaded a new change for review. Change subject: Move fencing logic out of API.py ......................................................................
Move fencing logic out of API.py API.py should not contain logic. In this patch I also: - Separate the `status` action to a different function because it does something completely different from the other action and has different logic. - Refactor for better code reusability readability. - Use real exception internally and have the API layer translate them to actual exceptions. - Don't support port strings in methods, just in the interface. - Don't support string booleans in the methods, just in the interface. - Added shutdownEvent to ClientIF so there is a public standard way of inspecting vdsm shutdown. - Make the fencing logic use betterPopen Change-Id: I944c6548a42612f705a410fb4290215451bca035 Signed-off-by: Saggi Mizrahi <[email protected]> --- M vdsm.spec.in M vdsm/API.py M vdsm/Makefile.am M vdsm/clientIF.py 4 files changed, 25 insertions(+), 72 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/7190/1 diff --git a/vdsm.spec.in b/vdsm.spec.in index 23afd6b..f122cb5 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -558,6 +558,7 @@ %{_datadir}/%{vdsm_name}/caps.py* %{_datadir}/%{vdsm_name}/clientIF.py* %{_datadir}/%{vdsm_name}/API.py* +%{_datadir}/%{vdsm_name}/fenceAgent.py* %{_datadir}/%{vdsm_name}/hooking.py* %{_datadir}/%{vdsm_name}/hooks.py* %{_datadir}/%{vdsm_name}/libvirtev.py* diff --git a/vdsm/API.py b/vdsm/API.py index ae04e01..5826d81 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -19,12 +19,9 @@ # import os -import signal import copy -import subprocess import pickle import time -import threading import logging from vdsm import utils @@ -40,6 +37,7 @@ from vdsm.define import doneCode, errCode, Kbytes, Mbytes import caps from vdsm.config import config +import fenceAgent import supervdsm @@ -981,82 +979,32 @@ agent is one of (rsa, ilo, drac5, ipmilan, etc) action can be one of (status, on, off, reboot).""" - def waitForPid(p, inp): - """ Wait until p.pid exits. Kill it if vdsm exists before. """ - try: - p.stdin.write(inp) - p.stdin.close() - while p.poll() is None: - if not self._cif._enabled: - self.log.debug('killing fence script pid %s', p.pid) - os.kill(p.pid, signal.SIGTERM) - time.sleep(1) - try: - # improbable race: p.pid may now belong to another - # process - os.kill(p.pid, signal.SIGKILL) - except: - pass - return - time.sleep(1) - self.log.debug('rc %s inp %s out %s err %s', p.returncode, - hidePasswd(inp), - p.stdout.read(), p.stderr.read()) - except: - self.log.error("Error killing fence script", exc_info=True) - - def hidePasswd(text): - cleantext = '' - for line in text.splitlines(True): - if line.startswith('passwd='): - line = 'passwd=XXXX\n' - cleantext += line - return cleantext - self.log.debug('fenceNode(addr=%s,port=%s,agent=%s,user=%s,' + 'passwd=%s,action=%s,secure=%s,options=%s)', addr, port, agent, username, 'XXXX', action, secure, options) - if action not in ('status', 'on', 'off', 'reboot'): - raise ValueError('illegal action ' + action) + secure = utils.tobool(secure) + port = int(port) - script = constants.EXT_FENCE_PREFIX + agent + if action == "status": + try: + power = fenceAgent.getFenceStatus(addr, port, agent, username, + password, secure, options) + + return {'status': doneCode, + 'power': power} + + except fenceAgent.FenceStatusCheckError as e: + return {'status': {'code': 1, 'message': str(e)}} try: - p = subprocess.Popen([script], stdin=subprocess.PIPE, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, - close_fds=True) - except OSError, e: - if e.errno == os.errno.ENOENT: - return errCode['fenceAgent'] - raise + fenceAgent.fenceNode(addr, port, agent, username, password, secure, + options, self._cif.shutdownEvent) - inp = ('agent=fence_%s\nipaddr=%s\nlogin=%s\noption=%s\n' + - 'passwd=%s\n') % (agent, addr, username, action, password) - if port != '': - inp += 'port=%s\n' % (port,) - if utils.tobool(secure): - inp += 'secure=yes\n' - inp += options - if action == 'status': - out, err = p.communicate(inp) - self.log.debug('rc %s in %s out %s err %s', p.returncode, - hidePasswd(inp), out, err) - if not 0 <= p.returncode <= 2: - return {'status': {'code': 1, - 'message': out + err}} - message = doneCode['message'] - if p.returncode == 0: - power = 'on' - elif p.returncode == 2: - power = 'off' - else: - power = 'unknown' - message = out + err - return {'status': {'code': 0, 'message': message}, - 'power': power} - threading.Thread(target=waitForPid, args=(p, inp)).start() - return {'status': doneCode} + return {'status': doneCode} + + except fenceAgent.UnsupportedFencingAgentError: + return errCode['fenceAgent'] def ping(self): "Ping the server. Useful for tests" diff --git a/vdsm/Makefile.am b/vdsm/Makefile.am index 62ba982..4a7bc57 100644 --- a/vdsm/Makefile.am +++ b/vdsm/Makefile.am @@ -32,6 +32,7 @@ clientIF.py \ configNetwork.py \ debugPluginClient.py \ + fenceAgent.py \ guestIF.py \ hooking.py \ hooks.py \ @@ -52,7 +53,8 @@ tc.py \ vdsmDebugPlugin.py \ vmChannels.py \ - vm.py + vm.py \ + $(NULL) dist_vdsmpylib_PYTHON = \ __init__.py \ diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py index 8ba25a7..8760437 100644 --- a/vdsm/clientIF.py +++ b/vdsm/clientIF.py @@ -96,6 +96,7 @@ self.lastRemoteAccess = 0 self._memLock = threading.Lock() self._enabled = True + self.shutdownEvent = threading.Event() self._netConfigDirty = False self._prepareMOM() threading.Thread(target=self._recoverExistingVms, @@ -219,6 +220,7 @@ for binding in self.bindings.values(): binding.prepareForShutdown() self._enabled = False + self.shutdownEvent.set() self.channelListener.stop() self._hostStats.stop() if self.mom: -- To view, visit http://gerrit.ovirt.org/7190 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I944c6548a42612f705a410fb4290215451bca035 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
