----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52097/#review149888 -----------------------------------------------------------
You have odd spacing throughout your patch. We use 2 spaces per indent. There are some additional caveats here: http://mesos.apache.org/documentation/latest/c++-style-guide/ but your patch mostly does not run into any special (indenting) cases. src/slave/container_loggers/logrotate.cpp (lines 148 - 154) <https://reviews.apache.org/r/52097/#comment217647> It makes sense to do this in the `main` function, after we do `setsid`. src/slave/container_loggers/logrotate.cpp (lines 198 - 202) <https://reviews.apache.org/r/52097/#comment217649> This comment is not relevant here, so you can remove it. From the `mesos/launch.cpp` code (that this comment was taken from), there is a chunk of code between reading the UID/GID's and changing the UID/GID's. That code potentially changes the subprocess's view of `/etc`. src/slave/container_loggers/logrotate.cpp (lines 207 - 210) <https://reviews.apache.org/r/52097/#comment217651> Similar not-relevant comment here. The container logger's companion executable doesn't run in a container (currently), so it does not have certain considerations. src/slave/container_loggers/logrotate.cpp (lines 324 - 330) <https://reviews.apache.org/r/52097/#comment217650> It would be appropriate to change the user after this block. - Joseph Wu On Sept. 20, 2016, 8:54 p.m., Sivaram Kannan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52097/ > ----------------------------------------------------------- > > (Updated Sept. 20, 2016, 8:54 p.m.) > > > Review request for mesos and Joseph Wu. > > > Bugs: MESOS-5856 > https://issues.apache.org/jira/browse/MESOS-5856 > > > Repository: mesos > > > Description > ------- > > Functionality to switch user when executor launches as non-root user. > > > Diffs > ----- > > src/slave/container_loggers/logrotate.cpp > 431bc3cbb54e94359078e4dae0b32ad301393640 > > Diff: https://reviews.apache.org/r/52097/diff/ > > > Testing > ------- > > > Thanks, > > Sivaram Kannan > >
