-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44706/#review125825
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 353)
<https://reviews.apache.org/r/44706/#comment188699>

    Should we return a Failure here  instead?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 395)
<https://reviews.apache.org/r/44706/#comment188702>

    There's a problem with the current code. 'collect' (in isolate()) on 
'attach' will fail if any of the call to the plugin (ADD) fails. And we'll call 
'cleanup' right after that, it's likely that another call to the plugin (ADD) 
is still pending. I don't think calling DEL while an ADD is still pending is 
gonna work.
    
    Therefore, I think we should use 'await' in isolate instead of 'collect' so 
that we can make sure all of them are in termimal state before we return the 
result.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 450)
<https://reviews.apache.org/r/44706/#comment188698>

    You want to read 'stderr' as well.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 525)
<https://reviews.apache.org/r/44706/#comment188701>

    Could you please add a comment about the fact that destroy cannot happen in 
the middle of attach and `_attach` because the containerizer will wait for 
isolate to finish before destroying the container.
    
    Also, add a CHECK(infos.container(containerId));


- Jie Yu


On March 26, 2016, 2:53 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> -----------------------------------------------------------
> 
> (Updated March 26, 2016, 2:53 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/CMakeLists.txt 7bda2ac684f38da94e334f0cef843614687ae4aa 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 7cda5715814a0cfc4b394eb04437831e6dc44e3f 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44706/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Reply via email to