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

Ship it!



src/slave/containerizer/isolators/filesystem/linux.cpp (lines 721 - 723)
<https://reviews.apache.org/r/38333/#comment155489>

    You are logging the sandbox as "working directory" and "sandbox" 
respectively for the two (w/ new rootfs and w/o) cases in `prepare()`, should 
we aim for symmetry here when in `cleanup()`?
    
    Or we can say `<< "Unmounting sandbox/work directory '" <<`?



src/slave/containerizer/isolators/filesystem/linux.cpp (line 728)
<https://reviews.apache.org/r/38333/#comment155491>

    Ditto about the name "sandbox" here.



src/slave/containerizer/provisioners/appc/provisioner.cpp (lines 368 - 369)
<https://reviews.apache.org/r/38333/#comment155494>

    Oh...
    
    I thought `root` was being copied but I guess the compilers says otherwise?



src/slave/containerizer/provisioners/appc/provisioner.cpp (line 379)
<https://reviews.apache.org/r/38333/#comment155495>

    Adjust the expected likelyhood a bit by s/likely/possible/?



src/slave/containerizer/provisioners/appc/provisioner.cpp (line 380)
<https://reviews.apache.org/r/38333/#comment155529>

    Expand to explain the reason for EBUSY?
    
    ```
    EBUSY because of the race between cleaning up this container and new 
containers copying the host mount table
    ```
    
    It's not immediately obvious without some comments.



src/slave/containerizer/provisioners/backends/bind.cpp (line 169)
<https://reviews.apache.org/r/38333/#comment155496>

    s/returns/return/.


- Jiang Yan Xu


On Sept. 12, 2015, 11:33 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38333/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2015, 11:33 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Timothy Chen, Vinod Kone, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-3349
>     https://issues.apache.org/jira/browse/MESOS-3349
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Made container sandbox a shared mount to address MESOS-3349.
> 
> See the discussion in https://reviews.apache.org/r/38329/ for more context.
> 
> The idea is to mark container sandbox a shared mount (do a self bind mount 
> first) so that persistent volume mounts can be propagated.
> 
> This is less invasive than marking '/' as a shared mount.
> 
> One followup for this patch is to set the default filesystem isolator to 
> posix as the linux isolator will manipulate host mount table.
> 
> We also need to address the TODO in `LinuxFilesystemIsolator::_recover` so 
> that tests do not leak mounts in the host mount table.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 0970b3d48b13d5e9d2e0160df5cf14a3dcd0acc9 
>   src/slave/containerizer/provisioners/appc/provisioner.cpp 
> cd29a00fa0db8af294c10bb7a2e0cb4252bd2993 
>   src/slave/containerizer/provisioners/backends/bind.cpp 
> 1cdae61786790dc6a475ae5f73c8cc92d2bbf739 
> 
> Diff: https://reviews.apache.org/r/38333/diff/
> 
> 
> Testing
> -------
> 
> sudo make check on Centos5 and Centos6
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to