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

Reply via email to