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 2017/02/09 00:19:31 UTC

[1/2] mesos git commit: Fixed error handling when writing to a cgroup control file.

Repository: mesos
Updated Branches:
  refs/heads/master dfe53db6b -> 973abd043


Fixed error handling when writing to a cgroup control file.

Due to internal buffering in ofstream, actual write to the underlying
device may not be performed by the time of checking for failures.
This patch flushes written data to ensure fail check uses actual value.

Review: https://reviews.apache.org/r/56080/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/78614f52
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/78614f52
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/78614f52

Branch: refs/heads/master
Commit: 78614f52e932a2f638d3da0b627c8e81772e3930
Parents: dfe53db
Author: Dmitry Zhuk <dz...@twopensource.com>
Authored: Wed Feb 8 15:42:51 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Feb 8 15:42:51 2017 -0800

----------------------------------------------------------------------
 src/linux/cgroups.cpp                     | 2 +-
 src/tests/containerizer/cgroups_tests.cpp | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/78614f52/src/linux/cgroups.cpp
----------------------------------------------------------------------
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index 630e2ec..6e39010 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -395,7 +395,7 @@ static Try<Nothing> write(
   // NOTE: cgroups convention does not append a endln!
   // Recent kernels will cause operations to fail if 'endl' is
   // appended to the control file.
-  file << value;
+  file << value << std::flush;
 
   if (file.fail()) {
     // TODO(jieyu): Does ofstream actually set errno?

http://git-wip-us.apache.org/repos/asf/mesos/blob/78614f52/src/tests/containerizer/cgroups_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/cgroups_tests.cpp b/src/tests/containerizer/cgroups_tests.cpp
index af8a201..76fabce 100644
--- a/src/tests/containerizer/cgroups_tests.cpp
+++ b/src/tests/containerizer/cgroups_tests.cpp
@@ -471,6 +471,8 @@ TEST_F(CgroupsAnyHierarchyTest, ROOT_CGROUPS_Write)
       cgroups::write(hierarchy, TEST_CGROUPS_ROOT, "invalid", "invalid"));
 
   ASSERT_SOME(cgroups::create(hierarchy, TEST_CGROUPS_ROOT));
+  EXPECT_ERROR(
+      cgroups::write(hierarchy, TEST_CGROUPS_ROOT, "cpu.shares", "invalid"));
 
   pid_t pid = ::fork();
   ASSERT_NE(-1, pid);


[2/2] mesos git commit: Simplified cgroups control files reading and writing.

Posted by ji...@apache.org.
Simplified cgroups control files reading and writing.

Review: https://reviews.apache.org/r/56122/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/973abd04
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/973abd04
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/973abd04

Branch: refs/heads/master
Commit: 973abd043701c06203060a4fafc723bbc002ee6a
Parents: 78614f5
Author: Dmitry Zhuk <dz...@twopensource.com>
Authored: Wed Feb 8 15:43:01 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Feb 8 15:43:01 2017 -0800

----------------------------------------------------------------------
 src/linux/cgroups.cpp | 38 ++------------------------------------
 1 file changed, 2 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/973abd04/src/linux/cgroups.cpp
----------------------------------------------------------------------
diff --git a/src/linux/cgroups.cpp b/src/linux/cgroups.cpp
index 6e39010..3a85329 100644
--- a/src/linux/cgroups.cpp
+++ b/src/linux/cgroups.cpp
@@ -350,25 +350,7 @@ static Try<string> read(
     const string& control)
 {
   string path = path::join(hierarchy, cgroup, control);
-
-  // TODO(benh): Use os::read. Note that we do not use os::read
-  // currently because it cannot correctly read /proc or cgroups
-  // control files since lseek (in os::read) will return error.
-  ifstream file(path.c_str());
-
-  if (!file.is_open()) {
-    return Error("Failed to open file " + path);
-  }
-
-  ostringstream ss;
-  ss << file.rdbuf();
-
-  if (file.fail()) {
-    // TODO(jieyu): Does ifstream actually set errno?
-    return ErrnoError();
-  }
-
-  return ss.str();
+  return os::read(path);
 }
 
 
@@ -386,23 +368,7 @@ static Try<Nothing> write(
     const string& value)
 {
   string path = path::join(hierarchy, cgroup, control);
-  ofstream file(path.c_str());
-
-  if (!file.is_open()) {
-    return Error("Failed to open file " + path);
-  }
-
-  // NOTE: cgroups convention does not append a endln!
-  // Recent kernels will cause operations to fail if 'endl' is
-  // appended to the control file.
-  file << value << std::flush;
-
-  if (file.fail()) {
-    // TODO(jieyu): Does ofstream actually set errno?
-    return ErrnoError();
-  }
-
-  return Nothing();
+  return os::write(path, value);
 }
 
 } // namespace internal {