> On July 8, 2015, 11:34 p.m., Vinod Kone wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 418 > > <https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line418> > > > > Is it possible for rootfs to not exist when we are here? If not, there > > should be a CHECK and a comment on what guarantees its presence (the fact > > that there is a forked pid?).
the field rootfs is just a option, so I think that's why you can just pass that in. > On July 8, 2015, 11:34 p.m., Vinod Kone wrote: > > include/mesos/slave/isolator.hpp, lines 70-71 > > <https://reviews.apache.org/r/34137/diff/2/?file=989753#file989753line70> > > > > We don't align arguments for constructors this way IIRC. > > > > ExecutorRunState(arg1, > > arg2, > > ....); Looks like the latest revision has the right format right? > On July 8, 2015, 11:34 p.m., Vinod Kone wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 1249 > > <https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line1249> > > > > why include the containerid? doesn't the caller of this method already > > know that? The caller knows this, but since this shows up in the log it's easier to correlate the error with a container id. > On July 8, 2015, 11:34 p.m., Vinod Kone wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 1280 > > <https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line1280> > > > > ditto. why include the container id? You're right I don't tihnk containerId here is needed. > On July 8, 2015, 11:34 p.m., Vinod Kone wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 818 > > <https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line818> > > > > where is the update to launchFlags to add 'rootfs' ? which review? Looks like this field is already added in master. commit 15bf9f67e3d9489e49b2176e1e4221a1a47fd0c9 Author: Ian Downes <idow...@twitter.com> Date: Thu Dec 18 15:46:15 2014 -0800 Support chrooting in MesosContainerizer launch helper. Review: https://reviews.apache.org/r/31444/ - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review91014 ----------------------------------------------------------- On July 12, 2015, 4:47 a.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34137/ > ----------------------------------------------------------- > > (Updated July 12, 2015, 4:47 a.m.) > > > Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > The MesosContainerizer can optionally provision a root filesystem for the > containers. > > > Diffs > ----- > > include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 > src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c > src/slave/containerizer/mesos/containerizer.hpp > 3ac2387eabded61c897a5016655aae10cd1bca91 > src/slave/containerizer/mesos/containerizer.cpp > 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 > src/slave/containerizer/provisioner.hpp PRE-CREATION > src/slave/containerizer/provisioner.cpp PRE-CREATION > src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 > src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 > src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f > src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 > src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 > src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 > src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e > > Diff: https://reviews.apache.org/r/34137/diff/ > > > Testing > ------- > > > Thanks, > > Ian Downes > >