Yaniv Bronhaim has posted comments on this change.
Change subject: [WIP]normalize startup: seperate supervdsm framework err and
function err
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(4 inline comments)
how is it better then current implementation? the split between framework error
and call error was handled also in previous version.
not sure if it adds something, but if you and one more will think that this
implementation is better or more clear and understandable than the current
version i will accept it
....................................................
File vdsm/supervdsm.py
Line 72: with self._supervdsmProxy.proxyLock:
Line 73: if not self._supervdsmProxy.isFunctional():
Line 74: self._supervdsmProxy.kill()
Line 75: self._supervdsmProxy.launch()
Line 76: raise
shouldn't you call the method again after launch?
Line 77: if ex:
Line 78: raise ex
Line 79: else:
Line 80: return ret
Line 127:
Line 128: def kill(self):
Line 129: try:
Line 130: if self._isSvdsmAlive():
Line 131: with open(self.pidfile, "r") as f:
you already read it, you can get it from _isSvdsmAlive as a return value
Line 132: pid = int(f.read().strip())
Line 133: misc.execCmd([constants.EXT_KILL, "-9", str(pid)],
sudo=True)
Line 134: except Exception:
Line 135: self._log.warn("Could not kill old Super Vdsm %s",
Line 146: spid = f.read().strip()
Line 147: with open(self.timestamp, "r") as f:
Line 148: createdTime = f.read().strip()
Line 149: except IOError as e:
Line 150: # pid file and timestamp file must be exist after first
launch,
you omit the first launch variable.. and i still think it useful. this is my
way to distinguish if someone deleted our svdsm files manually, or the files
weren't exist before we got here (possible only in first call).
Line 151: # otherwise excpetion will be raised to svdsm caller
Line 152: if e.errno == ENOENT:
Line 153: return False
Line 154: else:
....................................................
File vdsm/supervdsmServer.py
Line 80: try:
Line 81: ret = func(*args, **kwargs)
Line 82: except Exception:
Line 83: callbackLogger.error("Error in %s", func.__name__,
exc_info=True)
Line 84: ex = Exception
it doesn't keep the exception object.. it should be:
except Exception as e:
ex = e
Line 85: return (ret, ex)
Line 86:
Line 87: return wrapper
Line 88:
--
To view, visit http://gerrit.ovirt.org/10236
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib25809d4416f26bc95dc72e7b32b8b2a17a71879
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvro...@linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvro...@linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
