Change in vdsm[master]: Implement Popen without forking back to python
xiawenc at linux.vnet.ibm.com
xiawenc at linux.vnet.ibm.com
Wed May 9 09:19:45 UTC 2012
Wenchao Xia has posted comments on this change.
Change subject: Implement Popen without forking back to python
......................................................................
Patch Set 12: Verified; I would prefer that you didn't submit this
(4 inline comments)
Skill-full hacking!
Tested on following cases, no major issue found:
1 storage domain creating and deleting.
2 thread and popen race condition case.
Just a few comments inside the patch.
....................................................
File vdsm/betterPopen/createprocess.c
Line 36: "Execute a command command."},
on help(createProcess) I noticed "command command", maybe a mis-spell?
....................................................
File vdsm/betterPopen/__init__.py
Line 36: def __init__(self, args, close_fds=False, cwd=None, env=None):
In my test I found BetterPopen refuse command = "cat", but subprocess.Popen accept it. Better to add a few words here.
Line 48: def _execute_child(self, args, executable, preexec_fn, close_fds,
I think a check is needed to see whether Popen._init_ does other things before entering _execute_child, such as watching the child process's fds or lifetime. Actually I am worried about the situation cpython adding some codes between the Popen_init_ and _execute_child and do a check after child process dies in the future, In this case I am afraid a full test are need for each new python version vdsm is going to run on.
....................................................
File vdsm/betterPopen/Makefile.am
Line 27:
I guess this require building machine is the same with target machine, usually x86-64 to x86-64. But I think it is OK for most use cases.
--
To view, visit http://gerrit.ovirt.org/3944
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief4ae42eb57551d7f5893bf562da451a9c98f219
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Wenchao Xia <xiawenc at linux.vnet.ibm.com>
More information about the vdsm-patches
mailing list