Re: Review Request 48921: Added a check of namespaces in Posix launcher.

2016-06-20 Thread Jie Yu 


> On June 19, 2016, 6:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/launcher.cpp, line 94
> > 
> >
> > Is there a valid case where we would be calling `fork` with `namespaces 
> > = ` ? If not isn't checking namespaces.isSome() enough?

Unfortunately, we always pass in a `namespace` (i.e. Some). We need to change 
that later. I'll add a TODO at the call site.


- Jie


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


On June 19, 2016, 5:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 19, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 48921: Added a check of namespaces in Posix launcher.

2016-06-19 Thread Gilbert Song 

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


Ship it!




Ship It!

- Gilbert Song


On June 18, 2016, 10:10 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 18, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 48921: Added a check of namespaces in Posix launcher.

2016-06-19 Thread Avinash sridharan 

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




src/slave/containerizer/mesos/launcher.cpp (line 94)


Typo: 
Meant to ask "Is there a valid case where we would be calling `fork` with 
`namespaces = 0` ?


- Avinash sridharan


On June 19, 2016, 5:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 19, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 48921: Added a check of namespaces in Posix launcher.

2016-06-19 Thread Avinash sridharan 

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




src/slave/containerizer/mesos/launcher.cpp (line 94)


Is there a valid case where we would be calling `fork` with `namespaces = ` 
? If not isn't checking namespaces.isSome() enough?


- Avinash sridharan


On June 19, 2016, 5:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 19, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 48921: Added a check of namespaces in Posix launcher.

2016-06-19 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [48921]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On June 19, 2016, 5:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 19, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 48921: Added a check of namespaces in Posix launcher.

2016-06-18 Thread haosdent huang 

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


Ship it!




Ship It!

- haosdent huang


On June 19, 2016, 5:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48921/
> ---
> 
> (Updated June 19, 2016, 5:10 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, 
> and Kapil Arya.
> 
> 
> Bugs: MESOS-5533
> https://issues.apache.org/jira/browse/MESOS-5533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Posix launcher does not support namespaces. We should reject the launch
> if namespaces are desired (e.g., from isolators). The currently behavior
> of silently ignoring it will cause weird issues.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launcher.cpp 
> b1b323b4040fd72a7f01e60781eaf992e4ef766a 
> 
> Diff: https://reviews.apache.org/r/48921/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 48921: Added a check of namespaces in Posix launcher.

2016-06-18 Thread Jie Yu 

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

Review request for mesos, Avinash sridharan, Gilbert Song, haosdent huang, and 
Kapil Arya.


Bugs: MESOS-5533
https://issues.apache.org/jira/browse/MESOS-5533


Repository: mesos


Description
---

Posix launcher does not support namespaces. We should reject the launch
if namespaces are desired (e.g., from isolators). The currently behavior
of silently ignoring it will cause weird issues.


Diffs
-

  src/slave/containerizer/mesos/launcher.cpp 
b1b323b4040fd72a7f01e60781eaf992e4ef766a 

Diff: https://reviews.apache.org/r/48921/diff/


Testing
---

make check


Thanks,

Jie Yu