----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52285/#review150491 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp (line 102) <https://reviews.apache.org/r/52285/#comment218475> Suggestion: ``` Can only prepare the sandbox volume isolator for a MESOS container ``` src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp (line 106) <https://reviews.apache.org/r/52285/#comment218476> Suggestion: ``` The 'linux' launcher and 'filesystem/linux' isolator must be enabled to change the rootfs and bind mount. ``` src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp (lines 131 - 134) <https://reviews.apache.org/r/52285/#comment218477> Shouldn't this have a TODO? (For mounting the task's sandbox into arbitrary locations of the task's mount namespace.) src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp (lines 159 - 164) <https://reviews.apache.org/r/52285/#comment218479> The comment on `container_path` in the `Volume` proto says: > If the path is an absolute path, that path must already exist. Perhaps we should amend this comment? (Because `os::mkdir` doesn't error if the target directory already exists.) I see that the `!has_rootfs()` code path checks for existence. src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp (lines 176 - 183) <https://reviews.apache.org/r/52285/#comment218480> Is upwards traversal allowed here? (Should we validate?) - Joseph Wu On Sept. 26, 2016, 4:16 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52285/ > ----------------------------------------------------------- > > (Updated Sept. 26, 2016, 4:16 p.m.) > > > Review request for mesos, Gilbert Song and Joseph Wu. > > > Bugs: MESOS-6258 > https://issues.apache.org/jira/browse/MESOS-6258 > > > Repository: mesos > > > Description > ------- > > Added 'volume/sandbox_path' isolator. > > > Diffs > ----- > > src/CMakeLists.txt 981b36e55785b9de00a79693d5b25fe66fccb710 > src/Makefile.am 410b5f2e5bc50a5aa7856645c8cf4a3e43505a84 > src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp > PRE-CREATION > src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp > PRE-CREATION > > Diff: https://reviews.apache.org/r/52285/diff/ > > > Testing > ------- > > To be added. Wait for the nested container support. > > > Thanks, > > Jie Yu > >
