> On March 20, 2015, 5:21 p.m., Chi Zhang wrote: > > src/slave/containerizer/mesos/launch.cpp, line 296 > > <https://reviews.apache.org/r/31444/diff/2/?file=898403#file898403line296> > > > > 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. > > Jie Yu wrote: > I think make-rslave on / is to make sure any existing mounts are not > shared mount because we will umount them after pivot_root. We don't want > those umount be polulated to the host mount table. > > Chi Zhang wrote: > another data point: F20 uses systemd which marks all mount points shared > by default. inside the container namespace, a rslave on the / to make all of > them slave is necessary for the pivot_root to succeed. > > It seems to me that you can't have bidirectional shared mountpoints > inside the container otherwise pivot_root won't work.
+1 to Jie's comment, we need all the mounts to be slave mounts so we can move *and* umount them without affecting the host's mounts, i.e., this is more than is required for just a pivot_root. > On March 20, 2015, 5:21 p.m., Chi Zhang wrote: > > src/slave/containerizer/mesos/launch.cpp, line 132 > > <https://reviews.apache.org/r/31444/diff/2/?file=898403#file898403line132> > > > > 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)? A bind mount is only one example: "e.g., for a bind mount". It's assumed in the code that everything that is a path needs to be made relative to the chroot by prefixed with the chroot path. Note this is only for the mounts explicitly specified in the vector<Mount> mounts variable, not any mount table that's read from the system. Regardless, for your example of /dev/sda0, then devices should be created in <chroot>/dev so it would be correct to also prefix /dev/sda0 with the chroot path. - Ian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31444/#review77307 ----------------------------------------------------------- On April 8, 2015, 8:24 a.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31444/ > ----------------------------------------------------------- > > (Updated April 8, 2015, 8:24 a.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/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 > src/linux/fs.cpp 1c9cf3f2ffead37148e4f6a81cefdbb97f679b09 > src/slave/containerizer/mesos/launch.hpp > 7c8b535746b5ce9add00afef86fdb6faefb5620e > src/slave/containerizer/mesos/launch.cpp > 2f2d60e2011f60ec711d3b29fd2c157e30c83c34 > src/tests/launch_tests.cpp PRE-CREATION > > 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 > >
