----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31444/#review77307 -----------------------------------------------------------
src/slave/containerizer/mesos/launch.cpp <https://reviews.apache.org/r/31444/#comment125218> This works for the current table, but i don't think checking for the starting / is a logically correct way to check for bind mounts. e.g.: mount /dev/sda0 /mnt is just a regular mount maybe check &MS_BIND && !(&MS_REMOUNT)? src/slave/containerizer/mesos/launch.cpp <https://reviews.apache.org/r/31444/#comment125223> From my testing, you only need to make-slave on the newRoot for the pivot_root to work. Is the make-rslave on the / a good-to-have or required? could it possibly not matter to do it because once you do the chroot in a few steps, mount points under the old / can't be seen anyway? This patch has a lot of tricky maneuvers i think documenting the 'minimumly required' steps to achieve the goal is very important. it can be hard to make a change in the future. src/slave/containerizer/mesos/launch.cpp <https://reviews.apache.org/r/31444/#comment125219> I confirm that this is the case. This seems to have been fixed recently in the patch series that fixes https://bugs.launchpad.net/apparmor/+bug/1401621 src/slave/containerizer/mesos/launch.cpp <https://reviews.apache.org/r/31444/#comment125229> about the MS_REC here, it is a bit unclear to me the sub mount points under the new ROOT, at this point mountAll hasn't been called, even if MS_REC is not used here, these mounts will be hidden (see my reply to Jie as well). supposedly the submounts are prepared by the caller that tells us the newRoot, when execute is called with newRoot, are all the mounts under newRoot including newRoot shared (and the caller acknowledges that here in execute they are all made slave)? or only the newRoot is shared? (in that case are the other mount points under newRoot private)? src/slave/containerizer/mesos/launch.cpp <https://reviews.apache.org/r/31444/#comment125230> why not here just do the pivot_root . old chroot . like the man page recommends? - Chi Zhang On March 17, 2015, 10:44 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31444/ > ----------------------------------------------------------- > > (Updated March 17, 2015, 10:44 p.m.) > > > Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, > and James Peach. > > > Bugs: MESOS-2350 > https://issues.apache.org/jira/browse/MESOS-2350 > > > Repository: mesos > > > Description > ------- > > Optionally take a path that the launch helper should chroot to before > exec'ing the executor. It is assumed that the work directory is mounted to > the appropriate location under the chroot. In particular, the path to the > executor must be relative to the chroot. > > Configuration that should be private to the chroot is done during the launch, > e.g. mounting proc and statically configuring basic devices. It is assumed > that other configuration, e.g., preparing the image, mounting in volumes or > persistent resources, is done by the caller. > > Mounts can be made to the chroot (e.g., updating the volumes or persistent > resources) and they will propagate in to the container but mounts made inside > the container will not propagate out to the host. > > It currently assumes that at least {{chroot}}/tmp is writeable and that mount > points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot. > > This is specific to Linux. > > > Diffs > ----- > > src/slave/containerizer/mesos/launch.hpp > 7c8b535746b5ce9add00afef86fdb6faefb5620e > src/slave/containerizer/mesos/launch.cpp > 2f2d60e2011f60ec711d3b29fd2c157e30c83c34 > > Diff: https://reviews.apache.org/r/31444/diff/ > > > Testing > ------- > > Manual testing only so far. This is harder to automate because we need a > self-contained chroot to execute something in... Suggestions welcome. > > > Thanks, > > Ian Downes > >
