----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42048/#review114617 -----------------------------------------------------------
Ship it! I would rename the summary to be "Implemented the cgroup net_cls isolator." src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 40 - 41) <https://reviews.apache.org/r/42048/#comment175473> Please sort them according to alphebet order. src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 210) <https://reviews.apache.org/r/42048/#comment175609> No need to add 'Failed to prepare isolator', the caller should have already printed it. ``` return Failure("Failed to check if the cgroup already exists"); ``` src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 212) <https://reviews.apache.org/r/42048/#comment175610> Ditto. ``` return Failure("The cgroup already exists"); ``` src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 217) <https://reviews.apache.org/r/42048/#comment175611> Ditto. ``` return Failure("Failded to create the cgroup: " + create.error()); ``` src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 228) <https://reviews.apache.org/r/42048/#comment175474> Please insert a blank line above. We usually insert a line if the previous statement is a multi-line statement. src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 257) <https://reviews.apache.org/r/42048/#comment175612> Remove the space between `'` and `:` src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 291 - 316) <https://reviews.apache.org/r/42048/#comment175613> First of all, the return value of `_cleanup` does not make sense because onAny accepts a 'void' function. That return value won't be used by anyone. I think we can still keep 'info' for that container if destroy failed because the cgroup is still there. So you can do the following here: ``` return cgroups::destroy(...) .then(defer([=]() { infos.erase(containerId); return Nothing(); }); ``` - Jie Yu On Jan. 15, 2016, 7:06 p.m., Avinash sridharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42048/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2016, 7:06 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4262 > https://issues.apache.org/jira/browse/MESOS-4262 > > > Repository: mesos > > > Description > ------- > > Defined the CgroupNetClsIsolatorProcess. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/42048/diff/ > > > Testing > ------- > > make check . Added a test case for CgroupNetClsIsolatorProcess. > > > Thanks, > > Avinash sridharan > >
