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