----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53161/#review154050 -----------------------------------------------------------
Fix it, then Ship it! src/slave/containerizer/mesos/utils.hpp (line 34) <https://reviews.apache.org/r/53161/#comment223564> I think this method should be moved to `src/slave/containerizer/mesos/provisioner/utils.hpp|cpp` since this is provision specific. THe top level utils file is used for general utils used by Mesos containerizer. src/slave/containerizer/mesos/utils.cpp (line 52) <https://reviews.apache.org/r/53161/#comment223566> I'd rename 'source' to 'directory'. src/slave/containerizer/mesos/utils.cpp (line 55) <https://reviews.apache.org/r/53161/#comment223570> i'd prefer a new line above src/slave/containerizer/mesos/utils.cpp (lines 62 - 65) <https://reviews.apache.org/r/53161/#comment223571> Can we do the following to reduce the level of nesting: ``` if (node->fts_info != FTS_F) { continue; } if (!strings::startsWith(node->fts_name, docker::spec::WHITEOUT_PREFIX)) { continue; } ... ``` src/slave/containerizer/mesos/utils.cpp (line 65) <https://reviews.apache.org/r/53161/#comment223579> Do you need `string` here? src/slave/containerizer/mesos/utils.cpp (lines 68 - 69) <https://reviews.apache.org/r/53161/#comment223582> I'd format like the following: ``` Try<Nothing> setxattr = os::setxattr( path.dirname(), "trusted.overlay.opaque", "y", 0); ``` src/slave/containerizer/mesos/utils.cpp (line 74) <https://reviews.apache.org/r/53161/#comment223583> Why backslash? src/slave/containerizer/mesos/utils.cpp (lines 92 - 99) <https://reviews.apache.org/r/53161/#comment223584> Can you double check if it is safe to remove files while walking the fts tree? To be more defensive, i would probably remove them after finishing the walk. - Jie Yu On Oct. 25, 2016, 3:46 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53161/ > ----------------------------------------------------------- > > (Updated Oct. 25, 2016, 3:46 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6360 > https://issues.apache.org/jira/browse/MESOS-6360 > > > Repository: mesos > > > Description > ------- > > Implemented the conversion from AUFS whiteouts to OverlayFS whiteouts. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/docker/store.cpp > e192f86a1848b373f3aa73d29688a96375cac313 > src/slave/containerizer/mesos/utils.hpp > 178ebf3effac824e4788d7282795c18dc1cb5265 > src/slave/containerizer/mesos/utils.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/53161/diff/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >
