----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68399/#review207787 -----------------------------------------------------------
src/slave/paths.hpp Lines 424 (patched) <https://reviews.apache.org/r/68399/#comment291288> It looks good to me to introduce this new constant, but instead of exporting it, it seems better to me if we keep this constant internal and introduce the following function: ``` Try<list<string>> getPersistentVolumePaths( const string& workDir) { return fs::list(path::join( workDir, VOLUMES_DIR, ROLES_DIR, "*", // Role. "*")); // Persistence ID. } ``` So we could have a single place to control the filesystem layout. Also, it seems you need `"roles"` in the subsequent patch and I was wondering why there is no corresponding constant. ;) - Chun-Hung Hsiao On Aug. 16, 2018, 11:51 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68399/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2018, 11:51 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, Jie Yu, Joseph Wu, > and Jiang Yan Xu. > > > Bugs: MESOS-5158 > https://issues.apache.org/jira/browse/MESOS-5158 > > > Repository: mesos > > > Description > ------- > > The `disk/xfs` isolator needs to scan persistent volumes at recovery > time. Add a `VOLUMES_DIR` constant to make it easier to find the > relevant isolator code. > > > Diffs > ----- > > src/slave/paths.hpp 015896453410a33923eed07b3e676be19af62a48 > src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 > > > Diff: https://reviews.apache.org/r/68399/diff/1/ > > > Testing > ------- > > sudo make check (Fedora 28) > > > Thanks, > > James Peach > >
