Royce Lv has posted comments on this change.
Change subject: Using CrabRPC to communicate between vdsm and super vdsm
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/supervdsmServer.py
Line 98:
Line 99: @exportToSuperVdsm
Line 100: def ping(*args, **kwargs):
Line 101: # This method exists for testing purposes
Line 102: return True
I hope these functions can be in a class, or the structure of this file can be
misleading.
Line 103:
Line 104:
Line 105: @exportToSuperVdsm
Line 106: def getHardwareInfo(*args, **kwargs):
Line 388: raise \
Line 389: Exception('trying to set supervdsm pipe that already
initialized')
Line 390:
Line 391: vdsm_r, svdsm_w = os.pipe()
Line 392: svdsm_r, vdsm_w = os.pipe()
I think you do in _startSuperVdsm is the right way to init super, this part should go to
RPC server initialization, would you see the piece of code I share with you for
reference?
Line 393:
Line 394: supervdsm.setPipe(vdsm_r, vdsm_w)
Line 395: son_pid = os.fork()
Line 396: if son_pid != 0:
Line 407: os.close(vdsm_w)
Line 408: if svdsm_r is not None:
Line 409: os.close(svdsm_r)
Line 410: if svdsm_w is not None:
Line 411: os.close(svdsm_w)
These go to RPCServer too, such as providing a shutdown() function.
Line 412:
Line 413:
Line 414: def _startSuperVdsm(caller_r, caller_w, son_pid):
Line 415: global log
--
To view, visit
http://gerrit.ovirt.org/11910
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f1053e7d1264003fa265e44899d8b02f98bd68a
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.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: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server