10:22 p.m.
Royce Lv has uploaded a new change for review.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
[WIP]change startup process for vdsm and supervdsm
Using vdsmService to take care of start up and respawn process of
vdsm and supervdsm. Remove the function of vdsm to start previlege
supervdsm process.
Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
---
M vdsm.spec.in
M vdsm/Makefile.am
M vdsm/storage/misc.py
M vdsm/supervdsm.py
M vdsm/supervdsmServer.py
M vdsm/vdsm
A vdsm/vdsmService
M vdsm/vdsmd.init.in
M vdsm_reg/Makefile.am
9 files changed, 208 insertions(+), 63 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/4145/1
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
4:58 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Yaniv Bronhaim has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11:
all with topic "change-vdsm-startup" -
http://gerrit.ovirt.org/#/q/status:open+project:vdsm+branch:master+topic:...
can be abandoned.
thanks.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:09 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Dan Kenigsberg has abandoned this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Abandoned
supervdsm startup has been revamped base on this work, but now the patch can be
abandoned.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
9:32 p.m.
Royce Lv has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 4: (4 inline comments)
....................................................
File vdsm/supervdsm.py
Line 52: except:
For exception raised inside svdsm function I think we should not kill vdsm, will change
here
Line 53: os.unlink(ADDRESS)
You're right, even if svdsm has not clean up when exit, it will cleaned when restart,
it's redundent
Line 54: os.kill(os.getpid(), signal.SIGTERM)
The vdsm has signal handler:
def sigtermHandler(signum, frame):
if cif:
cif.prepareForShutdown()
if cif and cif.irs:
cif.irs.spmStop(cif.irs.getConnectedStoragePoolsList()['poollist'][0])
signal.signal(signal.SIGTERM, sigtermHandler)
That is why I choose to just send a signal to vdsm:)
....................................................
File vdsm/vdsm
Line 80: return
I will add test cases for all of start,restart and so on:)I thought non-root would fail
later, but here is better
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
11:54 p.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11:
Dear maintainers, as this patch is really long due and rebased for a lot of time. Would
you please give some feedback on this one? Thanks!
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:29 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11:
Thanks a lot for your review, Adam
Kill supervdsm really takes more time for the whole to restart (5s estimated), but it
will restart as we expect at last.
The reason is prepareForshutdown may be called several times,and here always take a
while.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
2:31 p.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Adam Litke has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11:
Hi Royce,
I was testing out your patch and I found that when I kill the supervdsm process vdsm does
not restart but if I kill the main vdsmd one then it does restart everything. Is this
expected behavior?
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:40 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Adam Litke has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:12 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11:
Use prepareforShutdown instead of misc.panic(kill-9) to make spm stats consistent and not
being rude when error happens.
keep using os.kill in supervdsm proxy because add clientif.prepareforShutdown will make it
unclean, also vdsm file is not a python module, and not proper for supervdsm proxy to
reference.
supervdsmServer's os.kill keep it was,made nothing changed.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:13 p.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 10:
Saggi,
Thanks a lot for review.About tunnel all os.kill to a single point:
(1)vdsm check if supervdsm died and killed itself.
(2)vdsm check call supervdsm failed and kill self(the hsm or clientIF will except the
error)
(3)supervdsm check if vdsm alive or killed itself.
I thought they could not be tunneled to a single point. Please feel free to correct me.In
fact I wait on IRC for few days wanting to ask about this:)
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:46 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Saggi Mizrahi has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 10: I would prefer that you didn't submit this
There are a lot of places where you os.kil(os.getpid(), ...)
You should use misc panic for that. And I also don't really understand why that
can't be funneled to a single spot where you panic.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
7:34 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Xu He Jie has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 10: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
12:11 p.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Adam Litke has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 10: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:18 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
oVirt Jenkins CI Server has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 10:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/476/ : ABORTED
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:56 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11: (4 inline comments)
....................................................
File vdsm/supervdsm.py
Line 53: try:
Line 54: return callMethod()
Line 55: except(IOError, socket.error, AuthenticationError):
Line 56: os.kill(os.getpid(), signal.SIGTERM)
Line 57: raise
I'll rebase on yours
Line 58:
Line 59:
Line 60: class SuperVdsmProxy(object):
Line 61: """
Line 67: try:
Line 68: misc.retry(self._connect, Exception, timeout=60)
Line 69: except Exception:
Line 70: self._log.debug("error raised when connecting to super
vdsm")
Line 71: os.unlink(ADDRESS)
Done
Line 72: os.kill(os.getpid(), signal.SIGTERM)
Line 73: self._log.debug("Connected to Super Vdsm")
Line 74:
Line 75: def open(self, *args, **kwargs):
....................................................
File vdsm/vdsm
Line 142:
Line 143: # When key is None, connections are restricted to child processes
Line 144: manager = _SuperVdsmManager(ADDRESS, None)
Line 145: vdsmPid = os.getpid()
Line 146: svdsmProc = Process(target=restartSvdsm, args=(manager, vdsmPid, ))
<ref:python doc>:Manager() controls a server process which holds Python objects and
allows other processes to manipulate them using proxies.
According to me, Our supervdsm process is more than just callable objects:
supervdsm process = main thread(for thread join and zombie join)+heart beat for
vdsm+manager.
Correct me if I misunderstand you.:)
Line 147: try:
Line 148: svdsmProc.start()
Line 149: waitThread = threading.Thread(target=__waitSvdsm,
Line 150: args=[svdsmProc])
Line 145: vdsmPid = os.getpid()
Line 146: svdsmProc = Process(target=restartSvdsm, args=(manager, vdsmPid, ))
Line 147: try:
Line 148: svdsmProc.start()
Line 149: waitThread = threading.Thread(target=__waitSvdsm,
1. if svdsm stops what should vdsm do?
maybe I can try to let vdsm run until it discovered svdsm died when call svdsm proxy.
since vdsm does not has privilege to restart svdsm, it just leave us one choice to restart
vdsm and svdsm all over. Or else we will need a daemon dedicate for 'vdsm' and
'svdsm' respectively stop and start , this logic is more complex, that is what I
do in *patch set 1*, see Danken's opinion about that.
Line 150: args=[svdsmProc])
Line 151: waitThread.setDaemon(True)
Line 152: waitThread.start()
Line 153: startVdsm(logger)
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:11 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Xu He Jie has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 9: (1 inline comment)
....................................................
File vdsm/supervdsmServer.py
Line 354: log.debug("Creating remote object manager")
Line 355: manager.register('instance', callable=_SuperVdsm)
Line 356:
Line 357: server = manager.get_server()
Line 358: servThread = threading.Thread(target=server.serve_forever)
Why run server as another thread?
Line 359: servThread.setDaemon(True)
Line 360: servThread.start()
Line 361:
Line 362: chown(ADDRESS, VDSM_USER, VDSM_GROUP)
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:56 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 9:
verfiied:vdsm process kill,supervdsm process kill and service restart, storage test for
NFS,ISCSI,vm creation test
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:53 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
oVirt Jenkins CI Server has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 9:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/405/ : ABORTED
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:53 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
oVirt Jenkins CI Server has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 9:
Build Started http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/405/
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:49 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
oVirt Jenkins CI Server has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 8:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/404/ : ABORTED
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
8:49 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
oVirt Jenkins CI Server has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/404/
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
10:28 p.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 7:
verified:
(1)start & restart service
(2)kill vdsm, service automatically restart
(3)kill supervdsm, service automatically restart
(4)getVdsCaps functions after every restart
(5)libvirt connection never lost
(6)localfs:create domain/pool/volume/vm pass
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
4:05 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 6: Fails; I would prefer that you didn't submit this
Rebased code failed storage functional test, will submit new version after resolve it.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
10:08 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Saggi Mizrahi has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 6: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
10:25 p.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Royce Lv has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 6: Verified
verified:vdsm reboot after super vdsm killed, vdsm reboot after vdsm killed, vdsm can
react on xmlrpc, vdsm will not loose connection to libvirt after 10min.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
9:24 a.m.
Adam Litke has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 5:
Hi Royce. The code looks good. I think you are getting closer but it is still not
working quite right for me. I noticed that when I use kill -9 for the vdsm process it
restarts as expected but if I use any other signal it will not restart. Also, when I kill
the supervdsm process, vdsm notices and shuts down but the process stays alive and so it
is not respawned.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
10:20 p.m.
Wenyi Gao has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm/supervdsmServer.py
Line 63: VDSM_BIN = "/usr/share/vdsm/vdsm"
VDSM_PIDFILE path should be '@VDSMRUNDIR(a)/vdsmd.pid' in supervdsmServer.py.in?
VDSM_BIN path should be '@VDSMDIR@/vdsm' ?
Same problem for other path such as SVDSM_PIDFILE ?
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
1 a.m.
Xu He Jie has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 2: (4 inline comments)
....................................................
File vdsm/supervdsmServer.py
Line 313: chown(ADDRESS, METADATA_USER, METADATA_GROUP)
This user should be same with the user which used to startup vdsm.py
....................................................
File vdsm/vdsm
Line 28: print " -h - Display this help message"
I guess you forgot change this usage message
Line 104: os.setuid(uid)
Hmm, move this to supervdsmServer? let supervdsmServer to decide which user will be used.
vdsm needn't know which user it want to use.
....................................................
File vdsm/vdsmd.init.in
Line 492: LC_ALL=C NICELEVEL=$vdsm_nice daemon @VDSMDIR@/respawn --minlifetime 10
--daemon --masterpid $RESPAWNPIDFILE $SVDSM_BIN
for fedora, respawn can be done by systemd, did you have plan for this?
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
10:04 p.m.
Royce Lv has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 4: (1 inline comment)
I think the two failure you tested is because I should kill then start vdsm instead just
start vdsm.
....................................................
File vdsm/supervdsm.py
Line 53: os.unlink(ADDRESS)
After a second thought, I think if svdsm is killed and not clean this ADDRESS, but in the
startup period, vdsm is started prior to svdsm, and if it doesn't clean ADDR, it will
raise "address already in use" error, so it is necessary.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
3:53 p.m.
New subject: Change in vdsm[master]: [WIP]change startup process for vdsm and supervdsm star...
Saggi Mizrahi has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm start vdsm process
with supervdsmServer.py, use vdsm.init to respwan supervdsmServer. supervdsm will
terminate when vdsm crashes. If supervdsm crashes, vdsm can only do tasks which
doesn't need conne
......................................................................
Patch Set 3: I would prefer that you didn't submit this
Having a physical socket is redundant when you fork from the same root process. You should
use a pipe.
Also, having supervdsm as the main file is not recommended as supervdsm is a tool and not
the main file.
As I see it.
vdsm.py:
createPipes()
forkAndStartsvdsm(pipes)
setCreds()
startRegualVdsm()
In each of the processes:
If the pipe breaks on either side exit
Sorry if being too vague
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
11:08 a.m.
ShaoHe Feng has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File vdsm/storage/misc.py
Line 751: return func(*argList)
yes, * is used for tupple.
....................................................
File vdsm/vdsmService
Line 43: def __findServicePath(name):
it is similar to __supervdsmServerPath() in vdsm/supervdsm.py.
I think this function is private, so it will not be called by other module.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
2:17 a.m.
ShaoHe Feng has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File vdsm/vdsm
Line 72:
whitespace. you can set git color to find whitespace easily.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
8:27 p.m.
Shu Ming has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 1: (3 inline comments)
....................................................
File vdsm/vdsmService
Line 43: def __findServicePath(name):
Is here any specific reason to use double underscore to prefix the function name?
Line 67: def parse_args():
These global variables can be removed. And make the function return the parsed value with
dict object.
Line 106: sys.exit(1)
Adding the abnormal exit message here will be helpful.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
10:02 p.m.
Royce Lv has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 1:
Thanks a lot,Dan, for reviewing this patch, this is just a prototype,
but I really appreciate your opinion about
in which way this should be done so post it with bugs and dirty codes:)
The motivation of this patch is we expect vdsm and supervdsm
can crash independently,let something like respawn/vdsmService
to take care their authorised communication,
We indeed considered to start supervdsm and then let it to fork vdsm,
but we don't want svdsm server be mixed up with startup process,
more over,we expect super vdsm crash will not result in restarting vdsm.
It has been accomplished at this point.
Reason for rewriting respawn in python is
we'd like start up and restart process convenient to debug,
But I agree with you, let systemd or upstart to take care of it is better.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
9:56 p.m.
Royce Lv has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 1:
Thanks a lot,Dan, for reviewing this patch, this is just a prototype,but I really
appreciate your opinion about in which way this should be done so post it with bugs and
dirty codes:)
The motivation of this patch is we expect vdsm and supervdsm can crash independently,let
something like respawn/vdsmService to take care their authorised communication, We indeed
considered to start supervdsm and then let it to fork vdsm, but we don't want svdsm
server be mixed up with startup process, more over,we expect super vdsm crash will not
result in restarting vdsm.It has been accomplished at this point.
Reason for rewriting respawn in python is we'd like start up and restart process
convenient to debug, But I agree with you, let systemd or upstart to take care of it is
better.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
2:55 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
thanks for tackling this annoying issue! I wish vdsm startup is normalized.
however, replacing the general-porpose 'respawn' by a vdsm-specific non-trivial
script such as vdsmService is not quite the cleanup I was hoping for.
What is the motivation for your patch? Has it been accomplished in your opinion?
I would appreciate if you could keep 'respawn' (for EL6), and make it start
superVdsm. superVdsm can then fork, relinquish root, and exec vdsm. This is not trivial to
implement, since we would like superVdsm to die if vdsm crashes. But with this, the
'respawn' logic can be replace by systemd for Fedora or upstart in Debian.
In any case, I have not figured it all up; we may end up needing something as complex as
the current suggestion.
....................................................
File vdsm/storage/misc.py
Line 724: def retry(func, argList=[], expectedException=Exception, tries=None,
better use a tupple here.
....................................................
File vdsm/supervdsm.py
Line 51: raise
that's just like not handling the exception, but slower...
....................................................
File vdsm/vdsmService
Line 120: def respawn(cmd, sudo=True):
it took me some time to stabilize that bash code. what is the urgency to rewrite it in
Python? Note that modern distributions (aka Fedora) has systemd, which kinda makes
'respawn' redundant, and older ones can make with the current bash.
Line 150: uid, gid = getpwnam('vdsm')[2:4:]
@VDSMUSER@
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
2:46 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Yaniv Bronhaim has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11:
http://gerrit.ovirt.org/#/c/11910 - is based on your changes here and adds the use in
rpyc
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
2:19 a.m.
New subject: Change in vdsm[master]: change startup process for vdsm and supervdsm
Yaniv Bronhaim has posted comments on this change.
Change subject: change startup process for vdsm and supervdsm
......................................................................
Patch Set 11: I would prefer that you didn't submit this
(7 inline comments)
This patch is too big and changes not only svdsm implementation but also vdsm startup
process and the way it starts and stops svdsm.
You didn't include unit tests for this patch so it's hard to know if it works in
different cases.
I prefer that you will split this patch and rebase it over upstream code before accepting
it, but if you want to keep it that way (what I think is wrong..), try to rebase it and
add uts for that and I'll try to verify if it works properly.
thanks.
....................................................
File vdsm/supervdsm.py
Line 53: try:
Line 54: return callMethod()
Line 55: except(IOError, socket.error, AuthenticationError):
Line 56: os.kill(os.getpid(), signal.SIGTERM)
Line 57: raise
we don't need to kill svdsm if we get IOError or socket.error, it could be raised from
svdsm internal implementation, like when delNetwork raised IOError.. it doesn't mean
that something is wrong with svdsm.. we should just raise the error.
Although, AuthenticationError must lead to svdsm restart.
Line 58:
Line 59:
Line 60: class SuperVdsmProxy(object):
Line 61: """
Line 67: try:
Line 68: misc.retry(self._connect, Exception, timeout=60)
Line 69: except Exception:
Line 70: self._log.debug("error raised when connecting to super
vdsm")
Line 71: os.unlink(ADDRESS)
unlink can cause exception and the next kill line will never run.. use utils.rmFile
Line 72: os.kill(os.getpid(), signal.SIGTERM)
Line 73: self._log.debug("Connected to Super Vdsm")
Line 74:
Line 75: def open(self, *args, **kwargs):
....................................................
File vdsm/supervdsmServer.py
Line 100: return _getLsBlk(*args, **kwargs)
Line 101:
Line 102: @logDecorator
Line 103: def stopSuperVdsm(self):
Line 104: os.unlink(ADDRESS)
you should use utils.rmFile here. but why do we need the option to stop svdsm from
outside?..
Line 105: os.kill(os.getpid(), signal.SIGTERM)
Line 106:
Line 107: @logDecorator
Line 108: def readMultipathConf(self):
Line 335:
Line 336: log = logging.getLogger("SuperVdsm.Server")
Line 337: try:
Line 338: # kill old supervdsm
Line 339: _killSupervdsm(log)
supservdsm.py should control the initialization, termination and calling for requests to
svdsm, supervdsmServer supposed to handle all svdsm api and startup. (and if svdsm should
die when vdsm goes done, the suicide process).
This implementation mix this concepts and turn supervdsmServer.py to be the manager. so
why do you keep supervdsm.py?
Line 340:
Line 341: log.debug("Creating PID file")
Line 342:
Line 343: with open(PIDFILE, "w") as f:
....................................................
File vdsm/vdsm
Line 81: log = logging.getLogger('vds')
Line 82: try:
Line 83: logging.root.handlers.append(logging.StreamHandler())
Line 84: log.handlers.append(logging.StreamHandler())
Line 85: chown(LOGFILE, VDSM_USER, VDSM_GROUP)
We don't want to perform this chown every startup.. permission to log are not supposed
to change.
This's why we start vdsm with user vdsm and not as root, we don't want to run vdsm
as root because we don't know what other programmers added to vdsm startup.
It is safer to change the user from root before invoking your process.
Line 86: except:
Line 87: log.error("Exception raised", exc_info=True)
Line 88: return log
Line 89:
Line 142:
Line 143: # When key is None, connections are restricted to child processes
Line 144: manager = _SuperVdsmManager(ADDRESS, None)
Line 145: vdsmPid = os.getpid()
Line 146: svdsmProc = Process(target=restartSvdsm, args=(manager, vdsmPid, ))
why did you change to Process instead of multiprocessing manager?
Line 147: try:
Line 148: svdsmProc.start()
Line 149: waitThread = threading.Thread(target=__waitSvdsm,
Line 150: args=[svdsmProc])
Line 145: vdsmPid = os.getpid()
Line 146: svdsmProc = Process(target=restartSvdsm, args=(manager, vdsmPid, ))
Line 147: try:
Line 148: svdsmProc.start()
Line 149: waitThread = threading.Thread(target=__waitSvdsm,
I think it's important and we can add it to supervdsm.py (not here.. just to separate
responsibilities, this py should only initialize vdsm), but I think we shouldn't stop
vdsm if svdsm stops.. but i need another opinion about it ..
Line 150: args=[svdsmProc])
Line 151: waitThread.setDaemon(True)
Line 152: waitThread.start()
Line 153: startVdsm(logger)
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
2:40 p.m.
Adam Litke has posted comments on this change.
Change subject: [WIP]change startup process for vdsm and supervdsm
......................................................................
Patch Set 4: Fails; I would prefer that you didn't submit this
(15 inline comments)
I performed the following tests with your patch:
* Normal service start, stop, restart: OK
* Kill vdsm, what happens? FAIL
vdsm process shuts down but remains in defunct state. Nothing is respawned. The
pokeParent function is unable to identify a problem with the vdsm process since 'kill
-0' still returns 0. See the end of this comment for an idea to fix this.
* Kill vdsm, then restart service: OK.
* Kill supervdsm, what happens? FAIL.
Traceback (most recent call last):
File "/usr/share/vdsm/vdsm", line 98, in startVdsm
serve_clients(log)
File "/usr/share/vdsm/vdsm", line 46, in serve_clients
cif = clientIF.clientIF(log)
File "/usr/share/vdsm/clientIF.py", line 96, in __init__
self._prepareBindings()
File "/usr/share/vdsm/clientIF.py", line 134, in _prepareBindings
self._loadBindingXMLRPC()
File "/usr/share/vdsm/clientIF.py", line 120, in _loadBindingXMLRPC
default_bridge)
File "/usr/share/vdsm/BindingXMLRPC.py", line 48, in __init__
self.server = self._createXMLRPCServer()
File "/usr/share/vdsm/BindingXMLRPC.py", line 137, in _createXMLRPCServer
requestHandler=LoggingHandler, logRequests=True)
File "/usr/lib64/python2.7/SimpleXMLRPCServer.py", line 590, in __init__
SocketServer.TCPServer.__init__(self, addr, requestHandler, bind_and_activate)
File "/usr/lib64/python2.7/SocketServer.py", line 408, in __init__
self.server_bind()
File "/usr/lib64/python2.7/SocketServer.py", line 419, in server_bind
self.socket.bind(self.server_address)
File "/usr/lib64/python2.7/socket.py", line 224, in meth
return getattr(self._sock,name)(*args)
error: [Errno 98] Address already in use
I believe this happens because the vdsm child was still running and when supervdsm was
respawned it tried to start a new child.
* Kill supervdsm, then restart service: OK.
Would this be a better startup sequence...
1) As root, create a new process for supervdsmServer
2) Drop privileges in the parent
3) Start vdsm in the parent
Then, if vdsm detects a problem with supervdsm, it can just exit. This will cause respawn
to restart everything again. So if either process (vdsm, or supervdsm) dies, the whole
thing will be respawned. Maybe there are problems with my idea too. What do you think?
....................................................
File vdsm/supervdsm.py
Line 52: except:
This is a bad condition. I think there should be some logging of the exception that is
causing vdsm to shut down. Also, are you sure you want to do this for all types of
exceptions? I think it's ok but we should double-check.
Line 53: os.unlink(ADDRESS)
I don't understand this. Don't we want supervdsmServer to clean up it's own
socket file when it shuts itself down in response to this failure.
Am I missing something?
Line 54: os.kill(os.getpid(), signal.SIGTERM)
Is this all we need to do to properly shut down vdsm (a question for smarter reviewers
than I)? vdsm does have an API called prepareForShutdown() that performs a lot more work
to gracefully shut down the daemon. Maybe we need to look at using it instead.
Line 68: os.unlink(ADDRESS)
Again, I am dubious about removing the server's socket file from here.
Line 69: os.kill(os.getpid(), signal.SIGTERM)
prepareForShutdown again?
....................................................
File vdsm/supervdsmServer.py
Line 267
This function no longer operates on a parent. Let's rename it to 'pokeVdsm'
and change the arg to vdsmPid to make it more clear.
Line 300: #kill old supervdsm
Add a space between # and kill.
....................................................
File vdsm/vdsm
Line 80: return
Instead of quietly returning here I would prefer that you raise an exception. If the user
doesn't start vdsm as root we want to catch it right away and provide a clear error
message.
I know I contributed this example code :)
Please consider adding a functional test to cover this case.
Line 81: vdsm_uid,vdsm_gid = getpwnam(VDSM_USER)[2:4:]
space between ',' and vdsm_gid
Line 99:
This blank line is not necessary.
Line 110: supervdsmServer.restartSvdsm(manager, vdsmPid)
This is just a one-liner. Why not inline the code directly in the function below?
Line 116: # None key make children inherit it from parent
May I suggest rewording this comment for clarity?
# When key is None, connections are restricted to child processes.
Line 117: manager = _SuperVdsmManager(ADDRESS)
Since you reference the authKey parameter in a comment, best to supply it as an explicit
parameter to the function:
manager = _SuperVdsmManager(ADDRESS, None)
Line 127: os.kill(vdsmPid, signal.SIGTERM)
What happens if vdsmPid isn't defined yet because an exception was thrown by
vdsmProc.start() ? You should either move vdsmProc.start() out of the try block or test
that the value of vdsmPid makes sense.
Line 129: vdsmProc.join()
Trailing whitespace.
--
To view, visit http://gerrit.ovirt.org/4145
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I69aae6b0b9529c80291d90c6ad14ff82b21aea53
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
3874
days inactive
4371
days old
vdsm-patches@lists.fedorahosted.org
38 comments
10 participants
participants (10)
-
agl＠us.ibm.com -
Dan Kenigsberg -
Jenkins CI RO -
lvroyce＠linux.vnet.ibm.com -
shaohef＠linux.vnet.ibm.com -
shuming＠linux.vnet.ibm.com -
smizrahi＠redhat.com -
wenyi＠linux.vnet.ibm.com -
Xu He Jie -
ybronhei＠redhat.com