Yaniv Bronhaim has posted comments on this change.

Change subject: Using CrabRPC to communicate between vdsm and super vdsm
......................................................................


Patch Set 2: (2 inline comments)

....................................................
File vdsm/vdsm
Line 133: 
Line 134:     vdsmPid = os.getpid()
Line 135:     vdsm_r, svdsm_w = os.pipe()
Line 136:     svdsm_r, vdsm_w = os.pipe()
Line 137:     supervdsm.setPipe(vdsm_r, vdsm_w)
I get your point, it sounds reasonable to me. the creation and the managing of 
the rpc should be transparent to the rpc users or unrelated parts of code.. 
though here we initialize vdsm and as part of it i thought that initialize the 
internal communication should be here..
Line 138:     son_pid = os.fork()
Line 139: 
Line 140:     if son_pid != 0:
Line 141:         startSuperVdsm(svdsm_r, svdsm_w, son_pid)


Line 137:     supervdsm.setPipe(vdsm_r, vdsm_w)
Line 138:     son_pid = os.fork()
Line 139: 
Line 140:     if son_pid != 0:
Line 141:         startSuperVdsm(svdsm_r, svdsm_w, son_pid)
You right.. we don't invent anything, and the common use to query the 
subprocess as os.waitpid is enough for us.. the rpc is managed by crabRPC so we 
don't need the managers of multiprocessing, so why adding more external 
packages?
Line 142: 
Line 143:     else:
Line 144:         dropPrivileges()
Line 145:         log = initVdsmLogger()


--
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Royce Lv <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to