This is an automatically generated e-mail. To reply, visit:

src/slave/containerizer/docker.cpp (line 396)

    Looks like we use 'directory' consistently in this file for container 
working directory. So please

src/slave/containerizer/docker.cpp (line 397)


src/slave/containerizer/docker.cpp (line 398)


src/slave/containerizer/docker.cpp (lines 420 - 424)

    You may want to make sure `slave.work_dir` is a shared mount in its own 
peer group (see code in `LinuxFilesystemIsolatorProcess::create` and also 
ticket https://issues.apache.org/jira/browse/MESOS-3483).
    The reason we need to do that is because when we fork an executor process 
(in Mesos containerizer) with a new mount namespace, if the mount is a private 
mount, the mount in the new mount namespace will be a private mount as well, 
holding an extra reference to the mount. That mount can be for other 
containers. When the slave remove the mount followed by an rmdir (removing the 
mount point), the rmdir could fail due to the extra references to the mount.
    Although we did some best effort umount in the new mount namespace (trying 
to umount any irrelevant mounts), but race still exists. For instance, while 
we're umounting, the slave tries to umount+rmdir for some mounts.

src/slave/containerizer/docker.cpp (lines 512 - 513)

    Returning a failure doesn't match the existing semantics. Could you instead 
use a LOG(ERROR) here?

src/slave/containerizer/docker.cpp (line 518)

    Hum, can you make 'updatePersistentVolumes' a member function so that you 
have access to 'flags' and do not need to pass in this parameter.

src/slave/containerizer/docker.cpp (line 522)

    Insert a blank line above.

src/slave/containerizer/docker.cpp (lines 1281 - 1289)

    This is useless right now, isn't it? Can you remove it and instead, drop a 
TODO here. To fully support updating volumes, we need to have docker's mount 
propagation support.

src/slave/containerizer/docker.cpp (lines 1707 - 1719)

    Do you need this? Looks like 'unmountPersistentVolumes' will take care of 
the umount.

- Jie Yu

On Feb. 15, 2016, 3:39 p.m., Timothy Chen wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> -----------------------------------------------------------
> (Updated Feb. 15, 2016, 3:39 p.m.)
> Review request for mesos and Jie Yu.
> Bugs: MESOS-3413
>     https://issues.apache.org/jira/browse/MESOS-3413
> Repository: mesos
> Description
> -------
> Fixed persistent volumes with docker tasks.
> Diffs
> -----
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> Diff: https://reviews.apache.org/r/43015/diff/
> Testing
> -------
> make check
> Thanks,
> Timothy Chen

Reply via email to