You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2015/09/23 08:04:44 UTC
[2/2] mesos git commit: Replaced clone system call with os::clone.
Replaced clone system call with os::clone.
Review: https://reviews.apache.org/r/38625
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3b1eeec8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3b1eeec8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3b1eeec8
Branch: refs/heads/master
Commit: 3b1eeec85e51d548d9152da838a9cddebd14cf17
Parents: f279cf2
Author: haosdent huang <ha...@gmail.com>
Authored: Tue Sep 22 23:03:41 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Tue Sep 22 23:03:41 2015 -0700
----------------------------------------------------------------------
src/slave/containerizer/linux_launcher.cpp | 55 ++++---------------------
src/tests/containerizer/launch_tests.cpp | 22 +---------
src/tests/containerizer/ns_tests.cpp | 52 +++--------------------
3 files changed, 13 insertions(+), 116 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/slave/containerizer/linux_launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/linux_launcher.cpp b/src/slave/containerizer/linux_launcher.cpp
index fd0ffcf..459af1b 100644
--- a/src/slave/containerizer/linux_launcher.cpp
+++ b/src/slave/containerizer/linux_launcher.cpp
@@ -162,53 +162,6 @@ Future<hashset<ContainerID>> LinuxLauncher::recover(
}
-// Helper for clone() which expects an int(void*).
-static int childMain(void* _func)
-{
- const lambda::function<int()>* func =
- static_cast<const lambda::function<int()>*> (_func);
-
- return (*func)();
-}
-
-
-// The customized clone function which will be used by 'subprocess()'.
-static pid_t clone(
- const lambda::function<int()>& func,
- const Option<int>& namespaces)
-{
- // Stack for the child.
- // - unsigned long long used for best alignment.
- // - static is ok because each child gets their own copy after the clone.
- // - 8 MiB appears to be the default for "ulimit -s" on OSX and Linux.
- //
- // NOTE: We need to allocate the stack dynamically. This is because
- // glibc's 'clone' will modify the stack passed to it, therefore the
- // stack must NOT be shared as multiple 'clone's can be invoked
- // simultaneously.
- int stackSize = 8 * 1024 * 1024;
-
- unsigned long long *stack =
- new unsigned long long[stackSize/sizeof(unsigned long long)];
-
- int flags = namespaces.isSome() ? namespaces.get() : 0;
- flags |= SIGCHLD; // Specify SIGCHLD as child termination signal.
-
- LOG(INFO) << "Cloning child process with flags = "
- << ns::stringify(flags);
-
- pid_t pid = ::clone(
- childMain,
- &stack[stackSize/sizeof(stack[0]) - 1], // stack grows down.
- flags,
- (void*) &func);
-
- delete[] stack;
-
- return pid;
-}
-
-
static int childSetup(
int pipes[2],
const Option<lambda::function<int()>>& setup)
@@ -284,6 +237,12 @@ Try<pid_t> LinuxLauncher::fork(
// use CHECK.
CHECK_EQ(0, ::pipe(pipes));
+ int cloneFlags = namespaces.isSome() ? namespaces.get() : 0;
+ cloneFlags |= SIGCHLD; // Specify SIGCHLD as child termination signal.
+
+ LOG(INFO) << "Cloning child process with flags = "
+ << ns::stringify(cloneFlags);
+
Try<Subprocess> child = subprocess(
path,
argv,
@@ -293,7 +252,7 @@ Try<pid_t> LinuxLauncher::fork(
flags,
environment,
lambda::bind(&childSetup, pipes, setup),
- lambda::bind(&clone, lambda::_1, namespaces));
+ lambda::bind(&os::clone, lambda::_1, cloneFlags));
if (child.isError()) {
return Error("Failed to clone child process: " + child.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/tests/containerizer/launch_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launch_tests.cpp b/src/tests/containerizer/launch_tests.cpp
index dbe891c..de655ec 100644
--- a/src/tests/containerizer/launch_tests.cpp
+++ b/src/tests/containerizer/launch_tests.cpp
@@ -81,33 +81,13 @@ public:
launchFlags,
None(),
None(),
- lambda::bind(&clone, lambda::_1));
+ lambda::bind(&os::clone, lambda::_1, CLONE_NEWNS | SIGCHLD));
close(launchFlags.pipe_read.get());
close(launchFlags.pipe_write.get());
return s;
}
-
-private:
- static pid_t clone(const lambda::function<int()>& f)
- {
- static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
- return ::clone(
- _clone,
- &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down.
- CLONE_NEWNS | SIGCHLD, // Specify SIGCHLD as child termination signal.
- (void*) &f);
- }
-
- static int _clone(void* f)
- {
- const lambda::function<int()>* _f =
- static_cast<const lambda::function<int()>*> (f);
-
- return (*_f)();
- }
};
http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/tests/containerizer/ns_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/ns_tests.cpp b/src/tests/containerizer/ns_tests.cpp
index 3a22938..ac3c9ff 100644
--- a/src/tests/containerizer/ns_tests.cpp
+++ b/src/tests/containerizer/ns_tests.cpp
@@ -56,32 +56,6 @@ namespace internal {
namespace tests {
-// Helper for cloneChild() which expects an int(void*).
-static int cloneChildHelper(void* _func)
-{
- const lambda::function<int()>* func =
- static_cast<const lambda::function<int()>*> (_func);
-
- return (*func)();
-}
-
-
-static pid_t cloneChild(
- int flags,
- const lambda::function<int()>& func)
-
-{
- // 8 MiB stack for child.
- static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
- return ::clone(
- cloneChildHelper,
- &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down.
- flags | SIGCHLD,
- (void*) &func);
-}
-
-
// Test that a child in different namespace(s) can setns back to the
// root namespace. We must fork a child to test this because setns
// doesn't support multi-threaded processes (which gtest is).
@@ -121,7 +95,7 @@ TEST(NsTest, ROOT_setns)
None(),
None(),
None(),
- lambda::bind(&cloneChild, flags, lambda::_1));
+ lambda::bind(&os::clone, lambda::_1, flags | SIGCHLD));
// Continue in parent.
ASSERT_SOME(s);
@@ -164,9 +138,7 @@ TEST(NsTest, ROOT_setnsMultipleThreads)
}
-// Use a different child function for clone because it requires
-// int(*)(void*).
-static int childGetns(void* arg)
+static int childGetns()
{
// Sleep until killed.
while (true) { sleep(1); }
@@ -192,14 +164,7 @@ TEST(NsTest, ROOT_getns)
Try<int> nstype = ns::nstype(ns);
ASSERT_SOME(nstype);
- // 8 MiB stack for child.
- static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
- pid_t pid = clone(
- childGetns,
- &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down.
- SIGCHLD | nstype.get(),
- NULL);
+ pid_t pid = os::clone(childGetns, SIGCHLD | nstype.get());
ASSERT_NE(-1, pid);
@@ -224,7 +189,7 @@ TEST(NsTest, ROOT_getns)
}
-static int childDestroy(void* arg)
+static int childDestroy()
{
// Fork a bunch of children.
::fork();
@@ -251,14 +216,7 @@ TEST(NsTest, ROOT_destroy)
Try<int> nstype = ns::nstype("pid");
ASSERT_SOME(nstype);
- // 8 MiB stack for child.
- static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
-
- pid_t pid = clone(
- childDestroy,
- &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down.
- SIGCHLD | nstype.get(),
- NULL);
+ pid_t pid = os::clone(childDestroy, SIGCHLD | nstype.get());
ASSERT_NE(-1, pid);