----------------------------------------------------------- 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 > >
