----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58278/#review171475 -----------------------------------------------------------
Ship it! Ship It! src/slave/containerizer/mesos/isolators/filesystem/linux.cpp Line 827 (original), 829 (patched) <https://reviews.apache.org/r/58278/#comment244412> Not sure `strings::startsWith` would be the safest test if `entry.target` is a subdirectory of `sandbox`. Imagine that we have a mount point target at `${MESOS_DIRECTORY}-with-suffix` rather than `${MESOS_DIRECTORY}/sub/` and the sandbox is at `${MESOS_DIRECTORY}`, the `strings::startsWith` function will return `true` and the entry target got unmounted incorrectly. Not an issue for the scope of this diff, and possibly the `${MESOS_DIRECTORY}-with-suffix` case would be really rare but not impossible. Would we consider addressing cases like this? - Jason Lai On April 7, 2017, 11:45 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58278/ > ----------------------------------------------------------- > > (Updated April 7, 2017, 11:45 p.m.) > > > Review request for mesos, Gilbert Song, Jason Lai, and Zhitao Li. > > > Bugs: MESOS-7366 > https://issues.apache.org/jira/browse/MESOS-7366 > > > Repository: mesos > > > Description > ------- > > Use MNT_DETACH when unmounting persistent volumes in Linux filesystem > isolator to workaround an issue of incorrect handling of container > destroy failures. Currently, if isolator cleanup returns a failure, > the slave will treat the container as terminated, and will schedule > the cleanup of the container's sandbox. Since the mount hasn't been > removed in the sandbox (e.g., due to EBUSY), that'll result in data in > the persistent volume being incorrectly deleted. Use MNT_DETACH so > that the mount point in the sandbox will be removed immediately. See > MESOS-7366 for more details. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > ae0031d8d8d6dfe0334b605fbb85e83de88ab436 > > > Diff: https://reviews.apache.org/r/58278/diff/1/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Jie Yu > >
