mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <xuj...@apple.com>
Subject Re: Review Request 53791: Use the stout ELF parser to collect Linux rootfs files.
Date Thu, 02 Feb 2017 01:31:11 GMT


> On Feb. 1, 2017, 3:51 p.m., James Peach wrote:
> > src/tests/containerizer/rootfs.cpp, line 90
> > <https://reviews.apache.org/r/53791/diff/8/?file=1589112#file1589112line90>
> >
> >     What are you asking for here? This can fail for a number of reasons and `errno`
describes them. Squashing that to a fixed string would be misleading.
> 
> Jiang Yan Xu wrote:
>     I am saying we know "This can fail for a number of reasons and errno describes them"
because we are breaking the abstraction here. Perhaps my arugment seems silly because `os::exists()`
has a 9-line implementation and we can look at the source. But if it's 900 lines or we can't
see the source the same principle still applies right?
>     
>     `os::exists()` current exposes a `bool` and expects it to be used to interpret what's
happened. If we peek inside the implementation and write code based on the internals, it's
likely to break when the implementation changes.
>     
>     Perhaps `bool os::exists()` is currently lacking in ability to reveal errors, should
we then change it to `Try<bool> exists()`? If that's more suitable as a TODO or we are
not sure, I think we should just leave it as it is right now. There are hundreds of other
uses in the codebase which may be susceptible to conditions like "the path does exist but
we are not permitted to access it" so maybe it's worth a JIRA to fix them anyways? (Of course
that shouldn't block our change here)

Filed https://issues.apache.org/jira/browse/MESOS-7052 for this.


- Jiang Yan


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


On Feb. 1, 2017, 3:51 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53791/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2017, 3:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Kevin Klues, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6588
>     https://issues.apache.org/jira/browse/MESOS-6588
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The dependencies for the programs we need in the Linux root
> filesystem vary over time and across distributions. Since Stout
> has support for parsing the library dependencies of ELF binaries,
> use this to collect the necessary dependencies when constructing
> a root filesystem for the tests.
> 
> 
> Diffs
> -----
> 
>   src/tests/containerizer/rootfs.hpp 9648638c1d4b1f1c49708e058e9b930c5ae32138 
>   src/tests/containerizer/rootfs.cpp f11f126fa4f3130b6778bfe7e7d0f025a638f06e 
> 
> Diff: https://reviews.apache.org/r/53791/diff/
> 
> 
> Testing
> -------
> 
> sudo make check (Fedora 24)
> 
> 
> Thanks,
> 
> James Peach
> 
>


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