You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2014/06/02 22:49:08 UTC
Review Request 22160: Added setns utilities to stout.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/
-----------------------------------------------------------
Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/22160/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 22160: Added setns utilities to stout.
Posted by Jie Yu <yu...@gmail.com>.
> On June 2, 2014, 9:18 p.m., Timothy St. Clair wrote:
> > I'm quite curious to know the scope of application at a higher layer. Could you please associate the JIRA for tracking.
Just added it.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44552
-----------------------------------------------------------
On June 2, 2014, 10:38 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 10:38 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by "Timothy St. Clair" <ts...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44552
-----------------------------------------------------------
I'm quite curious to know the scope of application at a higher layer. Could you please associate the JIRA for tracking.
- Timothy St. Clair
On June 2, 2014, 8:49 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 8:49 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Jie Yu <yu...@gmail.com>.
> On June 2, 2014, 11:12 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp, line 69
> > <https://reviews.apache.org/r/22160/diff/2/?file=601976#file601976line69>
> >
> > can you comment on what is "308". is there a constant that you can use?
Added comment. If the constant is available, I will use the constant (see the above two #if). This is a workaround for a host that has a very old glibc but has a relatively new kernel.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44571
-----------------------------------------------------------
On June 2, 2014, 10:38 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 10:38 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44571
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment78985>
quotes around path.
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment78986>
can you comment on what is "308". is there a constant that you can use?
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment78987>
s/the namespace/namespace/
- Vinod Kone
On June 2, 2014, 10:38 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 10:38 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Jie Yu <yu...@gmail.com>.
> On June 4, 2014, 6:47 p.m., Ian Downes wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp, line 41
> > <https://reviews.apache.org/r/22160/diff/3/?file=602054#file602054line41>
> >
> > This is true for all namespaces *except* PID namespaces where setns() means that *children* of the re-associated thread will be in the specified namespace.
Disabled PID namespace for this function for now since its semantics is different from others.
> On June 4, 2014, 6:47 p.m., Ian Downes wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp, line 64
> > <https://reviews.apache.org/r/22160/diff/3/?file=602054#file602054line64>
> >
> > Why are we always using syscalls directly and not setns(2) when it's available?
> >
> > When would __NR_setns be defined and not SYS_setns?
There is no way that I can check if setns exists or not. Removed __NR_setns check.
> On June 4, 2014, 6:47 p.m., Ian Downes wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp, line 71
> > <https://reviews.apache.org/r/22160/diff/3/?file=602054#file602054line71>
> >
> > What happens on 32 bit systems?
The code won't compile. I think it's ok for now. If we do have a case for x86_32, we can add it later.
> On June 4, 2014, 6:47 p.m., Ian Downes wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp, line 69
> > <https://reviews.apache.org/r/22160/diff/3/?file=602054#file602054line69>
> >
> > Can you comment on which versions of glibc are old?
Added comments.
> On June 4, 2014, 6:47 p.m., Ian Downes wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp, line 88
> > <https://reviews.apache.org/r/22160/diff/3/?file=602054#file602054line88>
> >
> > What does it mean to associate a *thread* to another pid's namespace? This implies different threads in a process could be in different namespaces so which one do you associate to?
> >
> > Perhaps we only support associating all threads of the process to the target namespace.
Added a check so that if a process has multiple threads, this function will fail.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44736
-----------------------------------------------------------
On June 2, 2014, 11:20 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 11:20 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44736
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment79234>
This is true for all namespaces *except* PID namespaces where setns() means that *children* of the re-associated thread will be in the specified namespace.
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment79238>
Why are we always using syscalls directly and not setns(2) when it's available?
When would __NR_setns be defined and not SYS_setns?
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment79245>
Can you comment on which versions of glibc are old?
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment79236>
What happens on 32 bit systems?
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment79240>
Add a comment on why this is ordered like this?
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment79244>
What does it mean to associate a *thread* to another pid's namespace? This implies different threads in a process could be in different namespaces so which one do you associate to?
Perhaps we only support associating all threads of the process to the target namespace.
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp
<https://reviews.apache.org/r/22160/#comment79242>
What about checking os::exists(pid) first to see if the pid is running?
3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp
<https://reviews.apache.org/r/22160/#comment79243>
Why not test the other namespaces - NEWMNT, NEWPID and NEWUSER?
- Ian Downes
On June 2, 2014, 4:20 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 4:20 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44682
-----------------------------------------------------------
Patch looks great!
Reviews applied: [22160]
All tests passed.
- Mesos ReviewBot
On June 2, 2014, 11:20 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 11:20 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44882
-----------------------------------------------------------
Patch looks great!
Reviews applied: [22160]
All tests passed.
- Mesos ReviewBot
On June 5, 2014, 4:33 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 5, 2014, 4:33 a.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/#review44923
-----------------------------------------------------------
Ship it!
Ship It!
- Ian Downes
On June 4, 2014, 9:33 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22160/
> -----------------------------------------------------------
>
> (Updated June 4, 2014, 9:33 p.m.)
>
>
> Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
>
>
> Bugs: MESOS-1450
> https://issues.apache.org/jira/browse/MESOS-1450
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22160/diff/
>
>
> Testing
> -------
>
> make check
> sudo make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 22160: Added setns utilities to stout.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/
-----------------------------------------------------------
(Updated June 5, 2014, 4:33 a.m.)
Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
Changes
-------
Ian's comments.
Bugs: MESOS-1450
https://issues.apache.org/jira/browse/MESOS-1450
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/22160/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 22160: Added setns utilities to stout.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/
-----------------------------------------------------------
(Updated June 2, 2014, 11:20 p.m.)
Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
Changes
-------
Vinod's comments.
Bugs: MESOS-1450
https://issues.apache.org/jira/browse/MESOS-1450
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/22160/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu
Re: Review Request 22160: Added setns utilities to stout.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22160/
-----------------------------------------------------------
(Updated June 2, 2014, 10:38 p.m.)
Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone.
Changes
-------
Added "Bugs".
Bugs: MESOS-1450
https://issues.apache.org/jira/browse/MESOS-1450
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/3rdparty/Makefile.am 1474bbc
3rdparty/libprocess/3rdparty/stout/Makefile.am 8f32a66
3rdparty/libprocess/3rdparty/stout/include/stout/os/setns.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/tests/os/setns_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/22160/diff/
Testing
-------
make check
sudo make check
Thanks,
Jie Yu