----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44706/#review123181 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/cni.hpp (line 85) <https://reviews.apache.org/r/44706/#comment185362> Please see my comment of using a vector instead of map in the previous patches? src/slave/containerizer/mesos/isolators/network/cni.cpp (line 277) <https://reviews.apache.org/r/44706/#comment185363> either s/realpath/`realpath` or s/realpath/location src/slave/containerizer/mesos/isolators/network/cni.cpp (lines 329 - 389) <https://reviews.apache.org/r/44706/#comment185374> Firstly, lambdas should be short. Secondly, as per my other comment with code we will block the isolator. A better strategy would be to move this entire code into a private method, and invoke that function with a `defer`. This would allow you to await on the futures of the subprocess return without blocking the isolator. You can use this code as a reference (https://github.com/apache/mesos/blob/a1a9cd5939d25f82214a5c533bde96a3493f81f3/src/slave/containerizer/mesos/containerizer.cpp#L1318) This uses a static function, but I think you can use a private method and `defer` to the isolator libprocess thread to invoke it. src/slave/containerizer/mesos/isolators/network/cni.cpp (line 341) <https://reviews.apache.org/r/44706/#comment185366> s/, so here we will only get its stdout// src/slave/containerizer/mesos/isolators/network/cni.cpp (line 358) <https://reviews.apache.org/r/44706/#comment185369> If we block here, it will lock the isolator, and prevent the `MesosContainerizer` from launching any more containers? - Avinash sridharan On March 11, 2016, 2:10 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44706/ > ----------------------------------------------------------- > > (Updated March 11, 2016, 2:10 p.m.) > > > Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu. > > > Bugs: MESOS-4759 > https://issues.apache.org/jira/browse/MESOS-4759 > > > Repository: mesos > > > Description > ------- > > Implemented isolate() method of "network/cni" isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/44706/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Qian Zhang > >
