----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32426/#review77964 -----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126327> How about ``` return getPidFromNamespaceHandle(target.get()); ``` src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126326> s/Do not expect/Not expecting/ src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126328> s/ID/IDs/ src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126337> Instead of (or in addition to) saying what it is used to, can you mention what this map represents? Is it all the containers that have valid veth links, name space handles and container id symlinks? src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126329> s/ids/IDs/ ? to be consistent with #1456 src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126330> s/are/to be/ src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126331> s/handle/handles/ src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126333> Can you also add a bit about how this is possible? Since 'linkers' only contains pids which have veth links, is it possible for two different container ids to have valid veth links at the same time? src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126335> Can you add a bit about why this is OK? src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126338> When do the namespace handles and veth links for these get cleaned up? src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/32426/#comment126339> Shouldn't this be a CHECK? - Vinod Kone On March 26, 2015, 10:56 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32426/ > ----------------------------------------------------------- > > (Updated March 26, 2015, 10:56 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 8fc854e77ef6b5ebb76f25a9c9691f5a0d8c872b > > 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 > >
