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/05/01 19:26:10 UTC
Review Request 20958: Make execute in MesosContainerizer async signal safe.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/
-----------------------------------------------------------
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Repository: mesos-git
Description
-------
We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
Replace all stout calls with just the appropriate system call.
Construct Envp and pass into child and use execle for the environment.
Use open and dup2 rather than freopen.
Determine gid and uid and pass into child.
Diffs
-----
src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
Diff: https://reviews.apache.org/r/20958/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43245
-----------------------------------------------------------
I held off from making too many comments since I see you've been resolving things but haven't updated the diff yet.
Are there tests that ensure that the stderr/stdout file redirection is working correctly? If not we should definitely add a test here.
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77343>
What is "buf", can you use a more descriptive name?
"buffer" seems a bit odd here, perhaps "value" or "result" or "dummy"?
Please do a clean up of the names in MesosContainerizerProcess::exec as well.
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77342>
s/len/length/
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77348>
To add to Toby's comments, in the future we may want to add an ERRNO_ABORT() that uses a buffer with 'strerror_r' to print the error.
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77347>
newline here?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77346>
Can you add some single quotes around the command in the message?
Command '...' failed to execute successfully
- Ben Mahler
On May 1, 2014, 5:26 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 5:26 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Till Toenshoff <to...@me.com>.
> On May 20, 2014, 12:34 a.m., Tobias Weingartner wrote:
> > src/slave/containerizer/mesos_containerizer.cpp, line 442
> > <https://reviews.apache.org/r/20958/diff/3/?file=584749#file584749line442>
> >
> > Should contain a mode (0666).
Aye, that is what is biting us now in MESOS-1402
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43447
-----------------------------------------------------------
On May 19, 2014, 10:57 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 19, 2014, 10:57 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp 2a4816ec34f2298b71e8591c1e92d77f94ef4cb3
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Tobias Weingartner <tw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43447
-----------------------------------------------------------
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77601>
O_NONBLOCK is not set on these, right? :)
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77604>
Is there a "race" or interaction between this code, and the ABORT() calls?
Possibly an interaction between these operations and libc/libc++ stdio? setvbuf()? Unsure.
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77602>
Should contain a mode (0666).
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77603>
Ditto
- Tobias Weingartner
On May 19, 2014, 10:57 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 19, 2014, 10:57 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp 2a4816ec34f2298b71e8591c1e92d77f94ef4cb3
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Tobias Weingartner <tw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43452
-----------------------------------------------------------
Ship it!
The rest lgtm.
- Tobias Weingartner
On May 19, 2014, 10:57 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 19, 2014, 10:57 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp 2a4816ec34f2298b71e8591c1e92d77f94ef4cb3
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/
-----------------------------------------------------------
(Updated May 19, 2014, 3:57 p.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Changes
-------
Addressed comments. Test forthcoming.
Repository: mesos-git
Description
-------
We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
Replace all stout calls with just the appropriate system call.
Construct Envp and pass into child and use execle for the environment.
Use open and dup2 rather than freopen.
Determine gid and uid and pass into child.
Diffs (updated)
-----
src/slave/containerizer/mesos_containerizer.cpp 2a4816ec34f2298b71e8591c1e92d77f94ef4cb3
Diff: https://reviews.apache.org/r/20958/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43291
-----------------------------------------------------------
Ship it!
LGTM. Ditto BenM's comment. Do we have any existing test that tests the IO redirection? If not, we probably want to create one if possible.
- Jie Yu
On May 16, 2014, 10:06 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 16, 2014, 10:06 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp 2a4816ec34f2298b71e8591c1e92d77f94ef4cb3
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/
-----------------------------------------------------------
(Updated May 16, 2014, 3:06 p.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Changes
-------
Addressed review comments.
Repository: mesos-git
Description
-------
We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
Replace all stout calls with just the appropriate system call.
Construct Envp and pass into child and use execle for the environment.
Use open and dup2 rather than freopen.
Determine gid and uid and pass into child.
Diffs (updated)
-----
src/slave/containerizer/mesos_containerizer.cpp 2a4816ec34f2298b71e8591c1e92d77f94ef4cb3
Diff: https://reviews.apache.org/r/20958/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Ian Downes <ia...@gmail.com>.
> On May 16, 2014, 11:11 a.m., Tobias Weingartner wrote:
> > In general, I'd love to see us actually print out either the errno value, or even better the errno string upon an ABORT() or other failure caused by a syscall. It makes tracking down issues much easier.
Yes, that would be preferred. We don't yet have support for an async signal safe equivalent to perror + abort. Ticket for creating a safe ERRNO_ABORT as described by [~bmahler] below: https://issues.apache.org/jira/browse/MESOS-1381
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43231
-----------------------------------------------------------
On May 1, 2014, 10:26 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 10:26 a.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Ben Mahler <be...@gmail.com>.
> On May 16, 2014, 6:11 p.m., Tobias Weingartner wrote:
> > src/slave/containerizer/mesos_containerizer.cpp, lines 323-325
> > <https://reviews.apache.org/r/20958/diff/1/?file=572684#file572684line323>
> >
> > While this may work, there is no POSIX reason that you could not get a short read here...
Ian, if you instead use a "char" as the dummy value, does that eliminate the problem of a short read?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43231
-----------------------------------------------------------
On May 1, 2014, 5:26 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 5:26 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Tobias Weingartner <tw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review43231
-----------------------------------------------------------
In general, I'd love to see us actually print out either the errno value, or even better the errno string upon an ABORT() or other failure caused by a syscall. It makes tracking down issues much easier.
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77314>
While this may work, there is no POSIX reason that you could not get a short read here...
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77315>
Why not add perror() or at least the errno to this output?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77316>
Ditto
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77318>
Print error as to why?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment77319>
Ditto
- Tobias Weingartner
On May 1, 2014, 5:26 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 5:26 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review42390
-----------------------------------------------------------
Bad patch!
Reviews applied: [20816]
Failed command: git apply --index 20816.patch
Error:
error: patch failed: src/slave/containerizer/linux_launcher.hpp:38
error: src/slave/containerizer/linux_launcher.hpp: patch does not apply
error: patch failed: src/slave/containerizer/linux_launcher.cpp:16
error: src/slave/containerizer/linux_launcher.cpp: patch does not apply
- Mesos ReviewBot
On May 1, 2014, 5:26 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 5:26 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Ian Downes <ia...@gmail.com>.
> On May 6, 2014, 10:56 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos_containerizer.cpp, lines 544-551
> > <https://reviews.apache.org/r/20958/diff/1/?file=572684#file572684line544>
> >
> > This is now part of 'fetch', but it has nothing to do with 'fetch'. How about pulling it to a top level method and modify the chain in '_launch'?
I think it's natural to include it as the final part of fetch. I don't want to pull all pieces up to the chain at _launch so I'm going to leave it where it is. Feel free to object :-)
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review42301
-----------------------------------------------------------
On May 1, 2014, 10:26 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 10:26 a.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 20958: Make execute in MesosContainerizer async signal
safe.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20958/#review42301
-----------------------------------------------------------
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76080>
"Failed to open stdout"?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76090>
dup2 of what failed?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76092>
For the sake of consistency, do you need to check the return value of 'close' here?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76081>
"Failed to open stderr"?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76091>
ditto
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76077>
This is now part of 'fetch', but it has nothing to do with 'fetch'. How about pulling it to a top level method and modify the chain in '_launch'?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76076>
Why ABORT here? Shouldn't just return Failure("..")?
src/slave/containerizer/mesos_containerizer.cpp
<https://reviews.apache.org/r/20958/#comment76078>
Can we make os::getuid accept an optional user name? In that way, here you can just do:
Try<uid_t> uid = os::getuid(user);
....
- Jie Yu
On May 1, 2014, 5:26 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20958/
> -----------------------------------------------------------
>
> (Updated May 1, 2014, 5:26 p.m.)
>
>
> Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> We *may* have observed deadlocks in the forked child when testing some other new code so this makes the execute() function in MesosContainerizer completely async-signal safe.
>
> Reviewers: I'd appreciate it if you could carefully check that I've made this completely async signal safe.
>
>
> Replace all stout calls with just the appropriate system call.
>
> Construct Envp and pass into child and use execle for the environment.
>
> Use open and dup2 rather than freopen.
>
> Determine gid and uid and pass into child.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos_containerizer.cpp df57e5443d0e3a94588464d83822ed5e3e9f88ea
>
> Diff: https://reviews.apache.org/r/20958/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>