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



src/slave/cgroups_isolator.hpp
<https://reviews.apache.org/r/13038/#comment48213>

    Hmm. I'm not sure I like this struct. For one some of the variables are in 
(or should be in) CgroupInfo. More importantly, with clone cant the child 
process simply access this data from the shared memory? Was there a reason you 
explicitly passed it via a struct?



src/slave/cgroups_isolator.cpp
<https://reviews.apache.org/r/13038/#comment48212>

    why 16384?



src/slave/cgroups_isolator.cpp
<https://reviews.apache.org/r/13038/#comment48132>

    any particular reason you pulled out executorId into a variable?



src/slave/cgroups_isolator.cpp
<https://reviews.apache.org/r/13038/#comment48131>

    new line.


- Vinod Kone


On July 29, 2013, 10:22 p.m., Eric Biederman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13038/
> -----------------------------------------------------------
> 
> (Updated July 29, 2013, 10:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, and Vinod 
> Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Use clone instead of fork to create the executor process
> 
> In preparation for taking advantage of the linux namespaces replace the fork
> before the exec of the executor with a call to clone.
> 
> For the moment this preserves the existing behavior exactly, but this provides
> the opportunity for creating namespaces to isolate the executor with at the
> when the process for the executor is created.
> 
> clone(3) requires an extra stack and for the code to run in the child
> to be passed as a function.  I would prefer to build a wrapper of
> clone that doesn't require these unneeded shenanigans but glibc caches
> the processes current pid, and only invalidates the cache if you use
> one of glibc's wrappers of clone, fork and vfork. Sigh.
> 
> 
> Diffs
> -----
> 
>   src/slave/cgroups_isolator.hpp e86062e 
>   src/slave/cgroups_isolator.cpp 0faf7d5 
> 
> Diff: https://reviews.apache.org/r/13038/diff/
> 
> 
> Testing
> -------
> 
> make -j8 check
> 
> And all of the tests pass.
> 
> 
> Thanks,
> 
> Eric Biederman
> 
>

Reply via email to