> On Aug. 23, 2018, 1:33 a.m., Chun-Hung Hsiao wrote: > > src/slave/paths.hpp > > Lines 424 (patched) > > <https://reviews.apache.org/r/68399/diff/1/?file=2074096#file2074096line424> > > > > 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. ;)
Yeh I considered that approach but arbitrarily went with the constant. I'll make this change. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68399/#review207787 ----------------------------------------------------------- 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 > >
