You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2016/08/25 23:48:20 UTC
[2/2] mesos git commit: Avoided slicing of flags from `subprocess` in
mesos.
Avoided slicing of flags from `subprocess` in mesos.
While `FlagsBase` internally stores just maps of names and
flags, the functions stored in a `Flag` rely on the original type of
the `Flags` containing them (e.g., we perform dynamic casts to detect
their types).
Since `Option<T>` stores `T` as a value (i.e., it cannot contain
reference types) any interface taking an `Option<T>` cannot rely on
polymorphic behavior of `T`. To make use of polymorphism we should
instead store e.g., a pointer type to avoid slicing.
Here we change `Flags` arguments of `subprocess` to allow preserving
the original type so `Flag` functions can work reliably; we do this by
passing a non-owning pointer to a `Flags` so we do not restrict copying.
Review: https://reviews.apache.org/r/46822/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f3181999
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f3181999
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f3181999
Branch: refs/heads/master
Commit: f3181999fd03c99078994855687749839eb5365f
Parents: d05625f
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Thu Aug 25 13:53:15 2016 -0700
Committer: Michael Park <mp...@apache.org>
Committed: Thu Aug 25 16:17:14 2016 -0700
----------------------------------------------------------------------
src/docker/docker.cpp | 6 +++---
src/health-check/health_checker.cpp | 2 +-
src/launcher/posix/executor.cpp | 2 +-
src/linux/perf.cpp | 2 +-
src/slave/container_loggers/lib_logrotate.cpp | 4 ++--
src/slave/containerizer/docker.cpp | 2 +-
src/slave/containerizer/mesos/containerizer.cpp | 2 +-
.../mesos/isolators/docker/volume/driver.cpp | 4 ++--
.../mesos/isolators/network/cni/cni.cpp | 6 +++---
.../mesos/isolators/network/port_mapping.cpp | 4 ++--
.../containerizer/mesos/isolators/posix/disk.cpp | 2 +-
src/slave/containerizer/mesos/launcher.cpp | 2 +-
src/slave/containerizer/mesos/launcher.hpp | 4 ++--
src/slave/containerizer/mesos/linux_launcher.cpp | 2 +-
src/slave/containerizer/mesos/linux_launcher.hpp | 2 +-
src/tests/containerizer/isolator_tests.cpp | 16 ++++++++--------
src/tests/containerizer/launch_tests.cpp | 2 +-
src/tests/containerizer/launcher.hpp | 2 +-
.../containerizer/mesos_containerizer_tests.cpp | 2 +-
src/tests/containerizer/ns_tests.cpp | 2 +-
src/tests/containerizer/port_mapping_tests.cpp | 4 ++--
21 files changed, 37 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/docker/docker.cpp
----------------------------------------------------------------------
diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp
index 0a66046..16abf64 100755
--- a/src/docker/docker.cpp
+++ b/src/docker/docker.cpp
@@ -823,7 +823,7 @@ Future<Option<int>> Docker::run(
_stdout,
_stderr,
NO_SETSID,
- None(),
+ nullptr,
environment);
if (s.isError()) {
@@ -1267,7 +1267,7 @@ Future<Docker::Image> Docker::pull(
Subprocess::PIPE(),
Subprocess::PIPE(),
NO_SETSID,
- None());
+ nullptr);
if (s.isError()) {
return Failure("Failed to create subprocess '" + cmd + "': " + s.error());
@@ -1394,7 +1394,7 @@ Future<Docker::Image> Docker::__pull(
Subprocess::PIPE(),
Subprocess::PIPE(),
NO_SETSID,
- None(),
+ nullptr,
environment);
if (s_.isError()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/health-check/health_checker.cpp
----------------------------------------------------------------------
diff --git a/src/health-check/health_checker.cpp b/src/health-check/health_checker.cpp
index 45a5fe0..5bdbd2a 100644
--- a/src/health-check/health_checker.cpp
+++ b/src/health-check/health_checker.cpp
@@ -250,7 +250,7 @@ void HealthCheckerProcess::_commandHealthCheck()
Subprocess::FD(STDERR_FILENO),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- None(),
+ nullptr,
environment);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/launcher/posix/executor.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/posix/executor.cpp b/src/launcher/posix/executor.cpp
index 43573ca..50b9b30 100644
--- a/src/launcher/posix/executor.cpp
+++ b/src/launcher/posix/executor.cpp
@@ -111,7 +111,7 @@ pid_t launchTaskPosix(
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
SETSID,
- launchFlags);
+ &launchFlags);
if (s.isError()) {
ABORT("Failed to launch '" + commandString + "': " + s.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/linux/perf.cpp
----------------------------------------------------------------------
diff --git a/src/linux/perf.cpp b/src/linux/perf.cpp
index 9455210..556dc8d 100644
--- a/src/linux/perf.cpp
+++ b/src/linux/perf.cpp
@@ -134,7 +134,7 @@ private:
Subprocess::PIPE(),
Subprocess::PIPE(),
NO_SETSID,
- None(),
+ nullptr,
None(),
None(),
Subprocess::Hook::None(),
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/container_loggers/lib_logrotate.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/lib_logrotate.cpp b/src/slave/container_loggers/lib_logrotate.cpp
index 1fca486..0155275 100644
--- a/src/slave/container_loggers/lib_logrotate.cpp
+++ b/src/slave/container_loggers/lib_logrotate.cpp
@@ -147,7 +147,7 @@ public:
Subprocess::PATH("/dev/null"),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- outFlags,
+ &outFlags,
environment,
None(),
parentHooks);
@@ -194,7 +194,7 @@ public:
Subprocess::PATH("/dev/null"),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- errFlags,
+ &errFlags,
environment,
None(),
parentHooks);
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 8ecd773..110a1eb 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -1332,7 +1332,7 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
subprocessInfo.out,
subprocessInfo.err,
SETSID,
- launchFlags,
+ &launchFlags,
environment,
None(),
parentHooks,
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 7584a11..89b7e8d 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1253,7 +1253,7 @@ Future<bool> MesosContainerizerProcess::_launch(
: Subprocess::IO(subprocessInfo.out)),
(local ? Subprocess::FD(STDERR_FILENO)
: Subprocess::IO(subprocessInfo.err)),
- launchFlags,
+ &launchFlags,
environment,
namespaces); // 'namespaces' will be ignored by PosixLauncher.
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp b/src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp
index 842f2b5..ed3aa1d 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp
@@ -91,7 +91,7 @@ Future<string> DriverClient::mount(
Subprocess::PIPE(),
Subprocess::PIPE(),
NO_SETSID,
- None(),
+ nullptr,
None(),
None(),
{},
@@ -177,7 +177,7 @@ Future<Nothing> DriverClient::unmount(
Subprocess::PIPE(),
Subprocess::PIPE(),
NO_SETSID,
- None(),
+ nullptr,
None(),
None(),
{},
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
index 760d32b..d17a45f 100644
--- a/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
@@ -877,7 +877,7 @@ Future<Nothing> NetworkCniIsolatorProcess::__isolate(
Subprocess::PATH("/dev/null"),
Subprocess::PIPE(),
NO_SETSID,
- setup.flags);
+ &setup.flags);
if (s.isError()) {
return Failure(
@@ -1039,7 +1039,7 @@ Future<Nothing> NetworkCniIsolatorProcess::attach(
Subprocess::PIPE(),
Subprocess::PATH("/dev/null"),
NO_SETSID,
- None(),
+ nullptr,
environment);
if (s.isError()) {
@@ -1337,7 +1337,7 @@ Future<Nothing> NetworkCniIsolatorProcess::detach(
Subprocess::PIPE(),
Subprocess::PATH("/dev/null"),
NO_SETSID,
- None(),
+ nullptr,
environment);
if (s.isError()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
index 92f3c07..55a06cf 100644
--- a/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
+++ b/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
@@ -3082,7 +3082,7 @@ Future<Nothing> PortMappingIsolatorProcess::update(
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- update.flags);
+ &update.flags);
if (s.isError()) {
return Failure("Failed to launch update subcommand: " + s.error());
@@ -3209,7 +3209,7 @@ Future<ResourceStatistics> PortMappingIsolatorProcess::usage(
Subprocess::PIPE(),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- statistics.flags);
+ &statistics.flags);
if (s.isError()) {
return Failure("Failed to launch the statistics subcommand: " + s.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/isolators/posix/disk.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/posix/disk.cpp b/src/slave/containerizer/mesos/isolators/posix/disk.cpp
index f97ace9..5b9d5aa 100644
--- a/src/slave/containerizer/mesos/isolators/posix/disk.cpp
+++ b/src/slave/containerizer/mesos/isolators/posix/disk.cpp
@@ -503,7 +503,7 @@ private:
Subprocess::PIPE(),
Subprocess::PIPE(),
NO_SETSID,
- None(),
+ nullptr,
None(),
None(),
Subprocess::Hook::None(),
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launcher.cpp b/src/slave/containerizer/mesos/launcher.cpp
index 413a8af..8183a1c 100644
--- a/src/slave/containerizer/mesos/launcher.cpp
+++ b/src/slave/containerizer/mesos/launcher.cpp
@@ -112,7 +112,7 @@ Try<pid_t> PosixLauncher::fork(
const Subprocess::IO& in,
const Subprocess::IO& out,
const Subprocess::IO& err,
- const Option<flags::FlagsBase>& flags,
+ const flags::FlagsBase* flags,
const Option<map<string, string>>& environment,
const Option<int>& namespaces,
vector<process::Subprocess::Hook> parentHooks)
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/launcher.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launcher.hpp b/src/slave/containerizer/mesos/launcher.hpp
index 0eae095..61c2e84 100644
--- a/src/slave/containerizer/mesos/launcher.hpp
+++ b/src/slave/containerizer/mesos/launcher.hpp
@@ -67,7 +67,7 @@ public:
const process::Subprocess::IO& in,
const process::Subprocess::IO& out,
const process::Subprocess::IO& err,
- const Option<flags::FlagsBase>& flags,
+ const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& environment,
const Option<int>& namespaces,
std::vector<process::Subprocess::Hook> parentHooks =
@@ -119,7 +119,7 @@ public:
const process::Subprocess::IO& in,
const process::Subprocess::IO& out,
const process::Subprocess::IO& err,
- const Option<flags::FlagsBase>& flags,
+ const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& environment,
const Option<int>& namespaces,
std::vector<process::Subprocess::Hook> parentHooks =
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/linux_launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/linux_launcher.cpp b/src/slave/containerizer/mesos/linux_launcher.cpp
index 7377316..d0f9285 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -273,7 +273,7 @@ Try<pid_t> LinuxLauncher::fork(
const process::Subprocess::IO& in,
const process::Subprocess::IO& out,
const process::Subprocess::IO& err,
- const Option<flags::FlagsBase>& flags,
+ const flags::FlagsBase* flags,
const Option<map<string, string>>& environment,
const Option<int>& namespaces,
vector<Subprocess::Hook> parentHooks)
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/slave/containerizer/mesos/linux_launcher.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/linux_launcher.hpp b/src/slave/containerizer/mesos/linux_launcher.hpp
index 8fbe1e9..dca827c 100644
--- a/src/slave/containerizer/mesos/linux_launcher.hpp
+++ b/src/slave/containerizer/mesos/linux_launcher.hpp
@@ -45,7 +45,7 @@ public:
const process::Subprocess::IO& in,
const process::Subprocess::IO& out,
const process::Subprocess::IO& err,
- const Option<flags::FlagsBase>& flags,
+ const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& environment,
const Option<int>& namespaces,
std::vector<process::Subprocess::Hook> parentHooks);
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index 2725eb0..f8056ca 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -274,7 +274,7 @@ TYPED_TEST(CpuIsolatorTest, UserCpuUsage)
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
None(),
parentHooks);
@@ -387,7 +387,7 @@ TYPED_TEST(CpuIsolatorTest, SystemCpuUsage)
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
None(),
parentHooks);
@@ -560,7 +560,7 @@ TEST_F(RevocableCpuIsolatorTest, ROOT_CGROUPS_RevocableCpu)
Subprocess::PATH("/dev/null"),
Subprocess::PATH("/dev/null"),
Subprocess::PATH("/dev/null"),
- None(),
+ nullptr,
None(),
None());
@@ -670,7 +670,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_CFS_Enable_Cfs)
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
prepare.get().isSome() ? prepare.get().get().namespaces() : 0,
parentHooks);
@@ -775,7 +775,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_CFS_Big_Quota)
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
prepare.get().isSome() ? prepare.get().get().namespaces() : 0,
parentHooks);
@@ -874,7 +874,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
Subprocess::FD(inputPipes[0], Subprocess::IO::OWNED),
Subprocess::FD(outputPipes[1], Subprocess::IO::OWNED),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
prepare.get().isSome() ? prepare.get().get().namespaces() : 0,
parentHooks);
@@ -1372,7 +1372,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume)
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
prepare.get().get().namespaces());
ASSERT_SOME(pid);
@@ -1478,7 +1478,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume)
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
prepare.get().get().namespaces());
ASSERT_SOME(pid);
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/tests/containerizer/launch_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launch_tests.cpp b/src/tests/containerizer/launch_tests.cpp
index ef0c87c..76d71b3 100644
--- a/src/tests/containerizer/launch_tests.cpp
+++ b/src/tests/containerizer/launch_tests.cpp
@@ -79,7 +79,7 @@ public:
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- launchFlags,
+ &launchFlags,
None(),
lambda::bind(&os::clone, lambda::_1, CLONE_NEWNS | SIGCHLD));
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/tests/containerizer/launcher.hpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launcher.hpp b/src/tests/containerizer/launcher.hpp
index 94c62b7..f87b3ce 100644
--- a/src/tests/containerizer/launcher.hpp
+++ b/src/tests/containerizer/launcher.hpp
@@ -65,7 +65,7 @@ public:
const process::Subprocess::IO& in,
const process::Subprocess::IO& out,
const process::Subprocess::IO& err,
- const Option<flags::FlagsBase>& flags,
+ const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& env,
const Option<int>& namespaces,
std::vector<process::Subprocess::Hook> parentHooks));
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/tests/containerizer/mesos_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/mesos_containerizer_tests.cpp b/src/tests/containerizer/mesos_containerizer_tests.cpp
index b501ccb..72346c7 100644
--- a/src/tests/containerizer/mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/mesos_containerizer_tests.cpp
@@ -1221,7 +1221,7 @@ TEST_F(MesosLauncherStatusTest, ExecutorPIDTest)
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- None(),
+ nullptr,
None(),
None());
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/tests/containerizer/ns_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/ns_tests.cpp b/src/tests/containerizer/ns_tests.cpp
index 1f9d679..e52ebbe 100644
--- a/src/tests/containerizer/ns_tests.cpp
+++ b/src/tests/containerizer/ns_tests.cpp
@@ -88,7 +88,7 @@ TEST(NsTest, ROOT_setns)
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- None(),
+ nullptr,
None(),
lambda::bind(&os::clone, lambda::_1, flags | SIGCHLD));
http://git-wip-us.apache.org/repos/asf/mesos/blob/f3181999/src/tests/containerizer/port_mapping_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp
index db619ea..7fac6ca 100644
--- a/src/tests/containerizer/port_mapping_tests.cpp
+++ b/src/tests/containerizer/port_mapping_tests.cpp
@@ -345,7 +345,7 @@ protected:
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
Subprocess::FD(STDERR_FILENO),
- launchFlags,
+ &launchFlags,
None(),
CLONE_NEWNET | CLONE_NEWNS);
@@ -379,7 +379,7 @@ protected:
Subprocess::PIPE(),
Subprocess::FD(STDERR_FILENO),
NO_SETSID,
- statistics.flags);
+ &statistics.flags);
CHECK_SOME(s);