-----------------------------------------------------------
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
> 
>

Reply via email to