You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/04/29 01:44:13 UTC
Review Request 20816: Extend the Linux launcher to use clone.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20816/
-----------------------------------------------------------
Review request for mesos, Chi Zhang, Jie Yu, and Vinod Kone.
Summary (updated)
-----------------
Extend the Linux launcher to use clone.
Repository: mesos-git
Description (updated)
-------
The launcher can now support clone'ing processes with namespace flags.
Diffs (updated)
-----
src/slave/containerizer/linux_launcher.hpp 8f96c6996e0d06c37c29233f5e02f002db743dfb
src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
Diff: https://reviews.apache.org/r/20816/diff/
Testing (updated)
-------
make check
Thanks,
Ian Downes
Re: Review Request 20816: Extend the Linux launcher to use clone.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20816/#review41770
-----------------------------------------------------------
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75362>
static int _call?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75354>
Kill the extra space before '>':
static_cast<const lambda::function<int()>*>
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75364>
static?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75368>
Se we are moving away from process group now? Comments would be nice.
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75358>
Check cgroups existence failed ...?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75361>
Do you need this LOG here? Will the caller of this function print the same message?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75360>
Failed to create freezer cgroup ...?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75366>
Can you move this down:
pid_t pid = ::clone(...);
if (pid == -1) {
...
}
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75369>
_call is a little confusing. Maybe call it childMain, or something similar?
Also, it does not read well since we have:
childFunction
blockedChildFunction
child
func,
_call
What about the following:
static int childMain(void* args)
{
const lambda::function<int()>* func = (...) args;
(*func)();
}
static int _childMain(
const lambda::function<int()>& childFunction,
int pipes[2]);
lambda::function<> func = lambda::bind(&_childMain, childFunction, pipes);
::clone(
childMain,
...
static_cast<void*>(&func))
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/20816/#comment75367>
Add a comment about SIGCHLD here
- Jie Yu
On April 29, 2014, 6:25 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20816/
> -----------------------------------------------------------
>
> (Updated April 29, 2014, 6:25 p.m.)
>
>
> Review request for mesos, Chi Zhang, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The launcher can now support clone'ing processes with namespace flags.
>
>
> Diffs
> -----
>
> src/slave/containerizer/linux_launcher.hpp 8f96c6996e0d06c37c29233f5e02f002db743dfb
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
>
> Diff: https://reviews.apache.org/r/20816/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20816: Extend the Linux launcher to use clone.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20816/
-----------------------------------------------------------
(Updated May 1, 2014, 10:06 a.m.)
Review request for mesos, Chi Zhang, Jie Yu, and Vinod Kone.
Changes
-------
Rebased.
Repository: mesos-git
Description
-------
The launcher can now support clone'ing processes with namespace flags.
Diffs (updated)
-----
src/slave/containerizer/linux_launcher.hpp 8f96c6996e0d06c37c29233f5e02f002db743dfb
src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
Diff: https://reviews.apache.org/r/20816/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 20816: Extend the Linux launcher to use clone.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20816/#review41874
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On April 29, 2014, 11:18 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20816/
> -----------------------------------------------------------
>
> (Updated April 29, 2014, 11:18 p.m.)
>
>
> Review request for mesos, Chi Zhang, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The launcher can now support clone'ing processes with namespace flags.
>
>
> Diffs
> -----
>
> src/slave/containerizer/linux_launcher.hpp 8f96c6996e0d06c37c29233f5e02f002db743dfb
> src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
>
> Diff: https://reviews.apache.org/r/20816/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20816: Extend the Linux launcher to use clone.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20816/
-----------------------------------------------------------
(Updated April 29, 2014, 4:18 p.m.)
Review request for mesos, Chi Zhang, Jie Yu, and Vinod Kone.
Changes
-------
Addressed Jie's comments.
Repository: mesos-git
Description
-------
The launcher can now support clone'ing processes with namespace flags.
Diffs (updated)
-----
src/slave/containerizer/linux_launcher.hpp 8f96c6996e0d06c37c29233f5e02f002db743dfb
src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
Diff: https://reviews.apache.org/r/20816/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 20816: Extend the Linux launcher to use clone.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20816/
-----------------------------------------------------------
(Updated April 29, 2014, 11:25 a.m.)
Review request for mesos, Chi Zhang, Jie Yu, and Vinod Kone.
Changes
-------
Rebased. File rename patch has been committed so diffs are working now.
Repository: mesos-git
Description
-------
The launcher can now support clone'ing processes with namespace flags.
Diffs (updated)
-----
src/slave/containerizer/linux_launcher.hpp 8f96c6996e0d06c37c29233f5e02f002db743dfb
src/slave/containerizer/linux_launcher.cpp 530e0bd64d71bad761a2eab3d6e2f2179a167b4b
Diff: https://reviews.apache.org/r/20816/diff/
Testing
-------
make check
Thanks,
Ian Downes