> On Nov. 14, 2017, 9:32 a.m., James Peach wrote: > > This is looking pretty good. > > > > You should be able to write a test for this using the > > [ROOT_XFS_TestBase](https://github.com/apache/mesos/blob/master/src/tests/containerizer/xfs_quota_tests.cpp) > > fixture. Probably a reasonable approach would be to make a XFS filesystem > > with and without `d_type` support and verify that `Provisioner::create` > > succeeds or fails. > > Meng Zhu wrote: > Thank you for the quick and informative review, James. > > I was planning to add a unit test. But the problem is that it would > require XFS installed. We will have to add a new configuration flag which > seems to overkill. Thus I only did manual tests following a similar > procedure. > > What do you think? > > James Peach wrote: > The XFS isolator tests (see `ROOT_XFS_TestBase`) already deal with all > this. They check for XFS being supported on the kernel and have helpers to > construct XFS filesystems on demand.
That's right, I thought about this. But this test is only enabled with compilation flag `--enable-xfs-disk-isolator` (which checks system XFS support). I feel lumping the d_type test under this flag is not appropriate. I see three options: (1) wrap the test under this flag. (2) skip the test at the moment. (3) introduce a new flag such as `--enable-xfs` which checks system XFS support, enables the `d_type` test and servers as a parent flag for `--enable-xfs-disk-isolator` (and any future XFS related functions). What do you think? - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63652/#review190962 ----------------------------------------------------------- On Nov. 14, 2017, 1:30 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63652/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2017, 1:30 p.m.) > > > Review request for mesos, Gilbert Song and James Peach. > > > Bugs: MESOS-8121 > https://issues.apache.org/jira/browse/MESOS-8121 > > > Repository: mesos > > > Description > ------- > > In unified containerizer, the d_type cannot be 1 > if we are using the overlay fs backend. > Raise error if user specifies overlayfs as backend. > Fallback to other backends in the default case and > raise a warning. > > > Diffs > ----- > > src/linux/fs.hpp cbc8bf79083ce2bc34fa698808eaf92764a577a9 > src/linux/fs.cpp 40c31a3ad2ccbeb59868c5e2d7b1809c613da8de > src/slave/containerizer/mesos/provisioner/provisioner.cpp > 450a3b32d69d2882973a6ed4e94e169a0256056b > > > Diff: https://reviews.apache.org/r/63652/diff/4/ > > > Testing > ------- > > Manually tested slave creation with default and overlayfs backends on XFS > based work_dir with different ftype mount options. > > > Thanks, > > Meng Zhu > >
