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



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/32426/#comment126112>

    Here in recover, the first thing we look at is the veths on the system to 
determine _all_ candidates for recover or orphan cleanup. This is why veth used 
to be cleaned up as the last step in '_cleanup'.
    
    I think you need to keep that in '_cleanup'. Maybe you can re-adjust the 
creation order in this patch to add veth, touch pid file, mount pid file and 
touch symlink in isolate, and do the reverse in '_cleanup'?



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/32426/#comment126113>

    I have to agree with Vinod that the linker and linkee made it quite hard 
for me to understand too. 
    
    Can we use more explicit names like pidToContainerIdMap (just an example)?



src/slave/containerizer/isolators/network/port_mapping.cpp
<https://reviews.apache.org/r/32426/#comment126115>

    Is this by design that you don't want to do this section in the above for 
loop (over all files from ls)?
    
    Also I think you can achieve this without using _linkers explictly.


- Chi Zhang


On March 25, 2015, 6:43 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32426/
> -----------------------------------------------------------
> 
> (Updated March 25, 2015, 6:43 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, and Vinod Kone.
> 
> 
> Bugs: MESOS-2528
>     https://issues.apache.org/jira/browse/MESOS-2528
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Symlink the namespace handle with ContainerID for the port mapping isolator.
> 
> See ticket for details. This patch will allow a smooth upgrade (i.e., rolling 
> foward and back are both safe).
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> 4dd066a47d43cb1d52f93294d86309151738743e 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 4bf0adeeac1cb6fe59f9c2ca8d5980b1500f5ddd 
>   src/tests/port_mapping_tests.cpp 623840e71938791a562a32989775818275e6d37e 
> 
> Diff: https://reviews.apache.org/r/32426/diff/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> Many existing tests should already capture the regression. For example, 
> ROOT_CleanUpOrphanTest checks if the bind mount dir is empty after the 
> container is destroyed.
> 
> Will add a compatibility test tomorrow.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to