mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 37853: Overlay filesystem provisioning backend
Date Thu, 27 Aug 2015 22:42:42 GMT

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


This looks great! Thanks Mei! I think the main thing left here is the test. You should be
able to write some simple tests that test the overlay functionality (see the bind mount tests).
Looking forward to the tests!


src/slave/containerizer/provisioners/backends/overlay.cpp (line 58)
<https://reviews.apache.org/r/37853/#comment152403>

    You also want to check if overlay fs is supported or not. Not every linux kernel supports
overlay fs.



src/slave/containerizer/provisioners/backends/overlay.cpp (line 98)
<https://reviews.apache.org/r/37853/#comment152425>

    No need to use process:: prefix as you already have `using namespace process` above.
    
    Please all such occurances in this file.



src/slave/containerizer/provisioners/backends/overlay.cpp (lines 101 - 103)
<https://reviews.apache.org/r/37853/#comment152406>

    Hum, I'd like to understand why overlay backend cannot support 1 layer. Any reason?



src/slave/containerizer/provisioners/backends/overlay.cpp (lines 110 - 114)
<https://reviews.apache.org/r/37853/#comment152428>

    Could you please add a comment about the ordering the layers will be stacked.
    
    I think we should add a document at the backend interface as well stating that layers
are stacking in such a way that the front layer in the vector will be the bottom most layer
(i.e., applied first).
    
    The lowerdir here should actually reverse the order of the layers vector.
    
    ```
    The specified lower directories will be stacked beginning from the rightmost one and going
left.  In the above example lower1 will be the top, lower2 the middle and lower3 the bottom
layer.
    ```



src/slave/containerizer/provisioners/backends/overlay.cpp (line 111)
<https://reviews.apache.org/r/37853/#comment152424>

    I think you should be able to use strings::join here:
    
    ```
    strings::join(":", layers);
    ```


- Jie Yu


On Aug. 27, 2015, 9:11 p.m., Mei Wan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37853/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 9:11 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.
> 
> 
> Bugs: MESOS-2971
>     https://issues.apache.org/jira/browse/MESOS-2971
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented the overlay filesystem backend by layering the images as a read-only filesystem.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 7b620ff 
>   src/slave/containerizer/provisioners/backend.cpp 2f7c335 
>   src/slave/containerizer/provisioners/backends/overlay.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/backends/overlay.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37853/diff/
> 
> 
> Testing
> -------
> 
> I haven't done any official testing. When I was working off Ian's branch, I tested it
manually and the provisioning works.
> 
> 
> Thanks,
> 
> Mei Wan
> 
>


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