You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Budnik <ab...@mesosphere.com> on 2017/10/17 13:44:58 UTC
Review Request 63074: Changed return type of `ns::getns()` from `Try`
to `Result`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63074/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov, Jie Yu, and Kevin Klues.
Bugs: MESOS-7504
https://issues.apache.org/jira/browse/MESOS-7504
Repository: mesos
Description
-------
Previously, `getns` checked existence of process's pid before trying to
`stat` the namespace file. If the pid didn't exist, then it returned a
failure.
Now, `getns` checks existence of process's pid only on failure of
`stat` for the namespace file. If the pid doesn't exist, then it returns
`None()`.
Diffs
-----
src/linux/ns.hpp e96116343b132ff4aae36c7b3c1b0e99c41246af
src/slave/containerizer/mesos/utils.cpp ec6d6c79b0f93cabe880cd697094c20e999af4d1
src/tests/containerizer/isolator_tests.cpp 5072bafabc0e37bc756588b88e4d9f8a8c84d337
src/tests/containerizer/ns_tests.cpp 2e28139d241e5e37c8fd0f8b4531c3d271727c8d
Diff: https://reviews.apache.org/r/63074/diff/1/
Testing
-------
make check
Thanks,
Andrei Budnik
Re: Review Request 63074: Changed return type of `ns::getns()` from
`Try` to `Result`.
Posted by Andrei Budnik <ab...@mesosphere.com>.
> On Oct. 17, 2017, 9:11 p.m., Andrei Budnik wrote:
> > src/linux/ns.hpp
> > Lines 246 (patched)
> > <https://reviews.apache.org/r/63074/diff/1/?file=1860608#file1860608line250>
> >
> > There might be a race when `stat` has failed, but a new process with the same PID is launched before `os::exist` is called. This race reproduced when two pre-exec hooks was started.
Fixed.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63074/#review188395
-----------------------------------------------------------
On Oct. 17, 2017, 1:44 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63074/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:44 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7504
> https://issues.apache.org/jira/browse/MESOS-7504
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, `getns` checked existence of the process's pid before
> trying to `stat` the namespace file. If the pid didn't exist, then
> it returned a failure. However, the process might exit before `stat`
> is called.
>
> Now, `getns` doesn't check existence of the process's pid explicitly.
> The fact that the process is gone can be detected by checking returned
> errno of `stat`: if it returns ENOENT, then the function returns
> `None()`.
>
>
> Diffs
> -----
>
> src/linux/ns.hpp e96116343b132ff4aae36c7b3c1b0e99c41246af
> src/slave/containerizer/mesos/utils.cpp ec6d6c79b0f93cabe880cd697094c20e999af4d1
> src/tests/containerizer/isolator_tests.cpp 5072bafabc0e37bc756588b88e4d9f8a8c84d337
> src/tests/containerizer/ns_tests.cpp 2e28139d241e5e37c8fd0f8b4531c3d271727c8d
>
>
> Diff: https://reviews.apache.org/r/63074/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 63074: Changed return type of `ns::getns()` from
`Try` to `Result`.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63074/#review188395
-----------------------------------------------------------
src/linux/ns.hpp
Lines 246 (patched)
<https://reviews.apache.org/r/63074/#comment265377>
There might be a race when `stat` has failed, but a new process with the same PID is launched before `os::exist` is called. This race reproduced when two pre-exec hooks was started.
- Andrei Budnik
On Oct. 17, 2017, 1:44 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63074/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:44 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7504
> https://issues.apache.org/jira/browse/MESOS-7504
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, `getns` checked existence of process's pid before trying to
> `stat` the namespace file. If the pid didn't exist, then it returned a
> failure.
>
> Now, `getns` checks existence of process's pid only on failure of
> `stat` for the namespace file. If the pid doesn't exist, then it returns
> `None()`.
>
>
> Diffs
> -----
>
> src/linux/ns.hpp e96116343b132ff4aae36c7b3c1b0e99c41246af
> src/slave/containerizer/mesos/utils.cpp ec6d6c79b0f93cabe880cd697094c20e999af4d1
> src/tests/containerizer/isolator_tests.cpp 5072bafabc0e37bc756588b88e4d9f8a8c84d337
> src/tests/containerizer/ns_tests.cpp 2e28139d241e5e37c8fd0f8b4531c3d271727c8d
>
>
> Diff: https://reviews.apache.org/r/63074/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Andrei Budnik
>
>
Re: Review Request 63074: Changed return type of `ns::getns()` from
`Try` to `Result`.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63074/#review188689
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Oct. 17, 2017, 1:44 p.m., Andrei Budnik wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63074/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:44 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Kevin Klues.
>
>
> Bugs: MESOS-7504
> https://issues.apache.org/jira/browse/MESOS-7504
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, `getns` checked existence of the process's pid before
> trying to `stat` the namespace file. If the pid didn't exist, then
> it returned a failure. However, the process might exit before `stat`
> is called.
>
> Now, `getns` doesn't check existence of the process's pid explicitly.
> The fact that the process is gone can be detected by checking returned
> errno of `stat`: if it returns ENOENT, then the function returns
> `None()`.
>
>
> Diffs
> -----
>
> src/linux/ns.hpp e96116343b132ff4aae36c7b3c1b0e99c41246af
> src/slave/containerizer/mesos/utils.cpp ec6d6c79b0f93cabe880cd697094c20e999af4d1
> src/tests/containerizer/isolator_tests.cpp 5072bafabc0e37bc756588b88e4d9f8a8c84d337
> src/tests/containerizer/ns_tests.cpp 2e28139d241e5e37c8fd0f8b4531c3d271727c8d
>
>
> Diff: https://reviews.apache.org/r/63074/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Andrei Budnik
>
>