----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53369/#review167767 -----------------------------------------------------------
Fix it, then Ship it! The suggested snippet should fix the reviewbot error. src/slave/main.cpp Lines 170 (patched) <https://reviews.apache.org/r/53369/#comment239732> Nit: make it `static`? ``` static Try<Nothing> assignCgroups(const ::Flags& flags) ``` src/slave/main.cpp Lines 384-387 (patched) <https://reviews.apache.org/r/53369/#comment239731> I feel that I can understand the flow better without looking at the implementation of `assignCgroups` if it's enclosed by `if (flags.agent_subsystems.isSome())`. ``` if (flags.agent_subsystems.isSome()) { Try<Nothing> assign = assignCgroups(flags); if (assign.isError()) { EXIT(EXIT_FAILURE) << assign.error()(); } } ``` This along with "if necessary" in the comments makes it more clear this is a conditional thing. ``` // Move the agent process into its own cgroup for each of the specified // subsystems if necessary before the process is initialized. ``` Within the method we just need to do a CHECK_SOME. ``` static Try<Nothing> assignCgroups(const ::Flags& flags) { CHECK_SOME(flags.agent_subsystems); ... } ``` - Jiang Yan Xu On March 2, 2017, 12:36 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53369/ > ----------------------------------------------------------- > > (Updated March 2, 2017, 12:36 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-6523 > https://issues.apache.org/jira/browse/MESOS-6523 > > > Repository: mesos > > > Description > ------- > > This is to ensure that we do not accept traffic on the agent by opening > up libprocess port only after cgroup assigment for the agent is done. > > > Diffs > ----- > > src/slave/main.cpp a124d2e0cfa3e39e2400183f9523486196b9816d > src/slave/slave.cpp 6ae9458cc81a7623a1837cd636156434a972004b > > > Diff: https://reviews.apache.org/r/53369/diff/4/ > > > Testing > ------- > > All tests passed. > > > Thanks, > > Anindya Sinha > >
