> On Jan. 23, 2014, 7:32 p.m., Niklas Nielsen wrote: > > Again, good stuff :) A few easy style nits/questions.
Hi Niklas, Thanks for taking the time to review this. A new version has been pushed up and will be incorporated into Ian's branch and reflected here. I can't close issues here since I am not the original reporter. In summary: - all line-wraps and indentation comments are fixed. I also did another run to capture similar issues in the code. - The 3 LOG(FATAL)s in cgroups_helper::setup are ok because they are in the initialization path of an isolator. We can't do much after that, for example, if the memcg mount point couldn't be set up. I fixed the ones in ::prepare/::oomListen to LOG(ERROR) since an error in dealing with a container shouldn't halt the system as you pointed out. - constants.[h/c]pp have global scope while those constants are only used in their respective files, so I thought it could be okay to leave them in the implementation files, at least for now? - I believe I've fixed all other small issues. - Chi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16432/#review32647 ----------------------------------------------------------- On Jan. 21, 2014, 7:35 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16432/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2014, 7:35 p.m.) > > > Review request for mesos, Benjamin Hindman, Chi Zhang, samya, and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Forth part of isolation refactor - cgroup isolators > > CPU and Memory isolators using Linux cgroups. > > This code was written by Chi Zhang <[email protected]> and based on the > original cgroups_isolator. > > > Diffs > ----- > > src/slave/container/isolators/cgroups/cgroups.hpp PRE-CREATION > src/slave/container/isolators/cgroups/cgroups.cpp PRE-CREATION > src/slave/container/isolators/cgroups/cpushare.hpp PRE-CREATION > src/slave/container/isolators/cgroups/cpushare.cpp PRE-CREATION > src/slave/container/isolators/cgroups/mem.hpp PRE-CREATION > src/slave/container/isolators/cgroups/mem.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/16432/diff/ > > > Testing > ------- > > > Thanks, > > Ian Downes > >
