On 12/20/2013 09:34 AM, Daniel P. Berrange wrote:
On Fri, Dec 20, 2013 at 08:24:41PM +0400, Reco wrote:
Implement virProcessRunInMountNamespace, which runs callback of type virProcessNamespaceCallback in a container namespace.
Hope it'll nail it this time.
Fails 'make syntax-check': prohibit_fork_wrappers src/util/virprocess.c:874: switch (cpid = fork()) { maint.mk: use virCommand for child processes make: *** [sc_prohibit_fork_wrappers] Error 1
+ + switch (cpid = fork()) { + case 0: + if (setns(fd, 0) == -1) { + _exit(-1);
exit(-1) is wrong; the user will see $? of 255 (not -1), and it is not our typical exit status for failure (where we normally want to be < 128 so that it is not confused with exit due to signal). Besides, I hate magic numbers; better would be _exit(EXIT_FAILURE).
+ default: + if (virProcessWait(cpid, &status) < 0 || status < 0) {
No need to pass &status to virProcessWait() if you are going to insist on 0 status anyways; better to pass NULL and let virProcessWait do the validation on your behalf.
+ virReportSystemError(errno, + _("Callback failed with status %i"), + status);
This overwrites the (better) error message that virProcessWait() already generated.
+ ret = 1;
This returns a number > 0 on failure, even though our normal conventions is to return a negative number on failure.
+++ b/src/util/virprocess.h @@ -60,4 +60,10 @@ int virProcessSetNamespaces(size_t nfdlist, int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes); int virProcessSetMaxProcesses(pid_t pid, unsigned int procs); int virProcessSetMaxFiles(pid_t pid, unsigned int files); + +typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);
Worth documenting that the return value of this callback is passed on to _exit() (and thus that errors should be EXIT_FAILURE rather than our typical -1 on failure). I'll repost a full v5 series with my proposed fixes to these issues in a separate thread, and wait for a review (since this fixes a CVE, it needs a second set of eyes). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org