----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69149/#review210109 -----------------------------------------------------------
Fix it, then Ship it! Looks good to me! Nice simplification. src/linux/fs.cpp Lines 510 (patched) <https://reviews.apache.org/r/69149/#comment294742> Should not be critical, but maybe check that the user is not trying to remount in the first place? Like checking that `!(flags & MS_REMOUNT)`? src/linux/fs.cpp Line 738 (original), 739-740 (patched) <https://reviews.apache.org/r/69149/#comment294743> Delete the old line with `MS_BIND`. src/linux/fs.cpp Line 752 (original), 747-748 (patched) <https://reviews.apache.org/r/69149/#comment294744> Ditto. - Ilya Pronin On Oct. 24, 2018, 4:49 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69149/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2018, 4:49 p.m.) > > > Review request for mesos, Gilbert Song, Ilya Pronin, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-9354 > https://issues.apache.org/jira/browse/MESOS-9354 > > > Repository: mesos > > > Description > ------- > > To make a bind mount read-only, you have to first make the bind mount, > then remount it with the read-only flag. This is a bit arcane, which is > why `mount(8)` does it automatically. > > This change updates `fs::mount()` to do the read-only remount > automatically when it is making a read-only bind mount so that every > caller doesn't have to carry special code to make it work correctly. All > the callers that make an explicit remount are updated to simply pass > the `MS_READONLY` flag if necessary. > > > Diffs > ----- > > src/examples/test_csi_plugin.cpp 7fa325e94304aec8927e345ebdac380105f624a8 > src/linux/fs.hpp 502f85c4a32d8658bdd701975dd5ac3d802d308e > src/linux/fs.cpp 9055ef42edd1fb90e1026d1d603a9ba902cfc1fd > src/slave/containerizer/docker.cpp 192dc29576a99fdc671bb842c01f50cd30dc20e1 > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp > 24c9fd6beb9657b80b33dc31c2939083c1aa9110 > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > a47899cb528eef103f299def3bd3466905ac5b51 > src/slave/containerizer/mesos/isolators/gpu/isolator.cpp > dbbf92ffbe4a46cedca5b53f6ba172bfb308100e > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 64271dfbb5c074ad3ac8d2a64c3943d739c0fffa > src/slave/containerizer/mesos/isolators/volume/host_path.cpp > 2e03ef50a290c046ae2b02b332d3d007b572429d > src/slave/containerizer/mesos/isolators/volume/image.cpp > 53cbaefeef7a6e10149e241e07d6e9cb8d510fc9 > src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp > 21d9528c23d9142eccec456184b42d085b57d12c > src/slave/containerizer/mesos/provisioner/backends/bind.cpp > 7d564dc7de3e3de8726c7fd42a301d979c4a2574 > src/tests/containerizer/fs_tests.cpp > 23cad35b5db81a70b43bec1c1dbafe008c8dd4da > > > Diff: https://reviews.apache.org/r/69149/diff/1/ > > > Testing > ------- > > sudo make check (Fedora 28) > > > Thanks, > > James Peach > >
