----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54514/#review158504 -----------------------------------------------------------
3rdparty/stout/include/stout/posix/os.hpp (line 470) <https://reviews.apache.org/r/54514/#comment229272> It looks like you have decided not to take Jie's suggestion in [1], to implement this as a family of functions, `os::runstatedir`, `os:: localstatedir`, _etc_.? That's fine if so, I bring it up just to make sure that we're consciously making this decision to not do this, rather than accidentally letting it slip through the cracks. It looks like the vast majority of places we're putting something in `/var/run`, we're actually putting it in `/var/run/mesos`, so the utility of that abstraction is probably not particularly high. [1] https://reviews.apache.org/r/54335/ 3rdparty/stout/include/stout/posix/os.hpp (line 475) <https://reviews.apache.org/r/54514/#comment229270> Why call `os::var` a second time here? Did you mean for this to be simply `var.get()`? Here, and in the Windows version. 3rdparty/stout/include/stout/posix/os.hpp (line 476) <https://reviews.apache.org/r/54514/#comment229271> Do you think we should be adding `#include <stout/os/access.hpp>` to this and the Windows version make this header standalone? 3rdparty/stout/include/stout/posix/os.hpp (line 477) <https://reviews.apache.org/r/54514/#comment229273> I'm wondering if someone can speak to how we expect `runtime_dir` to be used here. If we expect that we're going to `su` or something after we get `runtime_dir` back, then it might be the case that either the user we're switching to, or from, doesn't have access. If that is the case, should we be checking this here, or after we get the path back? - Alex Clemmer On Dec. 8, 2016, 1:59 a.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54514/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2016, 1:59 a.m.) > > > Review request for mesos and Alex Clemmer. > > > Bugs: MESOS-6722 > https://issues.apache.org/jira/browse/MESOS-6722 > > > Repository: mesos > > > Description > ------- > > Encapsulates the platform-specific runtime directories > `/var/run/mesos` and `C:\ProgramData\mesos\runtime`. > Checks for read/write access to `/var/run` and `C:\ProgramData`, > and falls back to `os::temp()/mesos/runtime` on error. > > The `os::runtime_dir()` function is introduced to handle both the > platform-specificity of the the runtime data folders and the > error handling in the case of the default folder not being writable. > Note that previously the CLI code checked the usability of > `/var/run/mesos` on POSIX by checking `if (user == "root")` > instead of testing the read and write access of `/var/run`. > This made it so that Mesos would use `/tmp/mesos/runtime` > if it was run as any user other than `root`, > even if that user had write access to `/var/run`. > Checking for permission instead of checking a username > is the preferred way to handle permission checks. > > > Diffs > ----- > > 3rdparty/stout/include/stout/posix/os.hpp > 8443aa0cf0a8d8d52e36282611c2ab15ca4dd354 > 3rdparty/stout/include/stout/windows/os.hpp > 2f20ccc64e255a60a1b7f33d684969942f12e45f > > Diff: https://reviews.apache.org/r/54514/diff/ > > > Testing > ------- > > make && make check on Linux: no failures. > msbuild and attach to a master on Windows: no failures. > > > Thanks, > > Andrew Schwartzmeyer > >
