----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69132/#review209940 -----------------------------------------------------------
Fix it, then Ship it! LGTM! src/tests/containerizer/xfs_quota_tests.cpp Lines 147-152 (original), 147-164 (patched) <https://reviews.apache.org/r/69132/#comment294586> The command looks the same in both cases, the only difference is options. Maybe avoid duplicating code by constructing it? Like ```c++ string mountCmd = strings::format( "mount -t xfs%s %s %s", mountOptions.isNone() ? " " : " -o " + mountOptions.get(), loopDevice.get(), mntPath); ``` - Ilya Pronin On Oct. 23, 2018, 4:17 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69132/ > ----------------------------------------------------------- > > (Updated Oct. 23, 2018, 4:17 p.m.) > > > Review request for mesos, Ilya Pronin, Jacob Janco, and Jiang Yan Xu. > > > Repository: mesos > > > Description > ------- > > When we added the `filesystem/linux` isolator to the XFS isolation > tests, we broke them for older Linux distributions (specifically > CentOS 6). The `filesystem/linux` isolator needs to remount the > work directory in shared mode using `mount(8)`, but `mount(8)` > requires the mount point it is manipulating to be present in > `mtab(5)`. Since the XFS tests simply issued a `mount(2)` system > call, `mtab(5)` wasn't updated and the remount would fail. Modern > Linux distributions don't see this problem because `/` is mounted > shared by default, and `mtab(5)` is a symlink to `/proc/self/mounts` > (i.e. always in sync). > > The straightforward fix is to run `mount(8)` to make the XFS test > fixture mounts, so that `mount(8)` always sees a consistent view of > `mtab(5)`. > > > Diffs > ----- > > src/tests/containerizer/xfs_quota_tests.cpp > 084df067a8d97efdc86fc0ed878bc3f2edca06e5 > > > Diff: https://reviews.apache.org/r/69132/diff/1/ > > > Testing > ------- > > sudo make check (Fedora 28, CentOS 6) > > > Thanks, > > James Peach > >
