----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43932/#review120750 -----------------------------------------------------------
Fix it, then Ship it! Thanks! This is great! src/slave/containerizer/mesos/provisioner/backend.cpp (line 50) <https://reviews.apache.org/r/43932/#comment182221> Kill this line. src/slave/containerizer/mesos/provisioner/backend.cpp (line 52) <https://reviews.apache.org/r/43932/#comment182222> Add a new line above. src/slave/containerizer/mesos/provisioner/backends/overlay.hpp (lines 30 - 31) <https://reviews.apache.org/r/43932/#comment182223> can you wrap comments in 70 char width. It's less jagged IMO. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 28) <https://reviews.apache.org/r/43932/#comment182224> Let's use 'using process::XXX' explicitly. We try to avoid using 'using namespace'. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (lines 124 - 129) <https://reviews.apache.org/r/43932/#comment182293> To be safe, can you do the same thing to mark the mount as slave+shared (like we did in the bind backend). So the goal of doing that is: we want to make sure when slave fork a subprocess with a new mount namespace, it does not create an extra reference to the mount so that rmdir might fail later. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (lines 132 - 133) <https://reviews.apache.org/r/43932/#comment182238> Can you align the error message: ``` return Failure( "Failed to remount rootfs '" + rootfs + "' read-only: " + mount.error()); ``` src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 143) <https://reviews.apache.org/r/43932/#comment182239> Kill this line. src/slave/containerizer/mesos/provisioner/backends/overlay.cpp (line 148) <https://reviews.apache.org/r/43932/#comment182240> You can use mountTable->entries src/tests/containerizer/provisioner_backend_tests.cpp (line 50) <https://reviews.apache.org/r/43932/#comment182297> kill this line. src/tests/containerizer/provisioner_backend_tests.cpp (line 51) <https://reviews.apache.org/r/43932/#comment182306> We should add an TearDown method to unmount anything under sandbox. You can take a look at fs::unmountAll. src/tests/containerizer/provisioner_backend_tests.cpp (lines 78 - 79) <https://reviews.apache.org/r/43932/#comment182302> You can do EXPECT_SOME_EQ("1", read); src/tests/containerizer/provisioner_backend_tests.cpp (lines 83 - 84) <https://reviews.apache.org/r/43932/#comment182303> Ditto. src/tests/containerizer/provisioner_backend_tests.cpp (lines 87 - 88) <https://reviews.apache.org/r/43932/#comment182304> Ditto. src/tests/environment.cpp (line 489) <https://reviews.apache.org/r/43932/#comment182300> `s/_OVERLAYFS_/OVERLAYFS_/` - Jie Yu On Feb. 24, 2016, 4:38 p.m., Shuai Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43932/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2016, 4:38 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-2971 > https://issues.apache.org/jira/browse/MESOS-2971 > > > Repository: mesos > > > Description > ------- > > Added overlayfs provisioning backend. > > > Diffs > ----- > > src/CMakeLists.txt b13fb23219ebb23bcfd6db062e1c814ca2114aa4 > src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5 > src/slave/containerizer/mesos/provisioner/backend.cpp > 01d06ebc67e259272ee57ea5c75bf7077ede65c4 > src/slave/containerizer/mesos/provisioner/backends/overlay.hpp PRE-CREATION > src/slave/containerizer/mesos/provisioner/backends/overlay.cpp PRE-CREATION > src/slave/flags.cpp 1c6a87b670efde2deab4d6e3f24fd6eb3704a47d > src/tests/containerizer/provisioner_backend_tests.cpp > 25b28ef8fa5aae81e8dd0c9e33df4160dd912ce8 > src/tests/environment.cpp 6cd295f76496770774d090e0485ff87be378f74c > > Diff: https://reviews.apache.org/r/43932/diff/ > > > Testing > ------- > > sudo modprobe overlayfs > sudo make check -j4 > GTEST_FILTER='OverlayBackendTest.ROOT_OVERLAYFS_OverlayFSBackend' > > - OS: ubuntu 14.04 64bit vm > - Kernel: 4.2.0-27-generic > > > Thanks, > > Shuai Lin > >
