mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Clemmer <clemmer.alexan...@gmail.com>
Subject Re: Review Request 54514: Add `os::runtime_dir()` to Stout.
Date Thu, 08 Dec 2016 20:13:44 GMT


> On Dec. 8, 2016, 6:45 a.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/posix/os.hpp, line 477
> > <https://reviews.apache.org/r/54514/diff/1/?file=1579536#file1579536line477>
> >
> >     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?
> 
> Andrew Schwartzmeyer wrote:
>     I, too, would like this information. But I'll note that logically this is consistent
with the previous code, which ran the `user == "root"` check at the same time.

That's true, but before we were checking `user == "root"` in the CLI; here, we're always checking
it when we call `runtime_dir`. I guess I'm just asking if the semantics of _current user `os::access`_
should really be baked into what is essentially a function that reports a path. And, if it
is, is there a way to communicate those semantics to all users of the function? (_e.g._, by
adding an argument called `user` or something).


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54514/#review158504
-----------------------------------------------------------


On Dec. 8, 2016, 8:11 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54514/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2016, 8:11 p.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message